DF-5604 — post-unlock use of shared per-udev scratch in device_set_usb_desc

File: sys/bus/u4b/usb_util.c (pass-2 GLM 5.3 re-audit) Severity: Low · Class: CWE-367 (TOCTOU) / CWE-125 (OOB read) Verdict: reproduced (in-kernel mechanism, deterministic 5/5) — see verdict.json

What the finding is

device_set_usb_desc() (sys/bus/u4b/usb_util.c:94-115) fills the shared per-device scratch buffer udev->scratch.data (255 bytes, shared by every enumeration path of the udev and documented as "protected by the enumeration lock", sys/bus/u4b/usb_device.h:168-170) under usbd_enum_lock(), then drops the lock (line 110-111) and only afterwards hands the scratch pointer to:

• device_set_desc_copy(dev, temp_p) → device_set_desc_internal() → unbounded strlen(desc) + strcpy() (sys/kern/subr_bus.c:1759-1775), and
• device_printf(dev, "<%s>", temp_p) (usb_util.c:114) — printing whatever the (possibly concurrently rewritten) scratch contains to console/msgbuf.

A concurrent enumeration operation on the same udev that takes the enum lock in that window and calls usbd_req_get_string_any() (usb_request.c:1095) first DMAs a device-controlled raw descriptor copy over the scratch (clobbering the string and its NUL terminator) and only terminates the converted string at the very end (*s = 0, usb_request.c:1176). A malicious device that supplies 16-bit string characters (every raw byte nonzero, every char "silently skipped" by the filter) leaves the whole 255-byte scratch NUL-free for the entire fetch+conversion window — the reader's strlen() then walks out of scratch.data into the rest of struct usb_device and the adjacent kernel heap, and those bytes are copied into dev->desc and printed to the console (dmesg-visible kernel-heap disclosure; panic if the scan crosses into an unmapped page).

FreeBSD's current device_set_usb_desc (cgit, sys/dev/usb/usb_util.c) no longer fetches device strings into scratch here at all (it uses the ctrl lock and only formats cached udev->manufacturer/product strings) — DragonFly's variant is the stale pre-refactor shape with a strictly wider window (raw device-controlled bytes in the shared buffer).

Reproduce

Build + run on the audit QEMU guest (see ../ for guest lifecycle):

./build.sh      # cc via bsd.kmod.mk → usbutil_probe.ko (-Werror clean)
./run.sh        # kldload; runs case 0 (matrix) at load;
                # sysctl hw.usutil_probe=1  → control + raced runs
                # sysctl hw.usutil_probe=2  → deliberate NULL panic (LAST)

Expected decisive output (see run.log, run.2.log, panic.txt):

UPROBE case0 matrix: 108 cells, 0 failures (usb_make_str_desc write discipline)
UPROBE case1(control): strlen(scratch)=25 scratch=255 bytes_past_boundary=0 ...
UPROBE case1(RACED): strlen(scratch)=1023 scratch=255 bytes_past_boundary=769
    desc[255..290]="ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ"

• case0 is the negative evidence for the checklist item "check bLength/truncation discipline hard": exhaustive boundary matrix (max_len ∈ {0,1,2,3,4,5,254,255,256,511,512,513,1023,1024,32767,32768, 65534,65535} × strings {"", 1, 31, 126, 127, 1000 chars}) with canary checks — usb_make_str_desc never writes at or past totlen, never past max_len (odd tail byte included), bLength is always exact, UTF-16LE payload always correct. 108/108 PASS.
• case1 reproduces the DF-5604 race mechanism with the verbatim code shapes of the production writer (usbd_req_get_string_any, usb_request.c:1129-1176), the production lock discipline (usbd_enum_lock/unlock, usb_util.c:94-111) and the production reader (strlen+copy as in subr_bus.c:1759-1775). Control run: 25 bytes. Raced run: strlen runs 769 bytes past the 255-byte scratch boundary and captures adjacent-heap marker bytes (0x5A) into the would-be dev->desc / <%s> console string. Deterministic, 5/5 runs.
• case2 documents the missing s == NULL guard in usb_make_str_desc (usb_util.c:228 strlen(s)): a NULL string panics the kernel at strlen: cmpb $0,(%rdi) (fault address 0x0). No production caller passes NULL today (all four callers pass constants/sc->vendor), so this is a hardening note, not a second finding.

