[PATCH] usb: sl811-hcd: delete the driver timer on stop to avoid use-after-free

[PATCH] usb: sl811-hcd: delete the driver timer on stop to avoid use-after-free

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

sl811h_hub_control() arms sl811->timer (sl811h_timer) with mod_timer()
for port reset/resume timing; the timer callback dereferences the
hcd-private struct sl811 (sl811->ctrl1, sl811->lock, sl811->port1, and
register I/O via sl811).

sl811h_stop() deletes only the USB core's root-hub timer
(hcd->rh_timer); it never deletes sl811->timer, and nothing else does
either. struct sl811 lives in the HCD private area and is freed by
usb_put_hcd() after sl811h_remove() calls usb_remove_hcd(). If
sl811->timer is still pending when the controller is stopped and the HCD
is freed, sl811h_timer() runs against the freed struct sl811
(use-after-free).

Delete sl811->timer at the start of sl811h_stop(), next to the existing
hcd->rh_timer teardown and before port_power(sl811, 0) halts the
controller, so any pending sl811h_timer() is drained before the hardware
is halted and cannot re-enable it afterwards. usb_remove_hcd() has
already stopped root-hub activity before sl811h_stop() runs, so the timer
cannot be re-armed, and sl811h_timer() does not re-arm itself, so a
single timer_delete_sync() drains it.

Found by static analysis and code review; not tested on real hardware
(needs an SL811HS USB host controller).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
v2: delete sl811->timer BEFORE port_power(sl811, 0) (top of sl811h_stop(),
    next to hcd->rh_timer) instead of after it, so a pending sl811h_timer()
    is drained before the controller is halted and cannot re-enable it in
    the window after the lock is dropped. Corrected the log: only
    sl811h_hub_control() arms sl811->timer (not the transfer scheduler).
    Both per Sashiko AI review.

 drivers/usb/host/sl811-hcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index b044977..b667732 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1516,6 +1516,7 @@ sl811h_stop(struct usb_hcd *hcd)
 	unsigned long	flags;
 
 	timer_delete_sync(&hcd->rh_timer);
+	timer_delete_sync(&sl811->timer);
 
 	spin_lock_irqsave(&sl811->lock, flags);
 	port_power(sl811, 0);
-- 
2.43.0

[PATCH] usb: sl811-hcd: delete the driver timer on stop to avoid use-after-free

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

sl811h_hub_control() and the transfer scheduler arm sl811->timer
(sl811h_timer) with mod_timer(); the timer callback dereferences the
hcd-private struct sl811 (sl811->ctrl1, sl811->lock, sl811->port1, and
register I/O via sl811).

sl811h_stop() deletes only the USB core's root-hub timer
(hcd->rh_timer); it never deletes sl811->timer, and nothing else does
either. struct sl811 lives in the HCD private area and is freed by
usb_put_hcd() after sl811h_remove() calls usb_remove_hcd(). If
sl811->timer is still pending when the controller is stopped and the
HCD is freed, sl811h_timer() runs against the freed struct sl811
(use-after-free).

Delete sl811->timer in sl811h_stop(), after the port is powered down so
it cannot be re-armed, mirroring the existing hcd->rh_timer teardown.
sl811h_timer() does not re-arm itself, so a single timer_delete_sync()
drains it.

Found by static analysis and code review; not tested on real hardware
(needs an SL811HS USB host controller).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/usb/host/sl811-hcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index b044977..53a0bc8 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1520,6 +1520,8 @@ sl811h_stop(struct usb_hcd *hcd)
 	spin_lock_irqsave(&sl811->lock, flags);
 	port_power(sl811, 0);
 	spin_unlock_irqrestore(&sl811->lock, flags);
+
+	timer_delete_sync(&sl811->timer);
 }
 
 static int
-- 
2.43.0