[PATCH] usb: gadget: lpc32xx_udc: cancel pullup work on remove

[PATCH] usb: gadget: lpc32xx_udc: cancel pullup work on remove

[Hongyan Xu]

Several device interrupts can queue pullup_job, whose callback accesses
the UDC and its I2C transceiver. The IRQs are device-managed, so they
remain registered throughout the driver remove callback, and no path
drains work queued before the resources are released.

Disable and synchronize all publishing IRQs, then cancel pullup_job
before putting the I2C device and releasing the UDC resources.

Fixes: 24a28e428351 ("USB: gadget driver for LPC32xx")
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/usb/gadget/udc/lpc32xx_udc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 044c31869cfb..0a784c9252c5 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -3184,9 +3184,15 @@ static void lpc32xx_udc_remove(struct platform_device *pdev)
 		return;
 	}
 
+	disable_irq(udc->udp_irq[IRQ_USB_LP]);
+	disable_irq(udc->udp_irq[IRQ_USB_HP]);
+	disable_irq(udc->udp_irq[IRQ_USB_DEVDMA]);
+	disable_irq(udc->udp_irq[IRQ_USB_ATX]);
+
 	udc_clk_set(udc, 1);
 	udc_disable(udc);
 	pullup(udc, 0);
+	cancel_work_sync(&udc->pullup_job);
 
 	device_init_wakeup(&pdev->dev, 0);
 	remove_debug_file(udc);
-- 
2.50.1.windows.1

Re: [PATCH] usb: gadget: lpc32xx_udc: cancel pullup work on remove

[Aldo Ariel Panzardo]

On Mon, Sep 28, 2026 at 09:41:58PM +0800, Hongyan Xu wrote:
> Several device interrupts can queue pullup_job, whose callback accesses
> the UDC and its I2C transceiver. The IRQs are device-managed, so they
> remain registered throughout the driver remove callback, and no path
> drains work queued before the resources are released.

The use-after-free is real and the shape of the fix is right. The suspend
and resume interrupt paths both schedule_work(&udc->pullup_job), and the
device-managed IRQs stay live until after remove() returns, so disabling
the four IRQs before teardown and draining the work before
put_device(&udc->isp1301_i2c_client->dev) does close the window.

One side effect is worth reconsidering, though. remove() calls
pullup(udc, 0) right before the cancel. For a gadget that was enumerated
that reaches isp1301_pullup_enable(udc, 0, 0), which only defers the
actual transceiver write via schedule_work(); cancel_work_sync() then
discards that work before it runs, since the IRQs are already disabled and
nothing re-queues it. udc_disable() touches only the UDC core, not the
ISP1301, so the D+ pull-up is left asserted -- the explicit pullup(udc, 0)
ends up doing nothing on the common path and the device does not
electrically disconnect on removal.

Draining with flush_work() instead (or clearing the pull-up synchronously
before the cancel) would both close the use-after-free and keep the
disconnect: a job still queued runs with udc->pullup already 0, so it
performs the clear while the i2c client is still valid.

Thanks,
Aldo