Re: [PATCH] usbip: vhci_hcd: time out unanswered unlink requests

[Shuah Khan]

On 10/6/26 06:56, Hongyu Xie wrote:
> When the connection to the peer is alive, vhci_urb_dequeue() only queues
> a CMD_UNLINK PDU and relies on the peer to send RET_UNLINK (or
> RET_SUBMIT) before the unlinked URB is given back.  A peer that keeps
> the socket open but never answers therefore makes usb_kill_urb() wait
> forever.  The waiter is typically the hub work item, which runs with
> the shared address0_mutex held, so every later task that needs that
> mutex or the device lock chain - a second attach, usbfs open/release/
> ioctl, udev - piles up behind it and the whole USB stack wedges.
>
> syzbot reports this shape in three long-lived bugs:
>    INFO: task hung in hub_port_init
>    INFO: task hung in hub_event
>    INFO: task hung in usbdev_release
>
> Give every queued unlink request a deadline of its own and arm a
> per-device watchdog; when the watchdog fires, it tears the connection
> down through the existing error path (VDEV_EVENT_ERROR_TCP) if any
> request is still unanswered past its own deadline.  The teardown
> already knows how to give back every pending URB with -ENODEV, and a
> RET_UNLINK that arrives late (or never) after that is handled by the
> existing "already given back" paths.
>
> The watchdog is cancelled whenever no request is outstanding.  Queueing
> a request re-arms it to that request's deadline, so the callback does
> not trust the timer's expiry: it re-checks the outstanding requests
> against their own deadlines under priv_lock and becomes a no-op when
> none has run out.  A peer that answers promptly is therefore unaffected.
> Controller removal quiesces the device first - the event handler
> stops the per-device rx/tx threads and drains the pending events -
> and only then calls timer_shutdown_sync(), which also waits for a
> watchdog callback that is already running, so the callback cannot
> touch the controller after it is freed.
>
> A similar fix was proposed as an RFC by the syzbot assisted-fix
> pipeline; this version arms a single per-device watchdog instead of a
> timer per unlink request.

Did you test this change with a usbip_host and client? How does this
reproduce in a regular setup?

thanks,
-- Shuah