[PATCH v2] can: peak_usb: pcan_usb: shut down restart timer on free to avoid use-after-free

[PATCH v2] can: peak_usb: pcan_usb: shut down restart timer on free to avoid use-after-free

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

pcan_usb arms pdev->restart_timer from pcan_usb_restart_pending(), the
completion of the restart-command URB submitted by pcan_usb_restart_async()
on bus-off auto-restart or a manual CAN_MODE_START. That URB is not
anchored to dev->tx_submitted/rx_submitted, so peak_usb_unlink_all_urbs()
does not kill it, and the completion can run -- re-arming the timer via
mod_timer() -- after the interface has been stopped.

pcan_usb provides no teardown for the timer, so on disconnect (or a probe
error path reaching the adap_dev_free label) peak_usb_disconnect() calls
free_candev() with the timer still live. A pending or late-armed timer
then runs pcan_usb_restart() against the freed struct pcan_usb ->
use-after-free.

Add a pcan_usb dev_free callback that calls timer_shutdown_sync() on
restart_timer. peak_usb_disconnect() invokes dev_free() after
unregister_candev() and before free_candev(), by which point the
endpoint URBs have been flushed by usb_disable_interface(). Unlike
timer_delete_sync(), timer_shutdown_sync() also prevents a late URB
completion from re-arming the timer, closing the window a plain delete
would leave open.

Not reproduced on hardware (no PEAK PCAN-USB adapter); found by review of
the restart-timer teardown path.

Fixes: 46be265d3388 ("can: usb: PEAK-System Technik PCAN-USB specific part")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 8fd058c32856..113b2e745331 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -882,6 +882,19 @@ static int pcan_usb_start(struct peak_usb_device *dev)
 	return pcan_usb_set_ext_vcc(dev, 0);
 }
 
+static void pcan_usb_free(struct peak_usb_device *dev)
+{
+	struct pcan_usb *pdev = container_of(dev, struct pcan_usb, dev);
+
+	/*
+	 * The restart timer is armed from the restart-command URB completion
+	 * (pcan_usb_restart_pending()), which is not anchored and so outlives
+	 * ndo_stop/unlink_all_urbs. Shut it down -- not just delete it -- so a
+	 * late completion cannot re-arm it after free_candev().
+	 */
+	timer_shutdown_sync(&pdev->restart_timer);
+}
+
 static int pcan_usb_init(struct peak_usb_device *dev)
 {
 	struct pcan_usb *pdev = container_of(dev, struct pcan_usb, dev);
@@ -1051,6 +1064,7 @@ const struct peak_usb_adapter pcan_usb = {
 	.dev_decode_buf = pcan_usb_decode_buf,
 	.dev_encode_msg = pcan_usb_encode_msg,
 	.dev_start = pcan_usb_start,
+	.dev_free = pcan_usb_free,
 	.dev_restart_async = pcan_usb_restart_async,
 	.do_get_berr_counter = pcan_usb_get_berr_counter,
 };
-- 
2.43.0