DF-4644 — fb.c adapter/vidsw/vidcdevsw array lifecycle is not consistently

serialized (UAF window on vid_realloc_array + register index race)

• File: sys/dev/video/fb/fb.c:76-122, 150-181, 183-200, 230-245, 285-291
• Severity: Low (race window real, trigger platform-gated)
• Status: untested — triggering requires ≥2 video adapters registering/ unregistering concurrently (second card at attach, or a loadable fb driver module); the stock x86 guest has exactly one built-in vga0 and FB_INSTALL_CDEV is not compiled (LINT64-only), so the race cannot be driven on the audit guest.

The defects

1. Array realloc frees outside the serialization token. vid_realloc_array() (fb.c:76-122) allocates new arrays, copies, then kfree(adapter)/kfree(vidsw)/kfree(vidcdevsw) (fb.c:104-108) and swaps the global pointers (fb.c:110-114) — all under crit_enter() only, which is per-CPU and does not stop other CPUs. It is called from vid_register() (fb.c:162) before lwkt_gettoken(&vga_token) (fb.c:168).
2. Tokenless readers. vid_find_adapter() (fb.c:235-243) scans adapter[i] with no token; vid_get_adapter() (fb.c:288-290) reads adapter[index] with no token; vid_allocate() (fb.c:253-261) scans/updates under crit_enter() only. A concurrent realloc on another CPU frees the array underneath them → use-after-free reads. Note vid_unregister() does take the token (fb.c:186-198) and writes adapter[va_index] = NULL (fb.c:196) — a write that can land in the freed old array if realloc swapped it in between → use-after-free write window.
3. Concurrent registration index collision. Two CPUs in vid_register() both scan for a free slot (fb.c:157-160) before either holds the token (fb.c:168); both can claim the same index and both write adapter[index]/vidsw[index] (fb.c:172-173). The loser's adapter keeps a va_index owned by the winner → later fb_commonioctl() dispatch (*vidsw[adp->va_index]->get_info)(...) (fb.c:706, 714, 723) calls one driver's switch on another driver's video_adapter_t → type confusion.

On the stock x86 guest only one adapter ever registers (vga0, built-in, boot-time), so vid_realloc_array() never even runs (adapters stays 1). The race is reachable only on multi-adapter or module-load configurations.

Supporting guest probe (dispatch sanity, not a race PoC)

fb_dispatch_probe.c + probe_run.log in this directory were run as root on the audit guest against /dev/ttyv0 to validate the surrounding dispatch analysis: FBIO_ADPINFO returns every field kernel-written with va_reserved fully zeroed (by the ioctl layer's IOC_OUT bzero, sys/kern/sys_generic.c:685-690) — no uninitialized-leak class beyond the known DF-1786 pointer fields — and FBIO_MODEINFO rejects out-of-range mode ids (999, -5, 0x7fffffff) with ENODEV, confirming vga_get_info's get_mode_param() bounds (vga.c:527-537) hold at runtime.

Fix

Serialize the whole array lifecycle with vga_token: take the token in vid_register/vid_realloc_array/vid_find_adapter/vid_get_adapter/ vid_allocate (or a dedicated lock), and re-scan the free slot after acquiring it.

DF-4644 — VERDICT

Not verified on the guest (untested). The race is demonstrated by code inspection; driving it requires concurrent adapter registration/unregistration, which the audit guest cannot produce:

• vm.sh run_root facts: only vgapci0 (QEMU virtual video) exists; /dev/vga does not exist because FB_INSTALL_CDEV is defined only in LINT64 (sys/config/LINT64:2181; options mapping sys/conf/options:404), so fb.c's cdev block (fb.c:317-551: fb_attach/fb_detach/genfb*) is not compiled into the running X86_64_GENERIC kernel at all.
• vga is built into the kernel (sys/config/X86_64_GENERIC:155 device vga0 at isa?) and registers exactly one adapter during boot, serially. adapters never exceeds 1, so vid_realloc_array() never executes on this system.
• No loadable fb driver modules are present to add a second adapter at runtime.

Proven defect summary (path:line)

1. vid_realloc_array fb.c:76-122: frees the old global arrays (fb.c:104-108) and swaps adapter/vidsw/vidcdevsw pointers (fb.c:110-114) under crit_enter() (fb.c:89) — per-CPU only. Called from vid_register fb.c:162 BEFORE the token is taken at fb.c:168, so even token-taking readers are exposed to the free/swap.
2. Tokenless readers/writers: - vid_find_adapter fb.c:235-243 (reader; also called from vid_allocate fb.c:254 under only crit_enter fb.c:253), - vid_get_adapter fb.c:288-290 (reader; reachable from the console ioctl path scvidctl.c:474), - vid_unregister fb.c:186-198 (writer under token; its write at fb.c:196 can hit the freed old array if a concurrent realloc on another CPU swapped it out mid-flight) → UAF write window.
3. vid_register free-slot scan fb.c:157-160 happens before the token fb.c:168; two concurrent registrations can claim the same index, both writing adapter[index]/vidsw[index] (fb.c:172-173); the loser's adapter then has a va_index mapping to the winner's vidsw → type-confused dispatch through fb_commonioctl fb.c:706/714/723 ((*vidsw[adp->va_index]->get_info)(adp, ...)).

Impact ceiling: UAF read/write on a small pointer array (kernel heap info-leak or constrained wild write through a stale pointer slot) and cross-driver type confusion. Realistic exposure is low: multi-adapter x86 or kmod fb drivers only; no such configuration ships by default.

Runtime sanity evidence actually gathered (guest, root)

See fb_dispatch_probe.c / probe_run.log: - FBIO_ADAPTER → index 0; FBIO_ADPINFO → name "vga", all fields written, va_reserved 0/64 bytes untouched (ioctl layer zeroing) — confirms no uninitialized-stack leak in the ADPINFO fill beyond the known DF-1786 pointer fields (va_window=0x800b8000 observed). - FBIO_MODEINFO with vi_mode ∈ {0,2,999,-5,0x7fffffff} → ENODEV (EINVAL from vga_get_info mapped by fb.c:709-710); no crash; bounds hold. - FBIO_FINDMODE 80x25 → mode 24.

Recommended fix (fb.c)

--- a/sys/dev/video/fb/fb.c
+++ b/sys/dev/video/fb/fb.c
@@ -154,6 +154,8 @@ vid_register(video_adapter_t *adp)
    const video_driver_t *p;
    int index;

+   lwkt_gettoken(&vga_token);  /* serialize whole array lifecycle */
+retry:
    for (index = 0; index < adapters; ++index) {
        if (adapter[index] == NULL)
            break;
@@ -174,12 +176,15 @@ vid_register(video_adapter_t *adp)
        p = *list;
        if (strcmp(p->name, adp->va_name) == 0) {
            adapter[index] = adp;
            vidsw[index] = p->vidsw;
            lwkt_reltoken(&vga_token);
            return index;
        }
    }

    lwkt_reltoken(&vga_token);
    return -1;

plus: move the vid_realloc_array() call under the token (and have it not take the token itself), and take the token in vid_find_adapter/vid_get_adapter/ vid_allocate. (Exact diff to be finalized by the maintainer; no fix was built or validated at runtime for this finding — fix_status: not_testable.)

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.

(No fix.diff for this item: documented recommendation only - see Fix above.)