DF-3827 — e1000 HAL: unchecked PHY register read feeds stale data into a write-back (e1000_set_d0_lplu_state_82575)

What

sys/dev/netif/ig_hal/e1000_82575.c:833-837, inside the active == TRUE branch of e1000_set_d0_lplu_state_82575():

833     ret_val = phy->ops.read_reg(hw, IGP01E1000_PHY_PORT_CONFIG,
834                     &data);
835     data &= ~IGP01E1000_PSCFR_SMART_SPEED;      /* ret_val NOT checked */
836     ret_val = phy->ops.write_reg(hw, IGP01E1000_PHY_PORT_CONFIG,
837                      data);              /* overwrites it   */

The return value of the read at 833 is clobbered by the write at 836 without ever being tested. If the PHY read fails (MDIO timeout, PHY in reset, contention — exactly the error family of DF-3643), data still holds the IGP02E1000_PHY_POWER_MGMT value read at 821, and that stale 16-bit value — masked with a SmartSpeed bit belonging to a different register — is written into IGP01E1000_PHY_PORT_CONFIG.

Both sibling branches of the same function check their reads correctly (851-855 and 864-868: if (ret_val) goto out;), which confirms the 833 instance is an omission, not a design choice.

Impact

PHY misconfiguration only (register-level corruption of the port-config register on IGP03E/IGP04E PHYs during LPLU power-state transitions); no kernel memory-safety impact. Requires the specific PHYs (phy->id == IGP03E1000_E_PHY_ID || IGP04E1000_E_PHY_ID, dispatched at 286-295) and a failing MDIO read at that instant. Severity Low; this is the 82575-family twin of the ich8lan unchecked-PHY-read finding (DF-3643 family) named in the audit checklist (item b).

Reproduce

Not executable on the audit guest: the function is a phy->ops callback of the igb(4)/HAL stack and requires an attached Intel 82575/i350-class device with an IGP03/IGP04 PHY. The guest has no such device (pciconf -l in env.txt shows virtio-only). Verdict: not_reproduced, defect certain at source level.

Fix

fix.diff adds the missing if (ret_val) goto out; between read and write-back, mirroring the two sibling branches.

DF-3827 VERDICT

Bottom line

Source-certain unchecked-return defect; kernel-path reproduction impossible on this guest (no Intel PHY hardware); not_reproduced.

Trace

e1000_set_d0_lplu_state_82575() — sys/dev/netif/ig_hal/e1000_82575.c:

• 821-823: read IGP02E1000_PHY_POWER_MGMT into data, checked.
• active branch:
• 827-830: write back POWER_MGMT with D0_LPLU set, checked.
• 833-834: read IGP01E1000_PHY_PORT_CONFIG into data — return value never inspected.
• 835: data &= ~IGP01E1000_PSCFR_SMART_SPEED;
• 836-837: write data to IGP01E1000_PHY_PORT_CONFIG (return value of the read is overwritten by the write's).
• !active branch: both reads (851, 864) are checked with if (ret_val) goto out;.

On a failed read at 833 the callee (e1000_read_phy_reg_igp / e1000_read_phy_reg_mdic via the dispatch at 220-222) leaves data untouched (and typically returns -E1000_ERR_PHY / MDIC ready-timeout), so the stale POWER_MGMT value from 821 is masked and committed to PORT_CONFIG. That register controls interface mode / SmartSpeed for the IGP03/IGP04 PHYs; corrupting it during an LPLU transition degrades or drops the link until the next PHY reset. No kernel memory is affected — this is a config-integrity bug on the device, the same class as the ich8lan unchecked-PHY-read family (DF-3643), hunted here per checklist item (b).

Why not reproduced

The path requires: (1) an attached 82575/82576/i350-class NIC (dispatch e1000_init_function_pointers_82575 only runs from the igb(4) driver's device attach), (2) phy->id == IGP03E1000/IGP04E1000 (case at 286-295), (3) an MDIO read failure at exactly that call. The audit guest's PCI bus carries only virtio-net/blk and a bochs VGA (env.txt); igb never attaches, so phy->ops.set_d0_lplu_state is never reachable. One attempt: verified device absence + full source trace. Status not_reproduced, impact none (observed); theoretical impact = transient link failure on real hardware (availability of one link, recoverable) → severity Low, confidence certain.

Checked and killed nearby (negative results with weight)

• 254-279: M88E1112/1512/1543 init reads all check ret_val before use.
• 1243-1283 (e1000_check_for_link_media_swap): every read/write checked; the ignored return of e1000_check_for_link_82575 at 1277/1279 only affects a debug state, harmless.
• e1000_set_eee_i354 (3073-3145) / e1000_get_eee_status_i354 (3155-3180): all xmdio reads checked; *status in get_eee_status is left untouched on early-out (callers initialize), no stale-writeback.
• The I2C bit-bang helpers (3430-3777) return status per-step and are all checked by their callers (3291-3345).

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.