usbip: prevent priv from uaf in cmd submit

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

Posted by Edward Adam Davis 1 month ago

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);
 
-- 
2.43.0

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

Posted by Shuah Khan 4 days, 13 hours ago

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