[PATCH] usb: gadget: goku_udc: avoid double-free in error path

Guangshuo Li posted 1 patch 1 month, 2 weeks ago
drivers/usb/gadget/udc/goku_udc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH] usb: gadget: goku_udc: avoid double-free in error path
Posted by Guangshuo Li 1 month, 2 weeks ago
goku_probe() allocates struct goku_udc and passes &dev->gadget to
usb_add_gadget_udc_release() with gadget_release() as the release
callback. usb_add_gadget_udc_release() initializes the gadget device with
that release callback before calling usb_add_gadget().

If usb_add_gadget() fails, usb_add_gadget_udc_release() calls
usb_put_gadget(), which invokes gadget_release() and frees dev. The
current error path then falls through to kfree(dev), freeing the same
object again.

Set dev to NULL before jumping to the common error path so the explicit
kfree(dev) is skipped after ownership has already been dropped by the
gadget core.

This issue was found by a static analysis tool I am developing.

Fixes: 2a334cfaf393 ("usb: gadget: goku_udc: fix memory leak in goku_probe()")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/usb/gadget/udc/goku_udc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
index db42a5e3e805..46a7e0f6541e 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -1819,15 +1819,20 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	retval = usb_add_gadget_udc_release(&pdev->dev, &dev->gadget,
 			gadget_release);
-	if (retval)
+	if (retval) {
+		/*
+		 * usb_add_gadget_udc_release() calls the gadget release
+		 * function on failure, and gadget_release() frees dev.
+		 */
+		dev = NULL;
 		goto err;
+	}
 
 	return 0;
 
 err:
 	if (dev)
 		goku_remove (pdev);
-	/* gadget_release is not registered yet, kfree explicitly */
 	kfree(dev);
 	return retval;
 }
-- 
2.43.0
Re: [PATCH] usb: gadget: goku_udc: avoid double-free in error path
Posted by Guangshuo Li 1 month, 1 week ago
Hi,

Please ignore this patch.

On Fri, 1 May 2026 at 12:51, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> goku_probe() allocates struct goku_udc and passes &dev->gadget to
> usb_add_gadget_udc_release() with gadget_release() as the release
> callback. usb_add_gadget_udc_release() initializes the gadget device with
> that release callback before calling usb_add_gadget().
>
> If usb_add_gadget() fails, usb_add_gadget_udc_release() calls
> usb_put_gadget(), which invokes gadget_release() and frees dev. The
> current error path then falls through to kfree(dev), freeing the same
> object again.
>
> Set dev to NULL before jumping to the common error path so the explicit
> kfree(dev) is skipped after ownership has already been dropped by the
> gadget core.
>
> This issue was found by a static analysis tool I am developing.
>
> Fixes: 2a334cfaf393 ("usb: gadget: goku_udc: fix memory leak in goku_probe()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/usb/gadget/udc/goku_udc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index db42a5e3e805..46a7e0f6541e 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -1819,15 +1819,20 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
>         retval = usb_add_gadget_udc_release(&pdev->dev, &dev->gadget,
>                         gadget_release);
> -       if (retval)
> +       if (retval) {
> +               /*
> +                * usb_add_gadget_udc_release() calls the gadget release
> +                * function on failure, and gadget_release() frees dev.
> +                */
> +               dev = NULL;
>                 goto err;
> +       }
>
>         return 0;
>
>  err:
>         if (dev)
>                 goku_remove (pdev);
> -       /* gadget_release is not registered yet, kfree explicitly */
>         kfree(dev);
>         return retval;
>  }
> --
> 2.43.0
>

After taking a closer look, this appears to be a false positive. The
release callback passed to usb_add_gadget_udc_release() is installed on
the gadget device, so gadget_release() is called with &dev->gadget.dev,
not with the PCI device &pdev->dev.

goku_probe() only sets driver data on the PCI device via
pci_set_drvdata(pdev, dev). It does not set driver data on
&dev->gadget.dev. Therefore, dev_get_drvdata() in gadget_release() is
not proven to return the allocated struct goku_udc, and the reported
double-free is not valid.

Sorry for the noise.

Best regards,
Guangshuo