[PATCH] usb: gadget: aspeed_udc: cancel wake work on device removal

[PATCH] usb: gadget: aspeed_udc: cancel wake work on device removal

[Hyunjung Ko]

ast_udc_wakeup() schedules udc->wake_work to drive remote wakeup from
process context. The work handler ast_udc_wake_work() dereferences udc and
takes udc->lock.

udc is allocated with devm_kzalloc(), so devres frees it after
ast_udc_remove() returns. Removal never cancels wake_work, so a work queued
by a remote-wakeup request just before unbind can run after udc has been
freed and dereference freed memory -- a use-after-free.

Cancel wake_work in ast_udc_remove(), after the gadget is unregistered so
no new work can be queued, before the devm-allocated udc is freed.

Found by static analysis; not runtime-reproduced (needs Aspeed ast_udc
gadget hardware issuing a remote wakeup that races unbind).

Fixes: 055276c13205 ("usb: gadget: add Aspeed ast2600 udc driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/usb/gadget/udc/aspeed_udc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
index 81fa495..20dcbd9 100644
--- a/drivers/usb/gadget/udc/aspeed_udc.c
+++ b/drivers/usb/gadget/udc/aspeed_udc.c
@@ -1460,6 +1460,14 @@ static void ast_udc_remove(struct platform_device *pdev)
 	struct ast_udc_dev *udc = platform_get_drvdata(pdev);

 	usb_del_gadget_udc(&udc->gadget);
+
+	/*
+	 * ast_udc_wakeup() schedules wake_work, whose handler dereferences udc.
+	 * The gadget is unregistered above, so no new work is queued; drain a
+	 * pending one before the devm-allocated udc is freed.
+	 */
+	cancel_work_sync(&udc->wake_work);
+
 	if (udc->driver) {
 		/*
 		 * This is broken as only some cleanup is skipped, *udev is
--
2.43.0

Re: [PATCH] usb: gadget: aspeed_udc: cancel wake work on device removal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: gadget: aspeed_udc: wake_work is unreachable in
  ast_udc_wakeup()
--

commit 59e63d484d3cece4c9053b24c7e169339b0f2919
Author: Hyunjung Ko <hj351016@gmail.com>

usb: gadget: aspeed_udc: cancel wake work on device removal

This commit cancels udc->wake_work in ast_udc_remove() after unregistering
the gadget.

> diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> index 10214232c08fa..2acf84c489909 100644
> --- a/drivers/usb/gadget/udc/aspeed_udc.c
> +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> @@ -1460,6 +1460,14 @@ static void ast_udc_remove(struct platform_device *pdev)
>  	struct ast_udc_dev *udc = platform_get_drvdata(pdev);
>  
>  	usb_del_gadget_udc(&udc->gadget);
> +
> +	/*
> +	 * ast_udc_wakeup() schedules wake_work, whose handler dereferences udc.
> +	 * The gadget is unregistered above, so no new work is queued; drain a
> +	 * pending one before the devm-allocated udc is freed.
> +	 */
> +	cancel_work_sync(&udc->wake_work);

[Severity: Medium]
Can udc->wake_work ever be scheduled in this driver?

ast_udc_wakeup_all() is only called from ast_udc_wakeup() after checking
udc->wakeup_en:

drivers/usb/gadget/udc/aspeed_udc.c:ast_udc_wakeup() {
	...
	if (!udc->wakeup_en) {
		UDC_DBG(udc, "Remote Wakeup is disabled\n");
		rc = -EINVAL;
		goto err;
	}

	UDC_DBG(udc, "Device initiated wakeup\n");
	ast_udc_wakeup_all(udc);
	...
}

Nothing in drivers/usb/gadget/udc/aspeed_udc.c ever sets udc->wakeup_en to
true: ast_udc_ep0_handle_setup() forwards USB_REQ_SET_FEATURE and
USB_REQ_CLEAR_FEATURE directly to req_driver without handling
USB_DEVICE_REMOTE_WAKEUP, and ast_udc_ops does not implement
.set_remote_wakeup.  As a result, ast_udc_wakeup() always returns -EINVAL,
udc->wake_work is unreachable dead code, and the use-after-free described
in the commit message cannot happen.

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

Re: [PATCH] usb: gadget: aspeed_udc: cancel wake work on device removal

[khj]

Thanks -- you're right, please disregard this patch.

udc->wakeup_en is never set to true in aspeed_udc: there is no
.set_remote_wakeup op, and ast_udc_ep0_handle_setup() forwards
SET/CLEAR_FEATURE to the gadget driver without handling
USB_DEVICE_REMOTE_WAKEUP. So ast_udc_wakeup() always returns -EINVAL before
ast_udc_wakeup_all(), wake_work is never scheduled, and the use-after-free I
described cannot occur. The cancel is harmless but unnecessary; drop it.

(The dead wake_work plumbing could be removed instead, but that's a separate
cleanup, not a fix.)

Thanks,
Hyunjung

2026년 11월 10일 (화) 오후 8:43, <sashiko-bot@kernel.org>님이 작성:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] usb: gadget: aspeed_udc: wake_work is unreachable in
>   ast_udc_wakeup()
> --
>
> commit 59e63d484d3cece4c9053b24c7e169339b0f2919
> Author: Hyunjung Ko <hj351016@gmail.com>
>
> usb: gadget: aspeed_udc: cancel wake work on device removal
>
> This commit cancels udc->wake_work in ast_udc_remove() after unregistering
> the gadget.
>
> > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
> > index 10214232c08fa..2acf84c489909 100644
> > --- a/drivers/usb/gadget/udc/aspeed_udc.c
> > +++ b/drivers/usb/gadget/udc/aspeed_udc.c
> > @@ -1460,6 +1460,14 @@ static void ast_udc_remove(struct platform_device *pdev)
> >       struct ast_udc_dev *udc = platform_get_drvdata(pdev);
> >
> >       usb_del_gadget_udc(&udc->gadget);
> > +
> > +     /*
> > +      * ast_udc_wakeup() schedules wake_work, whose handler dereferences udc.
> > +      * The gadget is unregistered above, so no new work is queued; drain a
> > +      * pending one before the devm-allocated udc is freed.
> > +      */
> > +     cancel_work_sync(&udc->wake_work);
>
> [Severity: Medium]
> Can udc->wake_work ever be scheduled in this driver?
>
> ast_udc_wakeup_all() is only called from ast_udc_wakeup() after checking
> udc->wakeup_en:
>
> drivers/usb/gadget/udc/aspeed_udc.c:ast_udc_wakeup() {
>         ...
>         if (!udc->wakeup_en) {
>                 UDC_DBG(udc, "Remote Wakeup is disabled\n");
>                 rc = -EINVAL;
>                 goto err;
>         }
>
>         UDC_DBG(udc, "Device initiated wakeup\n");
>         ast_udc_wakeup_all(udc);
>         ...
> }
>
> Nothing in drivers/usb/gadget/udc/aspeed_udc.c ever sets udc->wakeup_en to
> true: ast_udc_ep0_handle_setup() forwards USB_REQ_SET_FEATURE and
> USB_REQ_CLEAR_FEATURE directly to req_driver without handling
> USB_DEVICE_REMOTE_WAKEUP, and ast_udc_ops does not implement
> .set_remote_wakeup.  As a result, ast_udc_wakeup() always returns -EINVAL,
> udc->wake_work is unreachable dead code, and the use-after-free described
> in the commit message cannot happen.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261110113523.2517891-1-hj351016@gmail.com?part=1