fix(usbip): an OUT reply said 0 bytes accepted, so every hidraw write on the pad "failed"
GE-Proton's `hidraw_enable_dualsense_usb_haptics` never enabled the DualSense's USB haptics
mode against our usbip pad — `err:hid:hidraw_device_set_output_report id 2 write failed
error: 2 No such file or directory`, then feature report 0x08 retried forever with
EINVAL/EAGAIN. Adaptive triggers, voice-coil haptics and the speaker are all gated behind
that one enable, so nothing downstream could ever show a result. Five theories were ruled
out by log inspection; the sixth was measured on the live pad on .41 today:
write(hidraw, output 0x02, 48 B) -> 0
ioctl(HIDIOCSFEATURE 0x08, 48 B) -> 0
ioctl(HIDIOCGFEATURE 0x05, 41 B) -> 41
The vendored simulator answered every non-isochronous OUT URB through the IN constructor
with an empty buffer, i.e. `actual_length = 0` — and a debug_assert pinned that as the
rule ("OUT nothing"). vhci_hcd copies the field into `urb->actual_length` verbatim
(`usbip_pack_pdu(pdu, urb, USBIP_RET_SUBMIT, 0)` in `vhci_recv_ret_submit()`) and has no
other source for it, so `usbhid_output_report()` returned 0 as `write()`'s byte count and
`usb_control_msg()` returned 0 for the SET_REPORT data stage. winebus checks `count > 0`,
takes 0 as failure, and prints the thread's *stale* errno — the ENOENT/EINVAL/EAGAIN in the
log were never kernel verdicts. The earlier `/tmp/hidwrite.py` "150/150 ok" was the same
illusion: `os.write` returning 0 does not raise. A real usbip stub reports the real URB's
`actual_length`, which on OUT is the bytes sent.
Fix: `UsbIpResponse::usbip_ret_submit_out_success(header, accepted)` acknowledges the bytes
taken (`data.len()`, which `read_from_socket` sized from `transfer_buffer_length`) with no
payload back; the handler uses it for OUT; the assertion now pins "OUT carries no buffer",
not "OUT claims 0". Two wire-byte tests pin both directions. The Steam Controller 2 shares
this handler, so its OUT writes were being reported as 0 bytes too.
`scripts/usbip-trace-analyse.py` flagged ANY nonzero OUT actual_length as a desync — the
wrong rule (its own framing never reads a payload back on OUT) and one that would have hid
this bug and flagged the fix. It now flags an OUT reply claiming more than it was sent, or
0 against a non-empty write.
This commit is contained in:
@@ -30,6 +30,12 @@ Modifications by the punktfunk project:
|
||||
table, which stalls any ISO endpoint. ISO completion is paced by
|
||||
bInterval × packet count, because for an audio endpoint the completion rate
|
||||
*is* the device's sample clock.
|
||||
- Non-isochronous **OUT** URBs are acknowledged with `actual_length` = the
|
||||
bytes accepted (`UsbIpResponse::usbip_ret_submit_out_success`). Upstream
|
||||
answered them through the IN constructor with an empty buffer, i.e.
|
||||
`actual_length = 0`; `vhci_hcd` copies that field into the URB verbatim, so
|
||||
every synchronous writer (`write()` on hidraw, `HIDIOCSFEATURE`) was told it
|
||||
transferred 0 bytes and treated the write as failed.
|
||||
|
||||
Only the USB/IP server *simulation* path is retained: the device model, the
|
||||
USB/IP wire protocol, and the `UsbInterfaceHandler` trait. The original MIT
|
||||
|
||||
+20
-4
@@ -145,9 +145,11 @@ async fn handle_iso_submit(
|
||||
/// rather than one URB failing. Real hardware truncates here, so we do too: a handler bug then
|
||||
/// costs one wrong reply instead of the pad.
|
||||
///
|
||||
/// An OUT transfer returns nothing at all. `usbip_recv_xbuff()` returns early for `usb_pipeout`,
|
||||
/// so any payload appended to an OUT reply is bytes the kernel never reads — and every byte after
|
||||
/// it in the stream is then misframed.
|
||||
/// An OUT transfer returns no payload at all. `usbip_recv_xbuff()` returns early for
|
||||
/// `usb_pipeout`, so any payload appended to an OUT reply is bytes the kernel never reads — and
|
||||
/// every byte after it in the stream is then misframed. (Its `actual_length` is another matter:
|
||||
/// that must still count the bytes accepted, see
|
||||
/// [`UsbIpResponse::usbip_ret_submit_out_success`].)
|
||||
///
|
||||
/// Field-diagnosed 2026-08-17: a 42-byte DualSense calibration report answering a 41-byte request
|
||||
/// killed `hid-playstation`'s probe with `-EPROTO` and took the controller with it. Every backend
|
||||
@@ -303,10 +305,24 @@ pub async fn handler<T: AsyncReadExt + AsyncWriteExt + Unpin>(
|
||||
}
|
||||
if out {
|
||||
trace!("<-Wrote {}", data.len());
|
||||
// Acknowledge the bytes we took, not the (empty) reply:
|
||||
// `actual_length` is what `write()` on the device node
|
||||
// returns to the process that wrote it. (punktfunk fix —
|
||||
// upstream said 0 here, and winebus read that as failure.)
|
||||
UsbIpResponse::usbip_ret_submit_out_success(
|
||||
&header,
|
||||
data.len() as u32,
|
||||
)
|
||||
} else {
|
||||
trace!("<-Resp {resp:02x?}");
|
||||
UsbIpResponse::usbip_ret_submit_success(
|
||||
&header,
|
||||
0,
|
||||
0,
|
||||
resp,
|
||||
vec![],
|
||||
)
|
||||
}
|
||||
UsbIpResponse::usbip_ret_submit_success(&header, 0, 0, resp, vec![])
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("Error handling URB: {err}");
|
||||
|
||||
@@ -385,11 +385,12 @@ impl UsbIpResponse {
|
||||
Vec::with_capacity(48 + transfer_buffer.len() + iso_packet_descriptor.len());
|
||||
|
||||
debug_assert!(header.command == USBIP_RET_SUBMIT.into());
|
||||
// For an isochronous URB `actual_length` totals the per-packet actual lengths in
|
||||
// BOTH directions — on OUT that is nonzero while the transfer buffer is empty, so
|
||||
// it cannot be checked against the buffer. This mirrors the kernel's
|
||||
// `usbip_recv_iso()`, which sums the table and tears the whole connection down on
|
||||
// a mismatch. Non-ISO keeps the plain rule: IN returns its payload, OUT nothing.
|
||||
// `actual_length` is "bytes the device transferred", in BOTH directions and for
|
||||
// every transfer type. Only an IN reply carries a buffer to check it against: an
|
||||
// OUT reply reports the bytes it *accepted* and sends nothing back, so on OUT the
|
||||
// one invariant is an empty buffer. For an isochronous URB the value is the sum
|
||||
// of the per-packet actuals, which is what the kernel's `usbip_recv_iso()`
|
||||
// recomputes and tears the whole connection down over on a mismatch.
|
||||
debug_assert!(if number_of_packets != 0 {
|
||||
actual_length
|
||||
== iso_packet_descriptor
|
||||
@@ -399,7 +400,7 @@ impl UsbIpResponse {
|
||||
} else if header.direction == Direction::In as u32 {
|
||||
actual_length == transfer_buffer.len() as u32
|
||||
} else {
|
||||
actual_length == 0
|
||||
transfer_buffer.is_empty()
|
||||
});
|
||||
|
||||
result.extend_from_slice(&header.to_bytes());
|
||||
@@ -455,7 +456,9 @@ impl UsbIpResponse {
|
||||
}
|
||||
}
|
||||
|
||||
/// Constructs a successful OP_REP_IMPORT response
|
||||
/// Constructs a successful `USBIP_RET_SUBMIT` for an **IN** URB: the payload the device
|
||||
/// produced, with `actual_length` counting it. Not for OUT — see
|
||||
/// [`usbip_ret_submit_out_success`](Self::usbip_ret_submit_out_success).
|
||||
pub fn usbip_ret_submit_success(
|
||||
header: &UsbIpHeaderBasic,
|
||||
start_frame: u32,
|
||||
@@ -475,6 +478,36 @@ impl UsbIpResponse {
|
||||
}
|
||||
}
|
||||
|
||||
/// Constructs a successful `USBIP_RET_SUBMIT` for a non-isochronous **OUT** URB (punktfunk
|
||||
/// addition): no payload back, `actual_length` = the bytes the device `accepted`.
|
||||
///
|
||||
/// Upstream answered OUT with [`usbip_ret_submit_success`](Self::usbip_ret_submit_success) and
|
||||
/// an empty buffer, i.e. `actual_length = 0`. `vhci_hcd` copies that field straight into
|
||||
/// `urb->actual_length` (`usbip_pack_pdu(pdu, urb, USBIP_RET_SUBMIT, 0)` in
|
||||
/// `vhci_recv_ret_submit()`), and for an OUT URB the kernel has no other source for the count.
|
||||
/// So every synchronous writer up the stack was told "0 bytes transferred" on success:
|
||||
/// `usbhid_output_report()` returns `actual_length` as the byte count, so a `write()` on the
|
||||
/// pad's hidraw returned **0**, and `usb_control_msg()` returns the data-stage length, so
|
||||
/// `HIDIOCSFEATURE` returned **0** too. Anything checking `> 0` — winebus's
|
||||
/// `hidraw_device_set_output_report`, whose failure branch prints the thread's *stale* errno,
|
||||
/// hence "write failed error: 2 No such file or directory" — took the write as failed, and
|
||||
/// GE-Proton's `hidraw_enable_dualsense_usb_haptics` never enabled the DualSense's USB haptics
|
||||
/// mode. Adaptive triggers, voice-coil haptics and the speaker are all gated behind that one
|
||||
/// enable. A real usbip stub reports the real URB's `actual_length`, which on OUT is the
|
||||
/// number of bytes sent. Field-diagnosed 2026-08-18.
|
||||
pub fn usbip_ret_submit_out_success(header: &UsbIpHeaderBasic, accepted: u32) -> Self {
|
||||
Self::UsbIpRetSubmit {
|
||||
header: header.clone(),
|
||||
status: 0,
|
||||
actual_length: accepted,
|
||||
start_frame: 0,
|
||||
number_of_packets: 0,
|
||||
error_count: 0,
|
||||
transfer_buffer: Vec::new(),
|
||||
iso_packet_descriptor: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Constructs a successful `USBIP_RET_SUBMIT` for an **isochronous** URB (punktfunk addition).
|
||||
///
|
||||
/// `replies` holds one payload per ISO packet (empty for an OUT endpoint) and `requested` the
|
||||
@@ -670,4 +703,32 @@ mod iso_tests {
|
||||
assert_eq!(&bytes[48..50], &[1, 2]);
|
||||
assert_eq!(be(&bytes[50 + 8..50 + 12]), 2, "actual_length clamped");
|
||||
}
|
||||
|
||||
/// The exact 2026-08-18 field failure: an interrupt-OUT HID output report (a 48-byte
|
||||
/// DualSense report `0x02`) must be acknowledged as 48 bytes *accepted*, with no payload
|
||||
/// after the 48-byte header. `vhci_hcd` copies `actual_length` into the URB verbatim and
|
||||
/// `usbhid_output_report()` hands it back as `write()`'s return value — the old reply said 0,
|
||||
/// so every hidraw write on the pad "succeeded" with 0 bytes and winebus took it as failed.
|
||||
#[test]
|
||||
fn non_iso_out_reply_acknowledges_the_bytes_accepted_and_carries_no_payload() {
|
||||
let bytes = UsbIpResponse::usbip_ret_submit_out_success(&header(0), 48).to_bytes();
|
||||
assert_eq!(
|
||||
bytes.len(),
|
||||
48,
|
||||
"header only: the kernel reads no payload back on OUT"
|
||||
);
|
||||
assert_eq!(be(&bytes[20..24]), 0, "status");
|
||||
assert_eq!(be(&bytes[24..28]), 48, "bytes accepted, NOT bytes returned");
|
||||
assert_eq!(be(&bytes[32..36]), 0, "not isochronous");
|
||||
}
|
||||
|
||||
/// The IN constructor keeps counting the payload it returns.
|
||||
#[test]
|
||||
fn non_iso_in_reply_counts_its_payload() {
|
||||
let bytes =
|
||||
UsbIpResponse::usbip_ret_submit_success(&header(1), 0, 0, vec![7u8; 41], vec![])
|
||||
.to_bytes();
|
||||
assert_eq!(bytes.len(), 48 + 41);
|
||||
assert_eq!(be(&bytes[24..28]), 41);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -124,10 +124,21 @@ def main(prefix):
|
||||
if p["cmd"] != RET_SUBMIT:
|
||||
continue
|
||||
req = submits.get(p["seq"])
|
||||
# The two rules vhci_hcd kills the whole connection over, checked against its own logic.
|
||||
if p["dir"] == 0 and not p["npkts"] and p["actual"]:
|
||||
bad.append((p, f"OUT reply declares actual_length={p['actual']}, but the kernel reads "
|
||||
f"NO payload back on OUT — those bytes desync every frame after it"))
|
||||
# The rule vhci_hcd kills the whole connection over, checked against its own logic — plus
|
||||
# the quieter one that kills nothing and breaks everything: an OUT reply's `actual_length`
|
||||
# is copied into the URB verbatim and is what `write()` on the device node returns, so an
|
||||
# OUT reply claiming 0 against a non-empty request tells every writer it sent nothing.
|
||||
# (An earlier version of this check flagged ANY nonzero OUT actual_length as a desync;
|
||||
# that was wrong — the kernel reads no payload back on OUT whatever the field says, so the
|
||||
# framing above never depends on it. Field-diagnosed 2026-08-18: winebus's DualSense
|
||||
# haptics enable "failed" for months on exactly this.)
|
||||
if req and p["dir"] == 0 and not p["npkts"] and p["actual"] > req["xfer_len"]:
|
||||
bad.append((p, f"OUT reply claims actual_length={p['actual']} > the {req['xfer_len']} "
|
||||
f"bytes the kernel sent — a device cannot accept more than it was given"))
|
||||
elif req and p["dir"] == 0 and not p["npkts"] and req["xfer_len"] and not p["actual"]:
|
||||
bad.append((p, f"OUT reply claims actual_length=0 against a {req['xfer_len']}-byte "
|
||||
f"write — write()/HIDIOCSFEATURE on the device node return 0 and every "
|
||||
f"caller checking '> 0' (winebus does) takes the write as failed"))
|
||||
elif req and p["dir"] == 1 and p["actual"] > req["xfer_len"]:
|
||||
bad.append((p, f"actual_length {p['actual']} > the {req['xfer_len']} requested "
|
||||
f"(setup {req['setup']}) — usbip_recv_xbuff() calls this a malicious "
|
||||
|
||||
Reference in New Issue
Block a user