Scope of the reproduction (read this)

The harness drives the production code shapes with adversarial scheduling inside a KLD; it does not call device_set_usb_desc() itself, because that requires a live enumerated USB device and a real concurrent same-udev enumeration thread (the natural sources are hub-explore racing a devd/kldload-triggered re-probe). The defect, the window and the OOB read are fully code-proven (VERDICT.md) and the mechanism is demonstrated deterministically in-kernel. The production interleave is not driven end-to-end — that is why severity stays Low and confidence likely.

Fix

fix.diff: snapshot the scratch string into a 256-byte stack buffer while the enum lock is still held, then unlock and use the snapshot.

DF-5604 VERDICT — post-unlock use of shared scratch in device_set_usb_desc

Bottom line

reproduced (mechanism, deterministic, 5/5 in-kernel runs) — Low severity, confidence likely. device_set_usb_desc() reads the shared per-udev scratch buffer after dropping the enumeration lock that is the buffer's only documented protection. A concurrent same-udev enumeration writer (the code path usbd_req_get_string_any() that this very function invokes) can transiently leave the 255-byte scratch NUL-free, turning the reader's strlen()/strcpy() (device_set_desc_copy → device_set_desc_internal, sys/kern/subr_bus.c:1759-1775) into an unbounded OOB heap read whose bytes are copied into dev->desc and printed by device_printf(dev, "<%s>") (usb_util.c:114) to console/msgbuf. Ceiling: kernel-heap info leak into dmesg; panic if the scan reaches an unmapped page.

Root cause, line-accurate

• sys/bus/u4b/usb_util.c:94 do_unlock = usbd_enum_lock(udev);
• sys/bus/u4b/usb_util.c:96 temp_p = (char *)udev->scratch.data;
• sys/bus/u4b/usb_util.c:100-103 device-controlled string fetched into scratch (usbd_req_get_string_any)
• sys/bus/u4b/usb_util.c:110-111 lock dropped
• sys/bus/u4b/usb_util.c:113 device_set_desc_copy(dev, temp_p); → unbounded strlen()/strcpy() (sys/kern/subr_bus.c:1759-1775)
• sys/bus/u4b/usb_util.c:114-115 device_printf(dev, "<%s>", temp_p, ...)

Scratch contract: sys/bus/u4b/usb_device.h:168-170 — "Access to this structure is protected by the enumeration lock".

Writer side that makes the window NUL-free (all device-controlled):

• sys/bus/u4b/usb_request.c:1104-1107 raw descriptor DMA'd into the caller's buffer (= scratch here): device-chosen bLength bytes, no NUL.
• sys/bus/u4b/usb_request.c:1129-1175 in-place conversion; characters with both bytes nonzero (any 16-bit codepoint whose low byte is also nonzero) are "silently skipped" — s never advances, so nothing is overwritten.
• sys/bus/u4b/usb_request.c:1176 *s = 0; — the only NUL, written at the very end of the fetch.

So for a device string of 126 skipped 16-bit chars, from raw-copy completion until *s = 0 there is no zero byte anywhere in scratch.data[0..254], and any unlocked reader's strlen() leaves the buffer. (For plain ASCII strings the unread raw tail still contains zero high-bytes, so the leak needs the 16-bit/skipped-char shape — fully attacker-chosen, USB string descriptors are arbitrary device data.)

One more precondition is scratch.data[254] being nonzero: reachable via a previous malformed-but-tolerated bLength=255 fetch whose raw copy covers all 255 bytes (odd bLength is not rejected before the copy).

Why the reader can actually race (threat model)

The reader runs in the attach path of every u4b child driver (60+ callers of device_set_usb_desc: umass.c:939, ukbd.c:1220, ucom drivers, …). Same-udev probing is normally serialized in the bus explore thread, but the scratch is shared by all enumeration paths of the udev, and concurrent writers are reachable when a driver-module load (devd/kldload reprobe) or another enumeration trigger overlaps hotplug enumeration. The attacker is therefore: a malicious USB device supplying the NUL-free string content + a lucky (or root-assisted) reprobe timing. That ceiling — plus the console-only disclosure — is why this is Low and not Medium.

