[PATCH] usbip: flush the event handler in usbip_stop_eh() to fix use-after-free

[PATCH] usbip: flush the event handler in usbip_stop_eh() to fix use-after-free

[João Moreira Fernandes]

usbip_stop_eh() is used by the stub, vhci and vudc sides to wait for the
event handler to finish with a usbip_device before the caller tears it
down.  It waits only on the event bitmask:

	wait_event_interruptible(ud->eh_waitq, !(ud->event & ~USBIP_EH_BYE));

but event_handler() clears those bits with unset_event() *before* its
final dereferences of ud:

	if (ud->event & USBIP_EH_SHUTDOWN) {
		ud->eh_ops.shutdown(ud);
		unset_event(ud, USBIP_EH_SHUTDOWN);   /* wait may now return */
	}
	...
	mutex_unlock(&ud->sysfs_lock);            /* still touches ud */
	wake_up(&ud->eh_waitq);                   /* still touches ud */

Once the last non-BYE bit is cleared the waiter can wake and the caller
proceeds to free ud.  For the stub side, stub_disconnect() ->
shutdown_busid() calls usbip_stop_eh() and then stub_device_free() frees
the stub_device that embeds ud, while event_handler() is still executing
mutex_unlock(&ud->sysfs_lock) and wake_up(&ud->eh_waitq), and may still
mutex_lock(&ud->sysfs_lock) on a second queued event for the same ud.
The result is a use-after-free in the usbip_event workqueue.

It is readily triggered by a reverse/exported-device teardown that
unbinds usbip-host while the importer is still attached: the socket EOF
queues SDEV_EVENT_ERROR_TCP (SHUTDOWN|RESET) from the stub_rx thread at
the same moment stub_disconnect() queues SDEV_EVENT_REMOVED
(SHUTDOWN|BYE), so the handler re-runs shutdown/reset on ud in the exact
window where it is being freed.

Reproduced under KASAN on v6.12.95 (usbip built as vhci_hcd importer and
usbip-host exporter on two separate VMs).  The freed object is the
stub_device, freed by the unbind write and then read by the event
workqueue:

  BUG: KASAN: slab-use-after-free in event_handler+0x2ba/0x3a0
  Read of size 8 at addr ffff888005cc6058 by task kworker/u8:5/63
  Workqueue: usbip_event event_handler
  Call Trace:
   event_handler+0x2ba/0x3a0
   process_one_work+0x5c2/0xfd0
   worker_thread+0x49d/0xb00
   kthread+0x246/0x300

  Allocated by task 1657:
   stub_probe+0xf0/0xb00
   usb_probe_device+0xaa/0x2e0
   bind_store+0xce/0x140
   vfs_write+0x87c/0xd70

  Freed by task 1660:
   kfree+0x1a4/0x3a0
   stub_disconnect+0x21e/0x340
   usb_unbind_device+0x6b/0x170
   device_release_driver_internal+0x384/0x550
   unbind_store+0xdb/0xf0
   vfs_write+0x87c/0xd70

The same run produced 37 KASAN splats against the one freed object,
reached through every place the handler still touches ud after clearing
the event bits: mutex_lock() on ud->sysfs_lock, the eh_waitq wake
(__wake_up_common / finish_wait / prepare_to_wait_event), and
stub_shutdown_connection() called from event_handler().

Waiting on the event bits cannot close the window, because the handler's
last accesses to ud are inherently after the bit is cleared.  Instead,
flush the event work in usbip_stop_eh() so the handler has fully returned
before the caller is allowed to free ud.  Every *_EVENT_REMOVED sets
USBIP_EH_BYE, after which usbip_event_add() no longer queues new work for
that ud, so flush_work() only has to wait out the in-flight run.  Guard
the flush with usbip_in_eh() so it is skipped in the (stub reset) path
that runs from the handler itself, and move the usbip_work declaration
above usbip_stop_eh() so it is in scope.  None of the three usbip_stop_eh()
callers hold ud->sysfs_lock, so flushing the worker (which takes it)
cannot deadlock.

With the patch applied, the same KASAN reproducer runs the teardown
suite repeatedly with zero KASAN reports.

Fixes: 363eaa3a450a ("usbip: synchronize event handler with sysfs code paths")
Cc: stable@vger.kernel.org
Signed-off-by: João Moreira Fernandes <joao.fernandes@ist.utl.pt>
Assisted-by: Claude:claude-opus-4-8
---
 drivers/usb/usbip/usbip_event.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/usbip_event.c b/drivers/usb/usbip/usbip_event.c
index 0e00c2d000f8..675fa2beeefd 100644
--- a/drivers/usb/usbip/usbip_event.c
+++ b/drivers/usb/usbip/usbip_event.c
@@ -97,6 +97,8 @@ static void event_handler(struct work_struct *work)
 	}
 }
 
+static DECLARE_WORK(usbip_work, event_handler);
+
 int usbip_start_eh(struct usbip_device *ud)
 {
 	init_waitqueue_head(&ud->eh_waitq);
@@ -116,6 +118,14 @@ void usbip_stop_eh(struct usbip_device *ud)
 		usbip_dbg_eh("usbip_eh waiting completion %lx\n", pending);
 
 	wait_event_interruptible(ud->eh_waitq, !(ud->event & ~USBIP_EH_BYE));
+
+	/*
+	 * The wait only tracks ud->event, which event_handler() clears before
+	 * its last dereferences of ud; flush so it cannot touch freed memory.
+	 */
+	if (!usbip_in_eh(current))
+		flush_work(&usbip_work);
+
 	usbip_dbg_eh("usbip_eh has stopped\n");
 }
 EXPORT_SYMBOL_GPL(usbip_stop_eh);
