DF-3642 — e1000_erase_flash_bank_ich8lan: flash address accumulates across erase retries; erase failures reported as success

What / where

sys/dev/netif/ig_hal/e1000_ich8lan.c:e1000_erase_flash_bank_ich8lan (lines 4692-4802), the ICH8/PCH hardware-sequencing flash bank erase used by every NVM (EEPROM-image) commit on em(4)-driven Intel GbE MACs (ICH8..I219). Three coupled defects in the retry loop:

1. Address accumulation (4779): flash_linear_addr += (j * sector_size); sits inside the do { ... } while (++count < ICH_FLASH_CYCLE_REPEAT_COUNT) retry loop instead of the outer for (j ...) loop. Every retry after a flash-cycle error (flcerr → continue) issues the next erase at a higher address: base + n·j·sector_size after n retries. With 256-byte sectors (iteration = 16) the drift reaches ~10·Σj·256 ≈ up to tens of KB past the intended sector, i.e. erase commands against the wrong sectors of the flash (both NVM banks, and the tail of whatever the GbE flash-region grant covers).
2. Retry budget never reset per sector (4798 / 4701): count is a function-scope variable; the 10-retry budget is shared across all j iterations, so later sectors can silently get zero retries.
3. Unconditional success (4801): when the retry budget is exhausted via the flcerr path, the do/while simply falls through and the function returns E1000_SUCCESS even though the last erase failed; the caller then programs dwords into an un-erased bank (the subsequent write failures cascade through retry_write_flash_*).

How it is reached (no user syscall needed)

em_attach → e1000_validate_nvm_checksum (if_em.c:781, retried at :787) → hw->nvm.ops.validate = e1000_validate_nvm_checksum_ich8lan (e1000_ich8lan.c:4364) → if the NVM image lacks the VALID_CSUM bit (:4395), it writes and updates the NVM (:4397-4400) → e1000_update_nvm_checksum_ich8lan (:4214) → e1000_erase_flash_bank_ich8lan(hw, bank) (:4246 / :4252).

So the erase engine runs at driver attach whenever the card's NVM content lacks the checksum-valid bit — a malicious/counterfeit PCIe GbE device, a bit-corrupted NVM, or a reflashed card is sufficient; no local privilege is required on the host.

Impact

Flash-content corruption outside the intended bank (wrong sectors erased → invalid NVM image / bricked NIC config; bounded by the PCH flash-controller region grants, so the BIOS/ME regions stay protected on conforming parts), plus wasted write cycles into an un-erased bank. Availability/integrity of the device, not kernel memory. Severity: Low (hardware/NVM-content precondition).

Reproduce (source-level)

sh findings/poc/DF-3642/trace.sh        # regenerates trace.log with all cited lines
git apply --check findings/poc/DF-3642/fix.diff

Runtime reproduction requires physical Intel ICH8/PCH GbE MAC hardware whose flash reports cycle errors; the audit QEMU guest has only virtio NICs (see guest_env.txt), so the defect is proven at source level and confirmed byte-identical in FreeBSD HEAD (upstream-inherited).

Expected result after fix

Each erase command targets bank_base_addr + j*sector_size regardless of retry count; a sector whose retry budget is exhausted returns -E1000_ERR_NVM and aborts the commit instead of silently "succeeding".

VERDICT — DF-3642

Status: not_reproduced (runtime) — defect certain (source), upstream-inherited.

What I did

1. Read the full 6,117-line file and traced every caller chain into e1000_erase_flash_bank_ich8lan (see README.md chain: if_em.c:781/787 → e1000_api.c:1270 → e1000_ich8lan.c:4395-4403 → :4214 → :4246/4252).
2. Proved the three code defects from the source (e1000_ich8lan.c:4779, :4701+:4798, :4801) — full extracted lines in trace.log.
3. Verified DragonFly made no local edits here: diffed against FreeBSD stable/12 (/tmp/opencode/dfly_vs_fs12.diff) — all non-whitespace deltas are upstream additions DragonFly lacks (PCH tgp/adp/mtp/ptp cases, k0s config).
4. Fetched FreeBSD HEAD (/tmp/opencode/fs_ich8lan.c, main branch 2026-09-15): the accumulation bug is still present at line 4931 — vendor-inherited and un-fixed upstream; a DragonFly-side fix is legitimately ours to contribute.
5. Guest verification (guest_env.txt): em(4)/ig_hal is compiled into X86_64_GENERIC, but the guest has no Intel GbE MAC (virtio NIC only, pciconf -l shows i440FX bridge devices 0x1237/0x7000/0x7010/0x7113, none are e1000 LAN). em_attach never matches, so the flash-cycle code cannot execute on this guest. A hardware-in-the-loop rig (real I217/I218/I219 or an FPGA PCIe card emulating one with a flaky flash controller) would be required to observe the wrong-sector erase at runtime.

Why not false-positive

The control flow is unambiguous: the += executes on every pass of the inner do/while (including every continue retry), count is initialized once at declaration (:4701) and never reset inside the for (j ...) loop, and the only return paths before :4801 are (a) flash_cycle_init failure and (b) the !flcdone timeout — exhausting the budget via the flcerr/continue path falls through to return E1000_SUCCESS. No interpretation required.

Impact assessment (honest ceiling)

• Not kernel memory corruption: all writes go to the device register FADDR; the flash controller enforces region grants, so the BIOS/ME regions are not reachable by the GbE master on conforming parts.
• Realistic ceiling: NVM image corruption / NIC config bricking during an attach-time repair flow on hardware whose flash reports cycle errors (failing/counterfeit part), reachable from NVM content alone.
• Precondition strength is why this is Low, not Medium: needs a specific physical device state, not a local user.

Fix

fix.diff (git-apply-able, git apply --check verified against the read-only tree): pin bank_base_addr once, recompute the address per attempt, reset the retry budget per sector, and propagate erase failure instead of returning success. Fix validation on hardware was not possible on this guest (fix_status: not_testable; baseline could not be reproduced either).

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.