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