Upstream note: FreeBSD's current device_set_usb_desc no longer fetches device strings into scratch at all (ctrl lock + cached udev->manufacturer/product only) — DragonFly carries the older, wider variant of this function.

What was reproduced (and what was not)

In-kernel KLD (usbutil_probe.ko), built -Werror clean on the audit guest (DragonFly 6.5-DEVELOPMENT X86_64_GENERIC, Jul 2 2026):

1. case 0 — negative evidence for the string-descriptor constructor: 108-cell boundary matrix over usb_make_str_desc() (max_len 0,1,2,3,4,5, 254,255,256,511,512,513,1023,1024,32767,32768,65534,65535 × strings "",1,31,126,127,1000 chars) with canary verification: 0 failures. Every write stays within [0, totlen), totlen == 2*(min(max_len/2-1, min(strlen,126))+1) exactly, header bytes and UTF-16LE payload correct, the odd-max_len tail byte is never touched. usb_make_str_desc's buffer math is safe — hand-proof: max_len>=2 guard prevents the /2-1 underflow; j is clamped to 126 before the min, so totlen<=254 fits uint8_t; loop writes only bString[0..max_len-1].
2. case 1 — DF-5604 mechanism: two kernel threads + lockmgr enum-lock with the verbatim production shapes (writer = raw descriptor bcopy + skipped-char conversion + terminal *s=0; reader = post-unlock strlen+copy+%s). Control: 25 bytes. Raced: strlen=1023, 769 bytes past the 255-byte scratch boundary, adjacent-heap markers (0x5A…) captured into the desc buffer — i.e. exactly the bytes that device_printf("<%s>") would disclose. Deterministic 5/5 (the writer's mid-conversion 1-tick stall models the control-transfer/filter latency; it widens the window, it does not change the algorithm).
3. case 2 — hardening note: usb_make_str_desc(buf, 64, NULL) → strlen(NULL) → Fatal trap 12 ... fault virtual address = 0x0 ... Stopped at strlen: cmpb $0,(%rdi) (panic.txt). No production caller passes NULL (ohci.c:2165, uhci.c:2560, ehci.c:3192, xhci.c:3389 all pass constants or sc->vendor), and FreeBSD has the same missing guard — recorded as a hardening note, not a finding.

Not reproduced: the end-to-end production interleave (physical malicious USB device + attach racing a reprobe). device_set_usb_desc() itself was not invoked by the harness — it needs a live enumerated udev; the harness instead reproduces the exact reader/writer/lock code shapes with adversarial scheduling. This is stated plainly so nobody over-reads the reproduced flag.

Fix

fix.diff: snapshot the scratch string into a 256-byte stack buffer while the enum lock is held; unlock; use the snapshot for device_set_desc_copy() and the printf. Fix not build-validated: a patched kernel + re-run of this mechanism harness would not exercise the patched production function (the harness drives code shapes, not device_set_usb_desc()), so a fix A/B would be theater; the change is 5 lines and correct by inspection (both writers NUL-terminate within [0,255) under the lock, so the snapshot is always a valid string).

Classes considered and killed (pass-2 negative results)

• usb_make_str_desc truncation/bLength/OOB write — safe (hand + 108/108 dynamic). Missing s==NULL guard noted (hardening).
• usb_printbcd — only caller usb_generic.c:833, ksnprintf bounded, output ≤6 bytes incl NUL into udi_release; safe.
• usb_trim_spaces — writes stay within the original string bounds (write index ≤ copy index; e trails the last non-space); safe.
• usb_pause_mtx — timo+1 overflow unreachable for all 84 call sites (max hz*65535*5/1000 ≪ INT_MAX); safe.
• usb_pause_ls — zero callers in the tree (dead code); would need a non-NULL serializer if ever wired up.
• device_set_usb_desc type confusion on device_get_ivars — all callers are u4b child drivers whose ivars are struct usb_attach_arg; killed.
• enum-lock recursion — usbd_enum_lock is recursion-aware (do_unlock), no unbalanced unlock on the recursive path; killed.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.