# DF-2323 — VPD error-cleanup loops rely on a NULL sentinel that the success-path shrink krealloc removes — VERDICT

**status: reproduced (on production-config kernel) / impact: corruption
(free-of-live-object + overlapping allocations; wild-pointer variant
panics; stock INVARIANTS kernel panics earlier at a krealloc KKASSERT
in the same device-driven path).**

## Line-accurate analysis (verified mechanically)

- Success path: when a VPD-R item completes, `pci.c:1223-1228` (fixed
  tree line numbers shifted +6) SHRINKS `vpd_ros` to exactly `off`
  entries via `krealloc(..., off * sizeof(struct vpd_readonly),
  M_DEVBUF, M_WAITOK | M_ZERO)` — 2 keywords => 128 -> 32 bytes. Every
  kept entry has a non-NULL `.value`, and the zero padding that used to
  act as the NULL sentinel is gone (krealloc 128->32 crosses size
  classes: kern_slaballoc.c krealloc allocates a fresh 32-byte chunk,
  bcopy(32), frees the 128-byte chunk). Same for `vpd_w` (pci.c:1278).
- Error path: `pci.c:1297-1304` runs when `cksumvalid == 0 || state <
  -1` and walks `for (off = 0; cfg->vpd.vpd_ros[off].value; off++)
  kfree(...)` — a NULL-sentinel scan with NO bound (pci.c:1300).
- **Correction to the finding's exact trigger**: a *second VPD-R item
  with a bad checksum* does NOT reach the shrunk array — case 0x10
  (pci.c:1117-1124) unconditionally kmallocs a FRESH zeroed 8-entry
  array (replacing and LEAKING the shrunk one), so the cleanup walk
  stops at the fresh array's zero sentinel. Proven mechanically by the
  `vpd badck2` run (rig/natural_5runs.log tail + badck2.log): bounded
  walk, no corruption, 3 leaked blocks (shrunk array + 2 keyword
  values).
- **The real trigger is the I/O-error path**: `state < -1` (VPD
  register-read timeout, pci.c:1073-1076 -> pci_read_vpd_reg ENXIO
  after 1M completion polls) ALSO enters the same cleanup block, and
  it hits whatever `vpd_ros` currently is — including a successfully
  SHRUNK array whose entries are all non-NULL. The malicious device
  serves one well-formed VPD-R item, then stalls the VPD address
  register's completion bit.

## What was run (all through the verbatim pci_read_vpd)

See README.md for build/run. Runs and logs:

