# 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_delay`s 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.