The usbip DualSense died because its calibration report was one byte too long #287

Merged
enricobuehler merged 1 commits from worktree-usbip-dualsense-fix into main 2026-08-17 15:01:57 +00:00
Owner

PUNKTFUNK_DUALSENSE_USBIP=1 enumerated the pad and then lost it ~400 ms later, taking the controller with it — the usbip transport replaces uhid, so there was nothing to fall back to. Three sessions blamed the ISO stream, then the link speed, then actual_length. It was none of them.

Root cause

DS_FEATURE_CALIBRATION is 42 bytes. hid-playstation asks for 41 (DS_FEATURE_REPORT_CALIBRATION_SIZE). On a USB backend an over-long reply is not truncated — it is fatal to the transport:

size = urb->actual_length;                 /* 42, what we declared */
if (size > urb->transfer_buffer_length)    /* 42 > 41 */
        goto error;                        /* "should not happen, probably malicious packet" */
error:
        dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);   /* ret is still its initialiser, 0 */
        usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);

VDEV_EVENT_ERROR_TCP tears down the whole connection, not the one URB. So the observed recv xbuf, 0 was never EOF and never a byte-stream desync — it is the overrun branch, and that 0 is int ret = 0; untouched. The tell is that it prints with no %s: function-name prefix; every other goto error has already assigned a real recv count. Everything downstream — -71 on report 5, Failed to create dualsense, sendmsg failed -32, and our own "Remote closed the connection" — is consequence, in an order that made the teardown look causal.

The blob had been wrong since it was written, and a FIXME said so. It stayed invisible because every other backend truncates: hidraw for the uhid pad, hidclass on Windows. USB/IP is the first transport that checks.

Changes

Three, because any one alone leaves the same trap set:

  • Trim the constant to 41, plus a test pinning all three feature-report sizes (calibration 41 / pairing 20 / firmware 64) against the driver's own constants.
  • clamp_reply() in the vendored usbip-sim truncates every reply to the requested length, and clears any payload a handler returns on an OUT transfer — the kernel never reads one, so those bytes would misframe every PDU after them. The debug_assert! meant to catch exactly this was compiled out in release, which is why it survived. A handler bug now costs one wrong reply instead of the device.
  • DualSenseUsbip::open waits for a HID driver to actually bind before reporting success (3 s, PUNKTFUNK_DUALSENSE_USBIP_GRACE_MS; 0 disables). A vhci_hcd attach succeeds immediately and enumerates asynchronously, so bringup faults were being reported as working pads — which is why the existing uhid fallback never fired. It now returns Err and open_transport's fallback catches it.

Tooling

  • PUNKTFUNK_USBIP_TRACE=<prefix> dumps both socket directions verbatim plus a per-call index.
  • scripts/usbip-trace-analyse.py walks a capture as PDUs and names the first frame whose declared length disagrees with what the kernel will consume. Validated against a synthetic capture carrying this exact bug.
  • The handler's Err arm is no longer discarded. It was the only signal distinguishing "we dropped the connection" from "the kernel did", and both read identically in dmesg.

Verification (.21, CachyOS, kernel 7.1.8)

playstation 0003:054C:0CE6.0004: Registered DualSense controller hw_version=0x01000208 fw_version=0x01000036
Bus 009 Device 004: ID 054c:0ce6 Sony Corp. DualSense wireless controller (PS5)
 2 [Controller     ]: USB-Audio - DualSense Wireless Controller

The device stays enumerated and snd-usb-audio mints a real ALSA card — the one GE-Proton's snd_card_next scan needs.

Isochronous audio ran for the first time. It never could before; the device always died first. A 300 Hz tone into the coil pair reads back channel-exact off the endpoint, sustained for the whole run:

chunks=129 samples=147456 (~768.0ms of 4ch audio) peak_speaker=0.0000 peak_coils=0.5000

A 4957-frame capture analyses clean (4957 CMD_SUBMITs, 4956 replies; the one unanswered is the interrupt-IN URB in flight at detach). The health-check failure path was exercised separately with GRACE_MS=1: clear error, vhci port released, no leaked device.

cargo fmt --check, cargo clippy -p usbip-sim -p pf-inject -p punktfunk-host --all-targets -- -D warnings, and cargo test are all green.

Reproducing

