[PATCH v2] usbip: vudc: Prevent transfer timer rearm during teardown

[Myeonghun Pak]

Commit d96209626a29 ("usbip: vudc: Fix use after free bug in
vudc_remove due to race condition") deletes the transfer timer before
vudc_remove() frees the vudc, but the receive thread keeps running
until usb_del_gadget_udc().  v_kick_timer() calls mod_timer() for
CMD_SUBMIT and CMD_UNLINK even in VUDC_TR_STOPPED, so a packet after
timer_delete_sync() rearms the timer and v_timer() uses the freed
vudc.

Use timer_shutdown_sync(), which ignores later arming.  The transfer
state is freed with the timer.  Annotate v_start_timer() and
v_kick_timer() with __must_hold(&udc->lock).

A KASAN and DEBUG_OBJECTS x86_64 QEMU guest reproduced the free-active
timer warning and the use-after-free in v_timer().  The same harness
completed 4000 iterations with this change.

Fixes: d96209626a29 ("usbip: vudc: Fix use after free bug in vudc_remove due to race condition")
Link: https://lore.kernel.org/all/20230316180940.1601515-1-zyytlz.wz@xxxxxxx/
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
---
Changes in v2:
- Drop the lock-held comment and mark v_start_timer() and
  v_kick_timer() with __must_hold(&udc->lock), on the prototypes and
  the definitions (Greg Kroah-Hartman).

 drivers/usb/usbip/vudc.h          | 6 ++++--
 drivers/usb/usbip/vudc_transfer.c | 8 +++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/usbip/vudc.h b/drivers/usb/usbip/vudc.h
index 5ef0e7d9b23a..c129f4c56e8a 100644
--- a/drivers/usb/usbip/vudc.h
+++ b/drivers/usb/usbip/vudc.h
@@ -157,8 +157,10 @@ int v_rx_loop(void *data);
 /* vudc_transfer.c */
 
 void v_init_timer(struct vudc *udc);
-void v_start_timer(struct vudc *udc);
-void v_kick_timer(struct vudc *udc, unsigned long time);
+void v_start_timer(struct vudc *udc)
+	__must_hold(&udc->lock);
+void v_kick_timer(struct vudc *udc, unsigned long time)
+	__must_hold(&udc->lock);
 void v_stop_timer(struct vudc *udc);
 
 /* vudc_dev.c */
diff --git a/drivers/usb/usbip/vudc_transfer.c b/drivers/usb/usbip/vudc_transfer.c
index d4ce85c4c6a2..ba625622975e 100644
--- a/drivers/usb/usbip/vudc_transfer.c
+++ b/drivers/usb/usbip/vudc_transfer.c
@@ -441,8 +441,6 @@ static void v_timer(struct timer_list *t)
 	spin_unlock_irqrestore(&udc->lock, flags);
 }
 
-/* All timer functions are run with udc->lock held */
-
 void v_init_timer(struct vudc *udc)
 {
 	struct transfer_timer *t = &udc->tr_timer;
@@ -452,6 +450,7 @@ void v_init_timer(struct vudc *udc)
 }
 
 void v_start_timer(struct vudc *udc)
+	__must_hold(&udc->lock)
 {
 	struct transfer_timer *t = &udc->tr_timer;
 
@@ -470,6 +469,7 @@ void v_start_timer(struct vudc *udc)
 }
 
 void v_kick_timer(struct vudc *udc, unsigned long time)
+	__must_hold(&udc->lock)
 {
 	struct transfer_timer *t = &udc->tr_timer;
 
@@ -490,8 +490,6 @@ void v_stop_timer(struct vudc *udc)
 {
 	struct transfer_timer *t = &udc->tr_timer;
 
-	/* Delete the timer synchronously before teardown frees udc. */
 	dev_dbg(&udc->pdev->dev, "timer stop");
-	timer_delete_sync(&t->timer);
-	t->state = VUDC_TR_STOPPED;
+	timer_shutdown_sync(&t->timer);
 }

base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
-- 
2.53.0