Re: [PATCH] usbip: vudc: reject submits after endpoint state changes

[성병찬 <tjdqudcks0424@naver.com>]

Hi Shuah,

Thank you for asking. I need to correct one part of my original commit
message before explaining the reproduction.

The sentence attributing the race-specific failure to the NULL urb->dev
diagnostic was inaccurate. That diagnostic is the separately confirmed parent
VUDC receive-error issue. In the endpoint-state race described by this patch,
the observed crash occurred in the v_recv_cmd_submit() path after the endpoint
descriptor had been cleared.

I reproduced the race in a network-isolated x86_64 QEMU guest at
ff47652a4b66c067c765a7ad464d930b5a9367cc with KASAN enabled. The PID-1
fixture creates a real configfs UAC2 gadget and attaches a local socketpair to
the production VUDC usbip_sockfd interface. It does not call an internal VUDC
function directly.

For the stale-submit case, the fixture performs the following sequence:

  1. Enable and enumerate an isochronous OUT endpoint.
  2. Send a SET_INTERFACE request selecting altsetting 0, immediately followed
     by the header of an ISO OUT CMD_SUBMIT, in one socket write.
  3. Let the receive thread consume the ISO submit header and begin waiting for
     its body.
  4. While it is waiting, let the VUDC transfer timer complete SET_INTERFACE
     and disable the endpoint.
  5. Send the delayed ISO body after the endpoint has been disabled.
  6. Re-enable the same logical endpoint and check the stale completion and a
     following EP0 operation.

There is no kernel-side delay or test hook. The receive thread is held in the
normal payload receive operation while the normal VUDC timer processes the
endpoint transition.

The baseline race is timing-sensitive. Five of six baseline stale runs
recovered without a sanitizer report, but one run produced a KASAN NULL pointer
dereference in the inlined v_recv_cmd_submit() path. This is consistent with
the endpoint descriptor becoming NULL while the unlocked code calculates the
endpoint maximum packet size. The other runs still demonstrated that the
submit could cross the endpoint state transition.

The generation check is intended to close both parts of that window:

  - endpoint type and maximum packet size are snapshotted while udc->lock is
    held, so parsing does not dereference a mutable descriptor after dropping
    the lock;
  - enable and disable each increment the endpoint generation while holding
    the same lock;
  - immediately before urb_queue insertion, the receiver reacquires
    udc->lock and requires both a present descriptor and the same generation.

Checking only the endpoint pointer would not be sufficient because disable and
re-enable reuse the same struct vep. The generation check distinguishes the
new endpoint state from the state used when the submit was parsed. -ESHUTDOWN
is returned because the endpoint instance for which the submit was received no
longer exists.

Using the exact code diff that I posted, I ran three fresh boots each for
normal operation, serialized disable, and the stale-generation sequence. The
three stale runs sent the body after disable, were rejected with -ESHUTDOWN,
remained free of KASAN/oops/panic reports, and completed a following EP0
request after bounded transport recovery.

I will correct the failure description in a v2 commit message. The code diff
will remain unchanged.

Thanks,
Sung byeongchan