daniel/danos
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects 1 Releases Wiki Activity

usb: a configuration block is as long as the device says it is

Browse Source 
The driver read the first 512 bytes of a configuration block into a fixed
buffer and parsed those. The block's length is the device's own choice
(wTotalLength, a u16), so anything larger was silently cut: interfaces past
the cut did not exist as far as the host was concerned, while the
SET_CONFIGURATION that follows still configured the device for all of them.
A headset is 500-900 bytes, a UVC webcam 1-3 KB, a multifunction printer
600+.

Now allocated at the declared length, so the ceiling is the field's u16 —
the specification's number rather than one of ours. A block shorter than its
own 9-byte header is refused rather than trusted.

The bring-up line reports the declared length and the bytes actually read,
so a truncation can never again be invisible, and usb-large-descriptor
asserts they match with a backreference.

That case has an honest limit, recorded in its comment: QEMU cannot produce
a block over 512 bytes. The boot keyboard, mouse and stick are 34-44, and
the largest device available is usb-audio in multi-channel mode at 211 —
which is exactly why the suite never caught this, and why it cannot now
reproduce the original trigger. What it does catch is the class: any clamp
below the attached device's block fails it, verified by pinning the buffer
to 128 and watching "config block 211 bytes, read 128" turn the case red.

Suite 115 -> 116.
This commit is contained in:
Daniel Samson
2026-08-08 11:53:01 +01:00
parent 69fbef40c0
commit 6328823ef1
4 changed files with 42 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/bounds-track-plan.md
+1 -1
View File
@@ -15,7 +15,7 @@ next one starts.*
| L1 | Reclamation: a dead task's registrations die with its claims | **stopped — the step was wrong; see open question 4** |
| L2 | Bounds build check + allowlist; declare what we have already touched | **done** — `zig build bounds`, 273 allowlisted, 5 declared |
| L3 | xHCI: slot count from `HCSPARAMS1.MaxSlots`, not 8 | **done** — QEMU reports 64; the driver tracked 8 |
| L4 | USB: configuration descriptor sized by `wTotalLength`, not 512 | not started |
| L4 | USB: configuration descriptor sized by `wTotalLength`, not 512 | **done** — QEMU tops out at 211 bytes, so the case catches the class, not the original trigger |
| L5 | USB: interfaces from the descriptor, and the misattributed-endpoint bug | not started |
| L6 | xHCI: a failed `allocateDevice` stops leaking an enabled slot | not started |
system/drivers/usb-xhci-bus/usb-xhci-library.zig
+19 -6
View File
@@ -1311,12 +1311,25 @@ pub const Controller = struct {
const configuration = std.mem.bytesToValue(usb_abi.ConfigurationDescriptor, &header);
device.configuration_value = @intFromEnum(configuration.configuration_value);
// Read the whole block into a local buffer and parse it here (in the bus
// driver) so the parse never has to cross the 256-byte IPC boundary.
var blob: [512]u8 = undefined;
const length = @min(configuration.total_length, blob.len);
if (!self.controlTransfer(device, usb_abi.getDescriptor(.configuration, 0, 0, @intCast(length)), blob[0..length], true)) return false;
parseConfiguration(device, blob[0..length]);
// Read the whole block and parse it here (in the bus driver) so the parse
// never has to cross the 256-byte IPC boundary. Sized by the device's own
// wTotalLength — the ceiling is then the field's u16, which is the USB
// specification's, not ours.
//
// This was a fixed 512 with an `@min` clamp, which silently truncated: a
// composite device routinely exceeds it (a headset is 500-900 bytes, a UVC
// webcam 1-3 KB, a multifunction printer 600+), and the interfaces past the
// cut simply did not exist — while SET_CONFIGURATION below still configured
// the device for all of them. QEMU's boot keyboard, mouse and stick are all
// under 100 bytes, which is why it survived.
if (configuration.total_length < header.len) return false; // shorter than its own header
const blob = memory.allocator().alloc(u8, configuration.total_length) catch return false;
defer memory.allocator().free(blob);
if (!self.controlTransfer(device, usb_abi.getDescriptor(.configuration, 0, 0, configuration.total_length), blob, true)) return false;
// Both numbers, so a truncation can never again be invisible: the block the
// device declared, and the bytes actually fetched and parsed. They must match.
std.log.info("config block {d} bytes, read {d}", .{ configuration.total_length, blob.len });
parseConfiguration(device, blob);
// Select the configuration, moving the device to the configured state.
if (!self.controlTransfer(device, usb_abi.setConfiguration(configuration.configuration_value), &.{}, false)) return false;
test/qemu_test.py
+22
View File
@@ -674,6 +674,28 @@ CASES = [
r"(?=.*hub slot \d+ port \d+ device:.*0x0627)"
r"(?=.*usb-hid-keyboard: ok \(device 3)",
"fail": r"DANOS-TEST-RESULT: FAIL"},
# The driver must never truncate a configuration block. Its size is the DEVICE's
# choice (wTotalLength, a u16); the driver used to read the first 512 bytes into a
# fixed buffer and parse those, so interfaces past the cut did not exist while
# SET_CONFIGURATION still configured the device for all of them.
#
# The backreference is the assertion: declared length and bytes read must match.
#
# Honest limit: QEMU cannot produce a block over 512 bytes. Its boot keyboard,
# mouse and stick are 34-44, and the largest device on offer is usb-audio in
# multi-channel mode at 211 — which is why the suite never saw the original bug,
# and why it cannot now reproduce that exact trigger. What this case does catch is
# the class: any clamp below the attached device's block size fails it, verified
# by pinning the buffer to 128 and watching it go red. A real headset (500-900
# bytes), UVC webcam (1-3 KB) or multifunction printer trips the 512 itself.
{"name": "usb-large-descriptor",
"build_case": "usb-hid",
"smp": 4,
"timeout": 150,
"qemu_extra": ["-device", "qemu-xhci,id=xhci2",
"-device", "usb-audio,bus=xhci2.0,port=1,multi=on"],
"expect": r"(?s)(?=.*usb-xhci-bus: config block (\d\d\d+) bytes, read \1)",
"fail": r"DANOS-TEST-RESULT: FAIL"},
# USB mass storage end to end: the boot usb-storage device (the FAT32 image,
# which has a real 0x55AA boot sector) is enough — the manager spawns
# usb-storage, which opens the device, runs the Bulk-Only / SCSI bring-up,
tools/bounds-allowlist.txt
-1
View File
@@ -76,7 +76,6 @@ system/drivers/usb-xhci-bus/usb-xhci-bus.zig:maker_buffer
system/drivers/usb-xhci-bus/usb-xhci-bus.zig:prev_connected
system/drivers/usb-xhci-bus/usb-xhci-bus.zig:product_buffer
system/drivers/usb-xhci-bus/usb-xhci-bus.zig:product_buffer
system/drivers/usb-xhci-bus/usb-xhci-library.zig:blob
system/drivers/usb-xhci-bus/usb-xhci-library.zig:buffer
system/drivers/usb-xhci-bus/usb-xhci-library.zig:bytes
system/drivers/usb-xhci-bus/usb-xhci-library.zig:data