[PATCH v2] usb: renesas_usbhs: fix use-after-free in ISR during device removal

Fan Wu posted 1 patch 1 month, 1 week ago
drivers/usb/renesas_usbhs/common.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH v2] usb: renesas_usbhs: fix use-after-free in ISR during device removal
Posted by Fan Wu 1 month, 1 week ago
In usbhs_remove(), the driver frees resources (including the pipe array)
while the interrupt handler (usbhs_interrupt) is still registered. If an
interrupt fires after usbhs_pipe_remove() but before the driver is fully
unbound, the ISR may access freed memory, causing a use-after-free.

Fix this by calling devm_free_irq() before freeing resources. This ensures
the interrupt handler is both disabled and synchronized (waits for any
running ISR to complete) before usbhs_pipe_remove() is called.

Fixes: f1407d5c6624 ("usb: renesas_usbhs: Add Renesas USBHS common code")
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---

v2:
  - Replace disable_irq() with devm_free_irq() as suggested by
    Alan Stern. While disable_irq() only masks the interrupt line,
    devm_free_irq() (via free_irq()) implicitly calls synchronize_irq()
    to wait for any in-progress handler to complete, and also ensures
    the interrupt is fully unregistered. This prevents both future
    interrupts and pending handlers from accessing freed resources.
  - Verified the fix in a QEMU environment with KASAN enabled.
    By injecting a delay in usbhs_remove() and using a high-frequency
    timer to simulate concurrent ISR execution, the original UAF
    report was reliably reproduced and is now confirmed to be
    eliminated with this patch.

 drivers/usb/renesas_usbhs/common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index cf4a0367d6d6..8c93bde4b816 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -815,6 +815,15 @@ static void usbhs_remove(struct platform_device *pdev)
 
 	usbhs_platform_call(priv, hardware_exit, pdev);
 	reset_control_assert(priv->rsts);
+
+	/*
+	 * Explicitly free the IRQ to ensure the interrupt handler is
+	 * disabled and synchronized before freeing resources.
+	 * devm_free_irq() calls free_irq() which waits for any running
+	 * ISR to complete, preventing UAF.
+	 */
+	devm_free_irq(&pdev->dev, priv->irq, priv);
+
 	usbhs_mod_remove(priv);
 	usbhs_fifo_remove(priv);
 	usbhs_pipe_remove(priv);
-- 
2.34.1