@@ -123,7 +133,6 @@ EXPORT_SYMBOL_GPL(usbip_stop_eh);
 #define WORK_QUEUE_NAME "usbip_event"
 
 static struct workqueue_struct *usbip_queue;
-static DECLARE_WORK(usbip_work, event_handler);
 
 int usbip_init_eh(void)
 {
-- 
2.54.0

Re: [PATCH] usbip: flush the event handler in usbip_stop_eh() to fix use-after-free

[Shuah Khan]

On 7/15/26 07:38, João Moreira Fernandes wrote:
> usbip_stop_eh() is used by the stub, vhci and vudc sides to wait for the
> event handler to finish with a usbip_device before the caller tears it
> down.  It waits only on the event bitmask:
> 
> 	wait_event_interruptible(ud->eh_waitq, !(ud->event & ~USBIP_EH_BYE));
> 
> but event_handler() clears those bits with unset_event() *before* its
> final dereferences of ud:
> 
> 	if (ud->event & USBIP_EH_SHUTDOWN) {
> 		ud->eh_ops.shutdown(ud);
> 		unset_event(ud, USBIP_EH_SHUTDOWN);   /* wait may now return */
> 	}
> 	...
> 	mutex_unlock(&ud->sysfs_lock);            /* still touches ud */
> 	wake_up(&ud->eh_waitq);                   /* still touches ud */
> 
> Once the last non-BYE bit is cleared the waiter can wake and the caller
> proceeds to free ud.  For the stub side, stub_disconnect() ->
> shutdown_busid() calls usbip_stop_eh() and then stub_device_free() frees
> the stub_device that embeds ud, while event_handler() is still executing
> mutex_unlock(&ud->sysfs_lock) and wake_up(&ud->eh_waitq), and may still
> mutex_lock(&ud->sysfs_lock) on a second queued event for the same ud.
> The result is a use-after-free in the usbip_event workqueue.
> 
> It is readily triggered by a reverse/exported-device teardown that
> unbinds usbip-host while the importer is still attached: the socket EOF
> queues SDEV_EVENT_ERROR_TCP (SHUTDOWN|RESET) from the stub_rx thread at
> the same moment stub_disconnect() queues SDEV_EVENT_REMOVED
> (SHUTDOWN|BYE), so the handler re-runs shutdown/reset on ud in the exact
> window where it is being freed.
> 
> Reproduced under KASAN on v6.12.95 (usbip built as vhci_hcd importer and
> usbip-host exporter on two separate VMs).  The freed object is the
> stub_device, freed by the unbind write and then read by the event
> workqueue:
> 
>    BUG: KASAN: slab-use-after-free in event_handler+0x2ba/0x3a0
>    Read of size 8 at addr ffff888005cc6058 by task kworker/u8:5/63
>    Workqueue: usbip_event event_handler
>    Call Trace:
>     event_handler+0x2ba/0x3a0
>     process_one_work+0x5c2/0xfd0
>     worker_thread+0x49d/0xb00
>     kthread+0x246/0x300
> 
>    Allocated by task 1657:
>     stub_probe+0xf0/0xb00
>     usb_probe_device+0xaa/0x2e0
>     bind_store+0xce/0x140
>     vfs_write+0x87c/0xd70
> 
>    Freed by task 1660:
>     kfree+0x1a4/0x3a0
>     stub_disconnect+0x21e/0x340
>     usb_unbind_device+0x6b/0x170
>     device_release_driver_internal+0x384/0x550
>     unbind_store+0xdb/0xf0
>     vfs_write+0x87c/0xd70
> 
> The same run produced 37 KASAN splats against the one freed object,
> reached through every place the handler still touches ud after clearing
> the event bits: mutex_lock() on ud->sysfs_lock, the eh_waitq wake
> (__wake_up_common / finish_wait / prepare_to_wait_event), and
> stub_shutdown_connection() called from event_handler().
> 
> Waiting on the event bits cannot close the window, because the handler's
> last accesses to ud are inherently after the bit is cleared.  Instead,
> flush the event work in usbip_stop_eh() so the handler has fully returned
> before the caller is allowed to free ud.  Every *_EVENT_REMOVED sets
> USBIP_EH_BYE, after which usbip_event_add() no longer queues new work for
> that ud, so flush_work() only has to wait out the in-flight run.  Guard
> the flush with usbip_in_eh() so it is skipped in the (stub reset) path
> that runs from the handler itself, and move the usbip_work declaration
> above usbip_stop_eh() so it is in scope.  None of the three usbip_stop_eh()
> callers hold ud->sysfs_lock, so flushing the worker (which takes it)
> cannot deadlock.
> 
> With the patch applied, the same KASAN reproducer runs the teardown
> suite repeatedly with zero KASAN reports.

Hmm. Did you test this change with usbip host and client? What kind of
testing was done on this change?

thanks,
-- Shuah