[PATCH] usbip: stub: keep URB array state consistent on errors

[PATCH] usbip: stub: keep URB array state consistent on errors

[Weiming Shi]

stub_priv_alloc() links a zeroed stub_priv to priv_init before the URB
array is allocated.  The error handler may therefore consume the object
after stub_recv_cmd_submit() reports SDEV_EVENT_ERROR_MALLOC.

Do not publish num_urbs until the array exists.  If a later URB allocation
fails, clear the freed array pointer and count before the event handler can
walk them.  This prevents a NULL dereference on array-allocation failure
and freed-array reads that can lead to invalid URB use or a double free on
later failures.

Fixes: ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver")
Cc: stable@vger.kernel.org # 5.4+
Reported-by: co+66c3f58096d0bde8@bugs.sh
Closes: https://lore.kernel.org/all/J3vQPnNHSkQ6YUiFvu1bk8ai3Q8aZ7a9J1Y7%40bugs.sh/
Assisted-by: Codex:gpt-5
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/usb/usbip/stub_rx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 1e9ae578810d9..33058fb05b2f4 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -534,10 +534,10 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
 	}
 
 	/* allocate urb array */
-	priv->num_urbs = num_urbs;
 	priv->urbs = kmalloc_objs(*priv->urbs, num_urbs);
 	if (!priv->urbs)
 		goto err_urbs;
+	priv->num_urbs = num_urbs;
 
 	/* setup a urb */
 	if (support_sg) {
@@ -641,6 +641,8 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
 
 err_urb:
 	kfree(priv->urbs);
+	priv->urbs = NULL;
+	priv->num_urbs = 0;
 err_urbs:
 	kfree(buffer);
 	sgl_free(sgl);
-- 
2.55.0

Re: [PATCH] usbip: stub: keep URB array state consistent on errors

[Shuah Khan]

On 9/12/26 02:39, Weiming Shi wrote:
> stub_priv_alloc() links a zeroed stub_priv to priv_init before the URB
> array is allocated.  The error handler may therefore consume the object
> after stub_recv_cmd_submit() reports SDEV_EVENT_ERROR_MALLOC.
> 
> Do not publish num_urbs until the array exists.  If a later URB allocation
> fails, clear the freed array pointer and count before the event handler can
> walk them.  This prevents a NULL dereference on array-allocation failure
> and freed-array reads that can lead to invalid URB use or a double free on
> later failures.

How is this problem reproduced? Why is this a problem in this code path?

> 
> Fixes: ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver")
> Cc: stable@vger.kernel.org # 5.4+
> Reported-by: co+66c3f58096d0bde8@bugs.sh
> Closes: https://lore.kernel.org/all/J3vQPnNHSkQ6YUiFvu1bk8ai3Q8aZ7a9J1Y7%40bugs.sh/
> Assisted-by: Codex:gpt-5
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>   drivers/usb/usbip/stub_rx.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
> index 1e9ae578810d9..33058fb05b2f4 100644
> --- a/drivers/usb/usbip/stub_rx.c
> +++ b/drivers/usb/usbip/stub_rx.c
> @@ -534,10 +534,10 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
>   	}
>   
>   	/* allocate urb array */
> -	priv->num_urbs = num_urbs;
>   	priv->urbs = kmalloc_objs(*priv->urbs, num_urbs);
>   	if (!priv->urbs)
>   		goto err_urbs;
> +	priv->num_urbs = num_urbs;
>   
>   	/* setup a urb */
>   	if (support_sg) {
> @@ -641,6 +641,8 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
>   
>   err_urb:
>   	kfree(priv->urbs);
> +	priv->urbs = NULL;
> +	priv->num_urbs = 0;
>   err_urbs:
>   	kfree(buffer);
>   	sgl_free(sgl);

thanks,
-- Shuah