[PATCH] usb: musb: fix double put of gadget device on setup failure

[PATCH] usb: musb: fix double put of gadget device on setup failure

[Anfernee Cao]

musb_gadget_setup() calls usb_add_gadget_udc() and, when that fails,
calls device_unregister(&musb->g.dev).  But usb_add_gadget_udc() already
releases the gadget on failure: usb_add_gadget_udc_release() calls
usb_put_gadget(), i.e. put_device(&gadget->dev), in its error path.

The device model documents the same rule in the kerneldoc of
device_register() (drivers/base/core.c):

    NOTE: _Never_ directly free @dev after calling this function, even
    if it returned an error! Always use put_device() to give up the
    reference initialized in this function instead.

The extra device_unregister() therefore runs device_del() on a device
that was never added (or was already deleted) and drops the reference a
second time, causing a refcount underflow.

Drop the redundant cleanup and just return the error; the UDC core has
already released the gadget.

This issue was found by manual code inspection.

Fixes: 3301c215a2bb ("USB: UDC: Expand device model API interface")
Cc: stable@vger.kernel.org # v5.10+
Signed-off-by: Anfernee Cao <anferneemelokc5@gmail.com>
---
 drivers/usb/musb/musb_gadget.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 016d3f3fc1e0..4bc4270088bc 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1794,13 +1794,9 @@ int musb_gadget_setup(struct musb *musb)
 
 	status = usb_add_gadget_udc(musb->controller, &musb->g);
 	if (status)
-		goto err;
+		return status;
 
 	return 0;
-err:
-	musb->g.dev.parent = NULL;
-	device_unregister(&musb->g.dev);
-	return status;
 }
 
 void musb_gadget_cleanup(struct musb *musb)
-- 
2.54.0.windows.1

Re: [PATCH] usb: musb: fix double put of gadget device on setup failure

[sashiko-bot]

> musb_gadget_setup() calls usb_add_gadget_udc() and, when that fails,
> calls device_unregister(&musb->g.dev).  But usb_add_gadget_udc() already
> releases the gadget on failure: usb_add_gadget_udc_release() calls
> usb_put_gadget(), i.e. put_device(&gadget->dev), in its error path.
> 
> The device model documents the same rule in the kerneldoc of
> device_register() (drivers/base/core.c):
> 
>     NOTE: _Never_ directly free @dev after calling this function, even
>     if it returned an error! Always use put_device() to give up the
>     reference initialized in this function instead.
> 
> The extra device_unregister() therefore runs device_del() on a device
> that was never added (or was already deleted) and drops the reference a
> second time, causing a refcount underflow.
> 
> Drop the redundant cleanup and just return the error; the UDC core has
> already released the gadget.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 3301c215a2bb ("USB: UDC: Expand device model API interface")
> Cc: stable@vger.kernel.org # v5.10+
> Signed-off-by: Anfernee Cao <anferneemelokc5@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179250629747.20696.17364135141613638741@gmail.com?part=1