Wrong loop bound (USB_DEV_QUIRKS_MAX vs USB_QUIRK_MAX) in USB_DEV_QUIRK_ADD/REMOVE quirk-name lookup

Summary

In USB_DEV_QUIRK_ADD and USB_DEV_QUIRK_REMOVE, the quirk-name-to-number lookup loop iterates y != USB_DEV_QUIRKS_MAX (384) instead of y != USB_QUIRK_MAX (96). Because usb_quirkstr() returns the string "UQ_UNKNOWN" for every y >= USB_QUIRK_MAX, a privileged caller supplying quirkname="UQ_UNKNOWN" matches at y == USB_QUIRK_MAX and the code then proceeds to store or remove quirk value 96 (== USB_QUIRK_MAX) in the table. There is no memory-corruption or privilege-escalation impact: the stored uint16 is in-bounds for pqe->quirks[], the value is never queried by any kernel caller (all UQ_* lookups pass values < USB_QUIRK_MAX), and it reads back harmlessly as "UQ_UNKNOWN" via USB_DEV_QUIRK_GET. It is a genuine logic bug (wrong constant) in a privileged code path and is reported only for correctness / defense-in-depth.

Root cause

usb_quirk.c:883-887 (ADD) and usb_quirk.c:921-927 (REMOVE) use for (y = 0; y != USB_DEV_QUIRKS_MAX; y++) to scan the quirk-name table, but the quirk-name enum/table only has USB_QUIRK_MAX (96) valid entries.

/* usb_quirk.c:883-887 — USB_DEV_QUIRK_ADD */
for (y = 0; y != USB_DEV_QUIRKS_MAX; y++) {     /* WRONG bound */
    if (strcmp(pgq->quirkname, usb_quirkstr(y)) == 0) {
        break;
    }
}
if (y == USB_DEV_QUIRKS_MAX) {                  /* never triggers via sentinel */
    return (EINVAL);
}

The subsequent not-found test (y == USB_DEV_QUIRKS_MAX) can therefore never trigger via the sentinel string, because usb_quirkstr(y) for any y in [USB_QUIRK_MAX, USB_DEV_QUIRKS_MAX) returns "UQ_UNKNOWN", so input "UQ_UNKNOWN" matches at y == USB_QUIRK_MAX and falls through both the not-found test and the if (y == UQ_NONE) test (usb_quirk.c:891, :929). The correct bound is USB_QUIRK_MAX, matching usb_strquirk() at usb_quirk.c:718.

Threat model & preconditions

• Attacker position: Local user holding SYSCAP_NODRIVER (caps_priv_check_self at usb_quirk.c:878/:916) — effectively uid 0.
• Privileges gained or impact: None. The only observable effect is that a root user can store a quirk entry whose quirk code equals USB_QUIRK_MAX (96), outside the valid UQ_* enum range. This value is inert: usb_test_quirk_by_info is only called by kernel USB drivers with valid UQ_* constants (< 96), so the bogus entry never matches; usb_quirkstr() renders it as "UQ_UNKNOWN". No memory unsafety, no info leak, no DoS.
• Required config or capabilities: Default kernel with usb configured; root.
• Reachability: ioctl(/dev/usb, USB_DEV_QUIRK_ADD, {.vid=0x1234, .pid=0x5678, .bcdDeviceLow=0, .bcdDeviceHigh=0xffff, .quirkname="UQ_UNKNOWN"}) returns 0 (success) instead of EINVAL.

Proof of concept

/* qadd.c — exercises the wrong-bound logic bug. No security payload. */
#include <fcntl.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <bus/u4b/usb_freebsd.h>
#include <bus/u4b/quirk/usb_quirk.h>

int main(void) {
    int fd = open("/dev/usb", O_RDONLY);
    if (fd < 0) { perror("open /dev/usb"); return 1; }
    struct usb_quirk_add_generic pgq = {
        .vid = 0x1234, .pid = 0x5678,
        .bcdDeviceLow = 0, .bcdDeviceHigh = 0xffff,
        .mode = 0, .index = 0,
    };
    strlcpy(pgq.quirkname, "UQ_UNKNOWN", sizeof(pgq.quirkname));
    int rc = ioctl(fd, USB_DEV_QUIRK_ADD, &pgq);
    printf("USB_DEV_QUIRK_ADD returned %d (errno=%d) — expected EINVAL\n",
        rc, rc ? 0 : -1);
    return 0;
}

Build & run

cc -o qadd qadd.c
sudo ./qadd
# Expected: "USB_DEV_QUIRK_ADD returned 0 (errno=0) — expected EINVAL"
sudo usbconfig dump_device_quirks
# Expected: an entry with vid=0x1234 pid=0x5678 quirkname=UQ_UNKNOWN

No panic, no corruption. The PoC only proves the logic defect; there is no security payload.

Impact

None beyond a polluted quirk-table entry that confuses tooling (e.g. usbconfig dump_device_quirks). Cannot affect device probe/attach behavior. Filed as Info per the AGENT.md rubric ("Hardening opportunity, defense-in-depth, no demonstrated impact").

Recommended fix

Change the loop bound and the not-found test in both USB_DEV_QUIRK_ADD and USB_DEV_QUIRK_REMOVE from USB_DEV_QUIRKS_MAX to USB_QUIRK_MAX.

--- a/sys/bus/u4b/quirk/usb_quirk.c
+++ b/sys/bus/u4b/quirk/usb_quirk.c
@@ -880,7 +880,7 @@
        }
        /* convert quirk string into numerical */
-       for (y = 0; y != USB_DEV_QUIRKS_MAX; y++) {
+       for (y = 0; y != USB_QUIRK_MAX; y++) {
            if (strcmp(pgq->quirkname, usb_quirkstr(y)) == 0) {
                break;
            }
        }
-       if (y == USB_DEV_QUIRKS_MAX) {
+       if (y == USB_QUIRK_MAX) {
            return (EINVAL);
        }
        if (y == UQ_NONE) {
@@ -918,7 +918,7 @@
        /* check privileges */
        err = caps_priv_check_self(SYSCAP_NODRIVER);
        if (err) {
            return (err);
        }
        /* convert quirk string into numerical */
-       for (y = 0; y != USB_DEV_QUIRKS_MAX; y++) {
+       for (y = 0; y != USB_QUIRK_MAX; y++) {
            if (strcmp(pgq->quirkname, usb_quirkstr(y)) == 0) {
                break;
            }
        }
-       if (y == USB_DEV_QUIRKS_MAX) {
+       if (y == USB_QUIRK_MAX) {
            return (EINVAL);
        }
        if (y == UQ_NONE) {

References

• sys/bus/u4b/quirk/usb_quirk.c:883-893 — USB_DEV_QUIRK_ADD wrong-bound loop
• sys/bus/u4b/quirk/usb_quirk.c:921-931 — USB_DEV_QUIRK_REMOVE wrong-bound loop
• sys/bus/u4b/quirk/usb_quirk.c:718-724 — usb_strquirk correct bound (USB_QUIRK_MAX)
• sys/bus/u4b/quirk/usb_quirk.h — USB_QUIRK_MAX (96) vs USB_DEV_QUIRKS_MAX (384)

Timeline

• 2026-07-14 Discovered during automated audit.