DF-3826 — e1000 HAL: double release of the SW_FW_SYNC PHY0 semaphore in e1000_read_i2c_byte_generic

What

sys/dev/netif/ig_hal/e1000_82575.c, e1000_read_i2c_byte_generic() (lines 3270-3349): when every I2C transfer attempt fails and the retry loop is exhausted, the fail: label (line 3333) releases the E1000_SWFW_PHY0_SM semaphore for each failed attempt, and then the unconditional post-loop release at line 3344 releases it once more — releases == acquires + 1. The sibling writer, e1000_write_i2c_byte_generic() (lines 3361-3422), acquires once before the loop, does not release inside fail:, and releases once after the loop — proving the read path's asymmetry is a defect, not a protocol choice.

Impact

The extra e1000_release_swfw_sync_82575(hw, E1000_SWFW_PHY0_SM) clears the PHY0 bit in E1000_SW_FW_SYNC at a moment when this code path no longer owns it. The two releases are separated by ~100 ms (msec_delay(100) at 3334 + e1000_i2c_bus_clear bit-banging at 3335). If, in that window, another function (multi-port i350 path, NVM/EEPROM path via e1000_acquire_nvm_82575, or management firmware) legitimately acquires PHY0, its ownership bit is silently dropped and a second party can then acquire the "same" resource → concurrent PHY register / I2C access, torn MDIO transfers, possible EEPROM/NVM access corruption. Logic/lifetime defect in the SW/FW semaphore protocol; not kernel memory corruption.

Hardware-gated: reachable only on real 82575/82576/82580/i350/i354/i210 silicon with a failing/unseated SFP or wedged I2C bus (the read is used by SFP detection, e1000_set_sfp_media_type_82575).

Reproduce (logic-level replica — no hardware required)

scp swfw_double_release.c dfbsd-maxx:/tmp/
ssh dfbsd 'cc -O2 -Wall -o /tmp/swfw_double_release /tmp/swfw_double_release.c'
ssh dfbsd '/tmp/swfw_double_release; echo exit=$?'

Expected (see run.log):

case1 (all I2C ops fail, retries exhausted): acquires=9 releases=10 -> DOUBLE RELEASE
case2 (first attempt succeeds): acquires=1 releases=1 -> balanced
case3 (acquire fails): acquires=0 releases=0 -> balanced
exit=0

swfw_double_release.c replicates the exact control flow of e1000_read_i2c_byte_generic() (retry starts at 1, while (retry < 10), release in fail:, unconditional release after the loop) with mocked acquire/release; case 1 shows 9 successful acquires and 10 releases — the kernel loop arithmetic is identical.

Kernel-path verification status

The KVM audit guest has no Intel 8257x-class NIC (pciconf -l: virtio-net + virtio-blk + bochs VGA only, see env.txt), so the in-kernel path cannot be executed there — verdict not_reproduced for the kernel reachability; the control-flow defect itself is proven at source level (certain) and by the replica.

Fix

fix.diff — only the post-loop release when the loop was exited via break (semaphore still held). Never applied to sys/ (read-only audit).

DF-3826 VERDICT

Bottom line

Source-certain logic defect; kernel-path reproduction impossible on this guest (target hardware absent); logic-level reproduction succeeded. status: not_reproduced (kernel reach), impact: none (as observed), the defect itself is proven with certainty from the source and the state-machine replica.

The defect

e1000_read_i2c_byte_generic() in sys/dev/netif/ig_hal/e1000_82575.c:

3284    do {
3285        if (hw->mac.ops.acquire_swfw_sync(hw, swfw_mask) != E1000_SUCCESS) {
3287            status = E1000_ERR_SWFW_SYNC;
3288            goto read_byte_out;          <- (a) no release: correct
        }
        ... I2C steps, each "goto fail" on error ...
3330        break;                              <- (b) success: semaphore held
3332    fail:
3333        hw->mac.ops.release_swfw_sync(hw, swfw_mask);   <- release per failed attempt
        msec_delay(100);
        e1000_i2c_bus_clear(hw);
        retry++;
3342    } while (retry < max_retry);            <- exhausted: exit WITHOUT holding
3344    hw->mac.ops.release_swfw_sync(hw, swfw_mask);     <- ALWAYS releases again

With retry = 1 and max_retry = 10 there are exactly 9 in-loop attempts; each failure releases once (3333), and the exhausted exit falls into the unconditional release at 3344 → 10 releases for 9 acquires. Exit (a) (acquire failure) correctly skips the post-loop release via goto read_byte_out; exit (b) (success) needs the post-loop release — so the missing distinction is only for exit path "exhausted", which is precisely the bug.

Contrast: e1000_write_i2c_byte_generic() (3373: acquire once before loop; fail: at 3408 does NOT release; single release at 3417) — balanced by construction.

Why it matters (semaphore symmetry — checklist item (e))

e1000_acquire_swfw_sync_82575/release_swfw_sync_82575 (1052-1118) implement a bit-mask mutual-exclusion protocol over E1000_SW_FW_SYNC shared with management firmware: acquire = set bit while holding the global hw semaphore; release = clear bit. There are no reference counts. A release without ownership clears a bit that may have been legitimately re-acquired in the ≥100 ms window between the fail: release and the post-loop release (delay 3334 + nine I2C clock pulses of e1000_i2c_bus_clear, 3750-3777). The next acquire then succeeds for two concurrent parties → concurrent PHY/MDIO/I2C access, torn PHY register writes (e.g. the M88E1512/1543 multi-step init scripts at 2827-3007 each assume exclusive PHY access). Every other acquire/release pair in this file was checked and is balanced: e1000_acquire_nvm_82575 (991→1022 error-path release), e1000_reset_hw_82580 (2496→2545), the PHY wrappers (608-614, 641-647, 2376-2382, 2402-2408), and the two i210 variants live in e1000_i210.c.

Reproduction

• run.log: replica state machine — acquires=9 releases=10 -> DOUBLE RELEASE on the exhaustion path; both control cases balanced. Exit 0.
• Kernel path: guest pciconf -l shows no 8086:8257x/i350/i210 device and no igb interface (env.txt) → the HAL entry point cannot be reached on this guest. Real-hardware reproduction would need an igb-attached port with an SFP cage whose module read fails 9 consecutive times while another port holds/re-acquires PHY0 — a lab setup, not available here.

Classification per honest-status table

Guest reachable + target device absent → not_reproduced, reproduced=0, impact=none for the observed run. Theoretical impact on real hardware: loss of SW/FW mutual exclusion (logic, availability of the PHY path); no kernel memory-safety consequence → severity Low, confidence certain.

Fix validation

fix.diff gates the post-loop release on status == E1000_SUCCESS. Fix-kernel build validation is not meaningful for a hardware-gated HAL path on a guest without the device (nothing observable changes), so fix_status: not_testable with the one-line rationale; the logic replica was re-run against the patched control flow (post-loop release skipped when the last attempt failed): releases == acquires for all three cases.

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.