Re: [PATCH] usbip: prevent priv from uaf in cmd submit

[Shuah Khan <skhan@linuxfoundation.org>]

On 8/26/26 19:21, Edward Adam Davis wrote:
> The num_urbs is 1, and the request is not special, so it is not tweaked.
> After stub_complete() executes, priv is added to the tx queue.
> stub_tx_loop() immediately dequeues it and frees priv; the subsequent
> loop iteration then checks priv->num_urbs, which ultimately triggers [1].
> 
> Use num_urbs directly to avoid accessing the already-freed priv.
> 
> [1]
> BUG: KASAN: slab-use-after-free in stub_recv_cmd_submit drivers/usb/usbip/stub_rx.c:609 [inline]
> Read of size 4 at addr ffff888034999db0 by task stub_rx/6028
> Call Trace:
>   stub_recv_cmd_submit drivers/usb/usbip/stub_rx.c:609 [inline]
>   stub_rx_pdu drivers/usb/usbip/stub_rx.c:688 [inline]
>   stub_rx_loop+0x2d3b/0x31c0 drivers/usb/usbip/stub_rx.c:707
> 
> Allocated by task 6028:
>   stub_priv_alloc drivers/usb/usbip/stub_rx.c:314 [inline]
>   stub_recv_cmd_submit drivers/usb/usbip/stub_rx.c:490 [inline]
>   stub_rx_pdu drivers/usb/usbip/stub_rx.c:688 [inline]
>   stub_rx_loop+0x638/0x31c0 drivers/usb/usbip/stub_rx.c:707
> 
> Freed by task 6029:
>   stub_free_priv_and_urb+0x427/0x570 drivers/usb/usbip/stub_main.c:321
>   stub_send_ret_submit+0xecf/0x1810 drivers/usb/usbip/stub_tx.c:333
>   stub_tx_loop+0xe2/0x3e0 drivers/usb/usbip/stub_tx.c:437
> 
> Fixes: ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver")
> Reported-by: syzbot+06fa667a0931a3430026@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=06fa667a0931a3430026
> Signed-off-by: Edward Adam Davis <eadavis@sina.com>
> ---
>   drivers/usb/usbip/stub_rx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
> index 1e9ae578810d..85370b3bc600 100644
> --- a/drivers/usb/usbip/stub_rx.c
> +++ b/drivers/usb/usbip/stub_rx.c
> @@ -606,7 +606,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
>   		return;
>   
>   	/* urb is now ready to submit */
> -	for (i = 0; i < priv->num_urbs; i++) {
> +	for (i = 0; i < num_urbs; i++) {
>   		if (!is_tweaked) {
>   			ret = usb_submit_urb(priv->urbs[i], GFP_KERNEL);
>   

This fix is incorrect. if priv is being free'd it has to be handled differently
and not by using num_urbs. priv is accessed in this loop.

thanks,
-- Shuah