[PATCH] usb: gadget: m66592-udc: Free the IRQ before unmapping registers

[PATCH] usb: gadget: m66592-udc: Free the IRQ before unmapping registers

[Myeonghun Pak]

m66592_remove() unmaps the controller registers before removing its shared
IRQ handler. Another device on the same IRQ can invoke m66592_irq() in
that interval, and the handler reads and writes registers even when the
controller has no enabled interrupt pending. CONFIG_DEBUG_SHIRQ can also
invoke the handler from free_irq() after the mapping has gone away.

Shut down the sampling timer and mask the controller interrupt sources
before unregistering the gadget. The timer can enable interrupts and
invoke the gadget driver's disconnect callback, while gadget teardown
clears the driver pointer and disables the controller's internal clocks.
Then free the IRQ before unmapping the registers so the handler finishes
while its MMIO mapping and private data are still valid.

This issue was identified during our ongoing static-analysis research
while reviewing kernel code.

Fixes: 4cf2503c6801 ("USB: m66592-udc: peripheral controller driver for M66592")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/usb/gadget/udc/m66592-udc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/m66592-udc.c b/drivers/usb/gadget/udc/m66592-udc.c
index d77c11c4eb38..773cd00fbf1d 100644
--- a/drivers/usb/gadget/udc/m66592-udc.c
+++ b/drivers/usb/gadget/udc/m66592-udc.c
@@ -1515,12 +1515,16 @@ static const struct usb_gadget_ops m66592_gadget_ops = {
 static void m66592_remove(struct platform_device *pdev)
 {
 	struct m66592		*m66592 = platform_get_drvdata(pdev);
-
-	usb_del_gadget_udc(&m66592->gadget);
+	unsigned long flags;
 
 	timer_shutdown_sync(&m66592->timer);
-	iounmap(m66592->reg);
+	spin_lock_irqsave(&m66592->lock, flags);
+	m66592_write(m66592, 0, M66592_INTENB0);
+	spin_unlock_irqrestore(&m66592->lock, flags);
+
+	usb_del_gadget_udc(&m66592->gadget);
 	free_irq(platform_get_irq(pdev, 0), m66592);
+	iounmap(m66592->reg);
 	m66592_free_request(&m66592->ep[0].ep, m66592->ep0_req);
 	if (m66592->pdata->on_chip) {
 		clk_disable(m66592->clk);
-- 
2.53.0

Re: [PATCH] usb: gadget: m66592-udc: Free the IRQ before unmapping registers

[Aldo Ariel Panzardo]

On Fri, Oct 02, 2026 at 11:49:08PM -0400, Myeonghun Pak wrote:
> m66592_remove() unmaps the controller registers before removing its shared
> IRQ handler. Another device on the same IRQ can invoke m66592_irq() in
> that interval, and the handler reads and writes registers even when the
> controller has no enabled interrupt pending. CONFIG_DEBUG_SHIRQ can also
> invoke the handler from free_irq() after the mapping has gone away.

Confirmed against mainline: m66592_remove() calls iounmap(m66592->reg)
before free_irq(), and m66592_irq() takes m66592->lock and does register
I/O (m66592_read()/m66592_write() of INTSTS0, INTENB0, CFIFOSEL, ...)
unconditionally. request_irq() uses IRQF_SHARED, so a co-device's
interrupt -- or CONFIG_DEBUG_SHIRQ at free_irq() time -- can run the
handler against the already-unmapped mapping.

Masking the controller's interrupt sources (INTENB0 = 0) under the lock
and freeing the IRQ before iounmap() closes the window: after free_irq()
returns the handler can no longer run, and every remaining access still
happens while the MMIO mapping and private data are valid.

Reviewed-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>