# DF-3641 — bnx_sysctl_std_refill() iterates bnx_tx_ringcnt while indexing bnx_rx_ret_ring[]

**File:** `sys/dev/netif/bnx/if_bnx.c` (pass 2, GLM 5.3)
**Class:** copy-paste loop-bound defect (CWE-193-adjacent; here correctness only)
**Severity filed:** Info, bucket `base:dev`, confidence certain (source fact)
**Verdict:** `inconclusive` at runtime — sysctl node requires attached bnx
hardware, absent on the audit guest.

## 1. Root cause (path:line)

`bnx_sysctl_std_refill()` — the handler for `dev.bnx.N.std_refill`:

- `sys/dev/netif/bnx/if_bnx.c:5669` — reads
  `ret = &sc->bnx_rx_ret_ring[0]` (an **RX** return ring)
- `sys/dev/netif/bnx/if_bnx.c:5679` — validates
  `(cntmax * sc->bnx_rx_retcnt) >= BGE_STD_RX_RING_CNT / 2` → EINVAL
- `sys/dev/netif/bnx/if_bnx.c:5684` —
  `for (i = 0; i < sc->bnx_tx_ringcnt; ++i)` ← **wrong count**
- `sys/dev/netif/bnx/if_bnx.c:5685` —
  `sc->bnx_rx_ret_ring[i].bnx_rx_cntmax = cntmax;`

The loop must run over `sc->bnx_rx_retcnt`. On the common BCM5719/5720 RSS
setup with `rx_retcnt > 1` and `tx_ringcnt == 1` (the non-bundled topology;
`bnx_setup_ring_cnt()` allows tx counts from 1..min(4, rx_retcnt)), a write to
`dev.bnx.N.std_refill` updates only ring 0 — rings 1..rx_retcnt-1 silently
keep the default `bnx_rx_cntmax` assigned in `bnx_dma_alloc()`
(`if_bnx.c:4259-4260`: `(BGE_STD_RX_RING_CNT / 4) / sc->bnx_rx_retcnt`), so
the operator-visible value disagrees with the per-ring refill thresholds
actually in force (used at `if_bnx.c:2825`, `ret->bnx_rx_cnt >=
ret->bnx_rx_cntmax`).

## 2. Not memory-unsafe (why Info)

`tx_ringcnt <= rx_retcnt` is structurally guaranteed before any of this code
runs: `bnx_setup_ring_cnt()` (`if_bnx.c:5784-5799`) clamps the TX ring map to
`min(BNX_TX_RING_MAX, rx_retcnt)` and `if_ringcnt_fixup()`
(`sys/net/if.c:3944-3953`) clamps the count; the KASSERT at `if_bnx.c:5797`
documents the invariant (`if_ringmap_align()` only equalizes CPU grids, not
counts — verified in `sys/net/if.c:4029-4036`). The loop therefore can never
run past the `bnx_rx_ret_ring[]` allocation.

Related wart (also fixed in fix.diff): the validation at 5679 admits negative
`cntmax` (`n * retcnt` is negative, never `>= 256`), which then truncates into
the `uint16_t bnx_rx_cntmax` (`if_bnxvar.h:204`) — e.g. −5 → 65531, making
the `bnx_rx_cnt >= bnx_rx_cntmax` test at 2825 almost never fire (refill
scheduling from that path stops; RX still refills via the independent
`bnx_rx_std_used >= BGE_STD_RX_RING_CNT/2` check at 2890). Root-only sysctl,
performance/correctness only.

## 3. Guest evidence (why `inconclusive`)

run.log (guest, byte-identical source):
- `pci/if_bnx` in kernel; `NO_BROADCOM_DEVICE`; `ifconfig -l` = vtnet0 lo0;
  `sysctl dev.bnx.0.std_refill` → unknown oid → the handler never executes
  on this guest.
- Guest-side dump of lines 5679-5686 shows the mismatch verbatim.
- `md5 if_bnx.c` == audited tree.

trigger.c probes `dev.bnx.0.std_refill` via sysctlbyname and correctly
reports it absent on the guest → honest `inconclusive` / `reproduced: 0`.

## 4. Fix

`fix.diff` — iterate `sc->bnx_rx_retcnt` and reject `cntmax < 1`.
git-apply-able (verified). Not runtime-validatable on this guest
(`fix_status: not_testable`).

## 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.