DF-4418 — e1000_82541: unchecked read_reg() in e1000_phy_init_script_82541() → uninitialized stack written to PHY registers

• File: sys/dev/netif/ig_hal/e1000_82541.c:1108,1153,1159-1164,1180-1182
• Severity: Low · CWE: CWE-457 · Confidence: certain (defect)
• Status: untested — no Intel 82541/82547 NIC in the audit QEMU guest; trigger additionally requires a failing MDIO read at reset time.

Build / Run / Expected

Requires 82541/82547 hardware with a faulting MDIO bus during e1000_reset_hw_82541()/e1000_phy_hw_reset_82541(). Baseline: init script continues after failed read and rewrites PHY reg 0x2F5B with uninitialized stack. Patched: init script aborts to out: returning the read error. See VERDICT.md (also contains the full pass-2 family checklist for this file).

Artifacts

VERDICT.md · verdict.json · fix.diff · manifest.json

DF-4418 — e1000_82541: e1000_phy_init_script_82541() ignores read_reg() failures and writes uninitialized stack into PHY registers

Verdict (short)

Status: untested — triggering requires a real 82541/82547 NIC whose MDIO read fails at init-script time (failing/flaky PHY); the audit QEMU guest has no Intel 82541 hardware. Defect is certain from source; impact bounded to writing an uninitialized kernel-stack u16 into a PHY management register.

Root cause (path:line)

sys/dev/netif/ig_hal/e1000_82541.c:1088-1191 (e1000_phy_init_script_82541):

1. Line 1108: ret_val = hw->phy.ops.read_reg(hw, 0x2F5B, &phy_saved_data); — the return value is stored but never checked. If the MDIO read fails, phy_saved_data stays uninitialized, the script proceeds through a dozen unconditional PHY writes, and at line 1153 hw->phy.ops.write_reg(hw, 0x2F5B, phy_saved_data); writes the uninitialized kernel-stack value into PHY register 0x2F5B (analog/dsp control register).

2. Lines 1159-1164 (82547 analog-fuse block): both hw->phy.ops.read_reg(hw, IGP01E1000_ANALOG_SPARE_FUSE_STATUS, &fused); and the follow-on ANALOG_FUSE_STATUS read discard their return values entirely; on failure fused (u16, uninitialized stack) drives the fused & IGP01E1000_ANALOG_SPARE_FUSE_ENABLED branch and is written back into IGP01E1000_ANALOG_FUSE_CONTROL at lines 1180-1182.

Callers: e1000_reset_hw_82541() (e1000_82541.c:346) and e1000_phy_hw_reset_82541() (e1000_82541.c:507) — i.e. every device reset on 82541/82547 (early revs run the script via dev_spec->phy_init_script, enabled by the shared-code default e1000_init_script_state_82541).

Impact analysis

• The uninitialized value goes to hardware MMIO (PHY register), not to userspace — no direct kernel-memory disclosure. (Read-back of PHY regs via driver sysctls would require the MDIO bus to be working, contradicting the failure precondition.)
• Realistic worst case: garbage analog/dsp configuration after a transient MDIO failure → degraded/dead link until next successful reset.
• Trigger requires physical-hardware fault conditions on an 82541/82547 (2003-era parts); not attacker-reachable remotely.

Severity Low / CWE-457 (use of uninitialized variable).

Family checklist results for e1000_82541.c (pass 2)

• (a) function-pointer completeness: OK — e1000_setup_init_funcs (e1000_api.c:447-450) installs null-op defaults (e1000_init_phy_ops_generic, e1000_phy.c:73-98) before family init, so unset ops (e.g. check_reset_block) are never NULL.
• (b) reset/wait loops: fixed msec_delays only (e1000_82541.c:310,318,338); e1000_phy_has_link_generic(hw, 1, 0, &link) at :605 bounded.
• (c) LED setup: no blink timers; plain LEDCTL RMW (:349-352, 563-566, 1051, 1076) and GMII_FIFO save/restore (:1040-1072) — checked, returns propagated.
• (d) e1000_reset_hw_82541 — no SWFW/semaphore ops at all on 82541 (predates SWFW_SYNC); no DF-4306 unbounded-release pattern. Clean.
• (e) NVM size derivation (:181-193): size = (word & 0x1C00) >> 10 (3 bits, ≤7), 1 << (7+7) = 16384 words max — bounded, no shift overflow.
• (f) e1000_get_cable_length_igp_82541 (:869-926): table = 128 entries; cur_agc_value == 0 || >= 127 rejected (:892-896); worst final index = (4*126)/4 = 126 < 128 and (199-1)/3 = 66 < 128 — in-bounds. Clean. FFE loop (:730-752): 20 or 100 iterations × 1 ms — bounded.
• e1000_config_dsp_after_link_change_82541 state machine: dsp/ffe transitions are single-direction per call; phy_saved_data is checked before use (:759-762, :817-820). Clean apart from the file-level finding above.

Fix

findings/poc/DF-4418/fix.diff — check the three read_reg() return values whose failures leave phy_saved_data/fused uninitialized, and bail to out: (which returns ret_val, so the caller still sees the error).

Reproduce (on hardware)

1. Machine with an 82541EI/ER/GI/PI or 82547 NIC (ig_hal-driven em).
2. Force an MDIO read failure during reset (e.g. bus analyzer injecting NACK, or a PHY held in reset): the first read_reg(0x2F5B) in the init script returns non-zero, the script continues, and register 0x2F5B is rewritten with the previous stack contents. With the patch applied, reset aborts cleanly with the read error.

Not runnable in this audit environment (no such NIC); status untested.

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.