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

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

Hi Shuah,

Yes, I reproduced it. Sorry, the commit message did not describe the
test harness and the synchronization clearly enough.

I tested it in a network-isolated QEMU guest running a KASAN kernel at
ff47652a4b66c067c765a7ad464d930b5a9367cc. The guest fixture ran as
PID 1, configured a UAC2 configfs gadget on the production VUDC, and
attached a local socketpair through:

  /sys/devices/platform/usbip-vudc.0/usbip_sockfd

The fixture sent ordinary USB/IP PDUs through that socket. It did not
call v_recv_cmd_submit() or any other internal VUDC function directly,
and I did not add a delay or test hook to the kernel.

The stale-endpoint test used the following sequence:

  1. Enumerate the gadget and select the ISO OUT endpoint
     (address 0x01, interface 1, alternate setting 1 in these runs).

  2. In one socket write, send two USBIP_CMD_SUBMIT headers:
     an EP0 SET_INTERFACE request selecting alternate setting 0,
     followed by a 64-byte ISO OUT submit header with one ISO packet.

  3. Do not send the ISO payload immediately. The receive thread can
     consume the ISO header and perform the endpoint lookup, then block
     waiting for the payload while the VUDC transfer timer processes
     the preceding SET_INTERFACE request and disables the endpoint.

  4. Wait 20 ms, then send the 64-byte payload and its ISO packet
     descriptor.

  5. Queue SET_INTERFACE back to alternate setting 1. This also tests
     the disable/re-enable case where the same struct vep is reused.

I ran the unmodified baseline case in six independent fresh boots. All
six runs reached the intended ordering and logged:

  P53V2_STALE header_seq=5 phase=LOOKUP_THEN_BODY_DELAY
  P53V2_STALE body_result=SENT_AFTER_DISABLE length=64

One of the six runs then faulted with:

  Oops: general protection fault, probably for non-canonical address
  KASAN: null-ptr-deref in range [0x0-0x7]
  RIP: v_rx_loop+0x599/0xb7f
  Kernel panic - not syncing: Fatal exception

v_recv_cmd_submit() was inlined into v_rx_loop in that build. The fault
was in the endpoint descriptor access after vep_disable() had cleared
ep->desc. The other five runs crossed the same endpoint transition but
did not happen to fault, so the crash itself is timing-sensitive.

For the patched kernel I ran normal operation, serialized disable, and
the exact disable/re-enable stale-submit case in three independent
fresh boots each. The selected stale runs were runs 2, 4, and 5. Each
one again logged SENT_AFTER_DISABLE, but the receive thread returned
-ESHUTDOWN without a KASAN report. After attaching a new local transport
to the same VUDC, a following EP0 request completed with status 0.

The fix covers two related parts of the window.

First, endpoint type and maximum packet size are copied while
udc->lock still protects ep->desc. This removes the unlocked parsing
reads of mutable descriptor state.

Second, vep_enable() and vep_disable() both increment the endpoint
generation while holding udc->lock. Immediately before adding the URB
to urb_queue, v_recv_cmd_submit() takes the same lock and checks both
descriptor presence and generation equality. This rejects a request
that crossed either disable or disable/re-enable, including reuse of
the same struct vep.

That is why I used a generation check in addition to checking ep->desc.
A descriptor-presence check alone would not distinguish the newly
enabled endpoint from the endpoint state used during the original
lookup.

I can also send the standalone guest fixture if that would be useful
for reviewing or reproducing the test.

Thanks,
Sung