DF-4638 — ig_hal: posted mailbox wrappers bypass the size clamps their non-posted siblings enforce

File: sys/dev/netif/ig_hal/e1000_mbx.c (pass 2, GLM 5.3)

What

• e1000_read_mbx() (e1000_mbx.c:80-81) clamps size down to mbx->size before dispatching to ops.read — this is what prevents ops.read (the for (i = 0; i < size; i++) msg[i] = E1000_READ_REG_ARRAY(...) loop at e1000_mbx.c:736-737 / :512-513) from walking past the caller's buffer.
• e1000_write_mbx() (e1000_mbx.c:105-106) rejects size > mbx->size.
• The posted variants call ops.read/ops.write directly, with no size handling at all: e1000_read_posted_mbx() at e1000_mbx.c:260 and e1000_write_posted_mbx() at e1000_mbx.c:287.

Today every in-tree caller (all in e1000_vf.c: :283/:288/:334/:337/:385-388/:436/ :498/:501/:559) passes size ∈ {1,2,3,16} ≤ mbx->size = 16 (E1000_VFMAILBOX_SIZE, set at e1000_mbx.c:540/:766), with correctly sized stack buffers — so there is no demonstrable overflow on the current tree. The hazard is latent: the VF mailbox API is live code (the igb driver attaches to 82576_VF/I350_VF PCI IDs — if_igb.c:108/:123 — and dispatches through e1000_api.c:511/:514 to e1000_init_function_pointers_vf), and any future caller of the posted API with size > mbx->size gets a kernel stack/heap overflow filled with PF-controlled mailbox words, plus out-of-range E1000_VMBMEM register-array writes.

Severity: Info (defense-in-depth / API hardening). Confidence: certain (the missing checks are source facts; the impact statement is explicitly latent).

Reproduce

Nothing to run — this is a hardening gap. The fix was compile-validated on the audit guest against the running kernel's headers:

# build.log (guest session):
cc -fsyntax-only -D_KERNEL -I/usr/obj/usr/src/sys/X86_64_GENERIC \
   -I/usr/src/sys -I/usr/src/sys/dev/netif/ig_hal e1000_mbx_fixed.c   # FIXED_CC_RC=0

fix.diff mirrors the sibling checks into the posted wrappers (read clamps, write rejects) — git-apply-clean against sys/.

DF-4638 VERDICT — posted mailbox wrappers bypass the size clamps

Status: untested (nothing runnable: no igb/SR-IOV hardware on the audit guest — env.txt; and there is no in-tree caller that overflows today, by design of the finding). Fix: compile-validated on guest (FIXED_CC_RC=0, BASE_CC_RC=0 — build.log).

Analysis

The non-posted wrappers implement the mailbox size discipline:

• e1000_read_mbx (e1000_mbx.c:72-87): if (size > mbx->size) size = mbx->size; (:80-81) — protects the destination buffer, since the per-MAC ops.read implementations copy size words unconditionally (e1000_read_mbx_vf :512-513, e1000_read_mbx_pf :736-737).
• e1000_write_mbx (e1000_mbx.c:98-112): if (size > mbx->size) ret_val = -E1000_ERR_MBX; (:105-106) — protects both the source buffer read (msg[i] in e1000_write_mbx_vf :476-477 / _pf :697-698) and the E1000_VMBMEM(_n) + i register-array window (16 words per VF, e1000_regs.h:589).

The posted wrappers dispatch around that discipline:

• e1000_read_posted_mbx (:246-263): after e1000_poll_for_msg succeeds, calls mbx->ops.read(hw, msg, size, mbx_id) (:260) with the caller's size unmodified — no clamp.
• e1000_write_posted_mbx (:275-294): checks !mbx->ops.write || !mbx->timeout (:283) then calls mbx->ops.write(hw, msg, size, mbx_id) (:287) — no size check.

Reachability today (why Info, not higher)

All in-tree callers of the posted API are in sys/dev/netif/ig_hal/e1000_vf.c: write/read 1 (:283/:288), 3 (:334/:337), 16 via e1000_write_msg_read_ack (:385-388, retmsg[E1000_VFMAILBOX_SIZE]), 16 (:436), 1 (:498/:501); plus one non-posted ops.read(hw, &in_msg, 1, 0) (:559). Every size ≤ mbx->size = 16 (e1000_mbx.h:62; installed at e1000_mbx.c:540/:766), every buffer correctly sized. No overflow is constructible on the current tree — the gap is a loaded footgun on a live API (igb's VF attach path: if_igb.c:108/:123 → e1000_api.c:511/:514 → e1000_vf.c:151 → e1000_mbx.c:542-545), aimed at exactly the "trust the VF/PF message size" family. A future caller passing a size derived from a message field or a larger constant gets a kernel-memory overflow whose content is PF-controlled mailbox words.

Note the API inconsistency is also a correctness trap in the other direction: e1000_read_mbx silently truncates while e1000_write_mbx errors — the posted variants do neither.

Classes hunted and killed on this file (pass-2 negative result)

• e1000_poll_for_msg/e1000_poll_for_ack (:181-234): countdown = mbx->timeout (0 or 2000, u32 in e1000_hw.h:983 — no sign issue); bounded 2000 × 500 µs; on exhaustion mbx->timeout latches 0 = permanent-fail-until-reset (conservative, not a bypass).
• e1000_obtain_mbx_lock_vf/pf (:426-446/:644-668): bounded 10 × 1 ms.
• e1000_check_for_msg_pf/check_for_ack_pf (:585/:606): E1000_MBVFICR_VFREQ_VF1 << vf_number / E1000_MBVFICR_VFACK_VF1 << vf_number is UB for vf_number ≥ 32, but the PF ops have no in-tree caller (DFly igb has no PF-side SR-IOV) — dead code; would deserve a bound (vf_number = vf_number % 32) the day a PF driver appears.
• e1000_init_mbx_params_pf (:755-784): missing break before default: is cosmetic (case body completes then falls into return E1000_SUCCESS); unsupported MACs leave ops NULL-zeroed and every dispatcher NULL-checks (:83, :108, :128, :148, :168, :253, :283).
• e1000_read_v2p_mailbox/check_for_bit_vf (:322-351): shadow-register merge is idempotent; only reachable on VF dispatch where dev_spec.vf is the valid union member.
• No fw/VF-controlled count reaches any loop bound in this file — all sizes are caller-supplied (that is precisely why the missing caller-side discipline matters).

Fix

fix.diff (git-apply-clean; compile-validated on the guest, build.log): mirror the sibling checks into the posted wrappers — read clamps to mbx->size, write rejects size > mbx->size.

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.