sudo punktfunk-host pad-usbip-test --seconds N on any box with vhci_hcdno client, no game, no physical pad. It reproduced in 15 seconds on the first try. That devtest already existed.

Not covered

The actual game binding (Spider-Man + GE-Proton on .181). This clears the transport blocker; it does not prove wine's ContainerId walk now succeeds.

`PUNKTFUNK_DUALSENSE_USBIP=1` enumerated the pad and then lost it ~400 ms later, taking the controller with it — the usbip transport *replaces* uhid, so there was nothing to fall back to. Three sessions blamed the ISO stream, then the link speed, then `actual_length`. It was none of them. ## Root cause `DS_FEATURE_CALIBRATION` is **42 bytes**. `hid-playstation` asks for **41** (`DS_FEATURE_REPORT_CALIBRATION_SIZE`). On a USB backend an over-long reply is not truncated — it is fatal to the transport: ```c size = urb->actual_length; /* 42, what we declared */ if (size > urb->transfer_buffer_length) /* 42 > 41 */ goto error; /* "should not happen, probably malicious packet" */ error: dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret); /* ret is still its initialiser, 0 */ usbip_event_add(ud, VDEV_EVENT_ERROR_TCP); ``` `VDEV_EVENT_ERROR_TCP` tears down the **whole connection**, not the one URB. So the observed `recv xbuf, 0` was never EOF and never a byte-stream desync — it is the overrun branch, and that `0` is `int ret = 0;` untouched. The tell is that it prints with no `%s:` function-name prefix; every other `goto error` has already assigned a real recv count. Everything downstream — `-71` on report 5, `Failed to create dualsense`, `sendmsg failed -32`, and our own "Remote closed the connection" — is consequence, in an order that made the teardown look causal. The blob had been wrong since it was written, and a `FIXME` said so. It stayed invisible because every other backend truncates: hidraw for the uhid pad, `hidclass` on Windows. **USB/IP is the first transport that checks.** ## Changes Three, because any one alone leaves the same trap set: - **Trim the constant to 41**, plus a test pinning all three feature-report sizes (calibration 41 / pairing 20 / firmware 64) against the driver's own constants. - **`clamp_reply()` in the vendored `usbip-sim`** truncates every reply to the requested length, and clears any payload a handler returns on an OUT transfer — the kernel never reads one, so those bytes would misframe every PDU after them. The `debug_assert!` meant to catch exactly this was compiled out in release, which is why it survived. A handler bug now costs one wrong reply instead of the device. - **`DualSenseUsbip::open` waits for a HID driver to actually bind** before reporting success (3 s, `PUNKTFUNK_DUALSENSE_USBIP_GRACE_MS`; `0` disables). A `vhci_hcd` attach succeeds immediately and enumerates asynchronously, so bringup faults were being reported as working pads — which is why the existing uhid fallback never fired. It now returns `Err` and `open_transport`'s fallback catches it. ## Tooling - `PUNKTFUNK_USBIP_TRACE=<prefix>` dumps both socket directions verbatim plus a per-call index. - `scripts/usbip-trace-analyse.py` walks a capture as PDUs and names the first frame whose declared length disagrees with what the kernel will consume. Validated against a synthetic capture carrying this exact bug. - The handler's `Err` arm is no longer discarded. It was the only signal distinguishing "we dropped the connection" from "the kernel did", and both read identically in `dmesg`. ## Verification (.21, CachyOS, kernel 7.1.8) ``` playstation 0003:054C:0CE6.0004: Registered DualSense controller hw_version=0x01000208 fw_version=0x01000036 Bus 009 Device 004: ID 054c:0ce6 Sony Corp. DualSense wireless controller (PS5) 2 [Controller ]: USB-Audio - DualSense Wireless Controller ``` The device stays enumerated and `snd-usb-audio` mints a real ALSA card — the one GE-Proton's `snd_card_next` scan needs. **Isochronous audio ran for the first time.** It never could before; the device always died first. A 300 Hz tone into the coil pair reads back channel-exact off the endpoint, sustained for the whole run: ``` chunks=129 samples=147456 (~768.0ms of 4ch audio) peak_speaker=0.0000 peak_coils=0.5000 ``` A 4957-frame capture analyses clean (4957 CMD_SUBMITs, 4956 replies; the one unanswered is the interrupt-IN URB in flight at detach). The health-check failure path was exercised separately with `GRACE_MS=1`: clear error, vhci port released, no leaked device. `cargo fmt --check`, `cargo clippy -p usbip-sim -p pf-inject -p punktfunk-host --all-targets -- -D warnings`, and `cargo test` are all green. ## Reproducing `sudo punktfunk-host pad-usbip-test --seconds N` on any box with `vhci_hcd` — **no client, no game, no physical pad**. It reproduced in 15 seconds on the first try. That devtest already existed. ## Not covered The actual game binding (Spider-Man + GE-Proton on .181). This clears the transport blocker; it does not prove wine's ContainerId walk now succeeds.
enricobuehler added 1 commit 2026-08-17 15:01:17 +00:00
fix(pad): the usbip DualSense died because its calibration report was one byte too long
ci / bun-nix (pull_request) Successful in 30s
ci / docs-site (pull_request) Successful in 1m22s
ci / web (pull_request) Successful in 4m52s
android / android (pull_request) Successful in 4m57s
ci / rust-arm64 (pull_request) Successful in 6m55s
ci / rust (pull_request) Successful in 27m20s
8e8cc84d1a
`PUNKTFUNK_DUALSENSE_USBIP=1` enumerated the pad and then lost it ~400 ms later,
taking the controller with it (the usbip transport replaces uhid, so there was
nothing to fall back to). Three sessions blamed the ISO stream, the link speed and
`actual_length` in turn. It was none of them.