| run | kernel | result |
|-----|--------|--------|
| `vpd stock` (shrink then stall) | DFPCI (stock, INVARIANTS) | **panic** `assertion "(flags & M_ZERO) == 0" failed in krealloc` at kern_slaballoc.c:1187, called from the shrink at pci.c:1225 (log shows `krealloc(...,32) flags=0x102 ENTER` then panic; trace krealloc <- dfh_krealloc <- dfh_run_vpd). panic_stock.txt |
| `vpd errfirst` (error, no prior shrink) | DFPCI | clean: fresh zeroed array, walk stops at sentinel, guest up (control) |
| `vpd natural` x5 (shrink then stall) | DFPCIP (production config, no INVARIANTS) | shrink runs (`krealloc -> new ptr`), stall -> state=-2, cleanup walks the shrunk array: 2 valid kfrees then reads entry[1].value OOB. In all 5 runs the adjacent 32-byte-zone memory was ZERO -> walk stopped benign. natural_5runs.log (snapshots logged) |
| `vpd groom` (adjacency controlled) | DFPCIP | **corruption**: shrink landed on the prepared chunk X; the walk read victim+8 (entry[1].value, i.e. ros+40 — 8 bytes past the 32-byte allocation) and `kfree()`d the LIVE 64-byte canary (logged `[*** UNTRACKED -- NOT OURS ***]`), stopped at victim+24 == 0, freed the array. Immediately after, `kmalloc(64)` returned the canary address and the harness wrote 0xAA through the new owner while the original owner still held it: **TWO LIVE OWNERS, SAME MEMORY**. Guest stayed up (silent heap corruption). groom.log |
| `vpd wild` (garbage pointer 0x4141414141414141 at victim+8) | DFPCIP | `kfree(wild)` -> `pmap_kvtom` page fault -> **panic: page fault**, trace `pmap_kvtom <- dfh_run_vpd` (the verbatim pci_read_vpd inlined). panic_wild.txt |
| `vpd badck2` (finding's claimed trigger) | DFPCIP | no corruption; fresh-array walk + 3-block leak (claimed mechanism refuted; real mechanism above) |
| `vpd ok` (success control) | DFPCIP | arrays kept, `ros[0] kw=PN val='XY'` readable, no cleanup |
| all of the above scenarios | DFPCIPF (production + fix.diff) | **no OOB**: cleanup bounded by `vpd_rocnt`; groom leaves the canary untouched (`kmalloc(64) -> (other); canary bytes now cc...`), 0 untracked frees. fixed_runs.log |
| `vpd stock` | DFPCIF (INVARIANTS + fix.diff) | **no panic**: fix also drops the unsupported `M_ZERO` from the 4 krealloc calls (krealloc KKASSERTs M_ZERO at kern_slaballoc.c:1187 — INVARIANTS builds), so the same scenario that panicked the stock kernel now completes with a bounded cleanup, guest up |

## Impact quantification on this kernel (team directive)

- Stock INVARIANTS kernel (as shipped in this dev tree): the claimed
  OOB-walk corruption is UNREACHABLE — every krealloc with M_ZERO
  (grow at pci.c:1150/1241, shrink at 1225/1280) panics FIRST with the
  KKASSERT. That is itself a device-triggerable kernel panic
  (any VPD-R item that completes, i.e. nearly every real VPD), but it
  is a different failure than claimed.
- Production-config kernel (INVARIANTS off — how release kernels
  build): the OOB walk is real. Natural runs stopped benignly only
  because the adjacent heap was zeroed at that moment; the groom run
  shows deterministic attacker influence over the freed pointer
  (device-controlled parse allocations and timing shape the 16/32/64
  byte zones at enumeration). Consequences observed:
  * kfree of a valid-looking LIVE object: SILENT — free lists corrupted,
    same chunk handed to a new owner while the old owner keeps it
    (overlapping live objects — classic exploitable slab corruption).
  * kfree of a wild pointer: kernel page fault -> panic.
  Not caught by any zone check on a non-INVARIANTS build (chunk
  integrity checks are INVARIANTS-only in kern_slaballoc.c).
- Threat model: malicious PCIe device with a VPD capability (AV:P /
  crafted passthrough). The lazy pci_read_vpd caller set in-tree is
  small (vga_pci.c:544, if_age.c), but the parse runs with device data
  only.

## Fix (validated A/B)

`fix.diff` (pack rig/fix.diff, hunks 4-11 of the combined diff):
1. Track the completed-entry count as entries complete
   (`vpd_rocnt = ++off` in state 3, `vpd_wcnt` in state 6) and reset
   the counts when a new VPD-R/VPD-W item starts (case 0x10/0x11).
2. Bound both error-cleanup loops by the count
   (`for (off = 0; off < cfg->vpd.vpd_rocnt; off++)`), matching the
   count-bounded convention already used by the device-teardown free
   path (pci.c:2146-2153) — the NULL-sentinel scan is then gone.
3. Drop `M_ZERO` from the four krealloc calls (krealloc does not
   support it — KKASSERT on INVARIANTS builds; and the sentinel is no
   longer needed once cleanups are count-bounded; when krealloc
   reallocates it passes flags through to _kmalloc which handles
   M_ZERO anyway, but the assert makes the flag unusable as shipped).

A/B: DFPCIPF (production+fix) — all scenarios re-run: 0 untracked
frees, canary intact in the groom run, success/control semantics
unchanged. DFPCIF (INVARIANTS+fix) — `vpd stock` no longer panics at
the krealloc assert.

## 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.)