`DS_FEATURE_CALIBRATION` is 42 bytes. `hid-playstation` asks for 41
(`DS_FEATURE_REPORT_CALIBRATION_SIZE`), and on a USB backend an over-long reply is
not truncated, it is fatal to the transport:

    size = urb->actual_length;                 /* 42, what we declared */
    if (size > urb->transfer_buffer_length)    /* 42 > 41 */
            goto error;                        /* "probably malicious packet" */
    error:
            dev_err(&urb->dev->dev, "recv xbuf, %d\n", ret);   /* ret still 0 */
            usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);

`VDEV_EVENT_ERROR_TCP` tears down the whole connection, not the one URB — hence
`recv xbuf, 0` (that 0 is the untouched initialiser, not a byte count), then
-EPROTO on the calibration read, `Failed to create dualsense`, and the disconnect.
The dmesg order made the teardown look like the cause; it was the consequence.

The blob had been wrong since it was written, and a FIXME said so. It stayed
invisible because every other backend truncates: hidraw for the uhid pad, hidclass
on Windows. USB/IP is the first transport that checks.

Three changes, because one of them alone would leave the same trap set:

- Trim the constant to 41 and pin all three feature-report sizes in a test.
- Clamp every reply to the requested length in the transport (`clamp_reply`), and
  drop any payload a handler returns on an OUT transfer — the kernel never reads
  one, so those bytes would misframe every PDU after them. A handler bug now costs
  one wrong reply instead of the device.
- `DualSenseUsbip::open` waits for the kernel to actually bind a HID driver before
  reporting success. A `vhci_hcd` attach succeeds immediately and enumerates
  asynchronously, so bringup faults were being reported as working pads; now they
  return Err and the caller's existing uhid fallback catches them.

Also adds `PUNKTFUNK_USBIP_TRACE` (both socket directions to disk) and
`scripts/usbip-trace-analyse.py`, which walks a capture and names the first frame
whose declared length disagrees with what the kernel will consume. The handler's
Err arm is no longer discarded either — it was the only signal distinguishing "we
dropped the connection" from "the kernel did", and both read identically in dmesg.

Verified on .21 (CachyOS, kernel 7.1.8): `Registered DualSense controller
hw_version=0x01000208 fw_version=0x01000036`, the device stays enumerated, and
snd-usb-audio mints a real ALSA card. Audio over the isochronous endpoint now runs
for the first time — a 300 Hz tone on the coil pair reads back channel-exact
(peak_coils=0.5000, peak_speaker=0.0000) for the whole run. A 4957-frame capture
analyses clean.
enricobuehler merged commit 33538582e2 into main 2026-08-17 15:01:57 +00:00
enricobuehler deleted branch worktree-usbip-dualsense-fix 2026-08-17 15:02:02 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unom/punktfunk#287