DF-4733 — ar5210_xmit: signed q<0 bypasses >=-only checks in ar5210Set/GetTxQueueProps; five exported xmit ops gate q only via inert HALASSERT

File: sys/dev/netif/ath/ath_hal/ar5210/ar5210_xmit.c Family: ar5210 twin of DF-4519 (ar5212, verified) and DF-4712 (ar5211) — signed-q + inert-HALASSERT. Severity: Low (latent; all in-tree callers pass validated 0..2) — certain on the code shape.

TL;DR

ar5210SetTxQueueProps/ar5210GetTxQueueProps take signed int q and check only q >= HAL_NUM_TX_QUEUES, so a negative q indexes ahp->ah_txq[q] below the 3-element array (ar5210.h:112, HAL_NUM_TX_QUEUES == 3 for 5210 at ar5210.h:96); ath_hal_setTxQProps (ah.c:951) then reads qi->tqi_type and writes ~15 fields through the OOB pointer. The five exported tx ops (ar5210GetTxDP:262, ar5210SetTxDP:287, ar5210StartTxDma:372, ar5210NumTxPending:406, ar5210StopTxDma:433) gate u_int q only via HALASSERT, which expands to nothing in production (ah_internal.h:669-676; AH_ASSERT is opt-in — sys/conf/options:549, absent from X86_64_GENERIC, verified on guest), leaving &ahp->ah_txq[q] OOB reads unvalidated.

No unprivileged trigger exists today: every in-tree caller passes driver-validated indices (if_ath.c:4013-4032 checks qnum == -1 and qnum >= nitems(sc->sc_txq); sc_bhalq likewise comes from ath_hal_setuptxqueue). Latent primitive for future/buggy HAL callers — same class the project already accepted fixes for in DF-4519/DF-4712.

Notable ar5210-specific deltas vs the 5211/5212 twins (why this is weaker than theirs): - No computed per-queue MMIO (AR_QTXDP(q) etc.) — 5210 uses fixed AR_TXDP0/AR_TXDP1 registers, so the assert-only ops expose only an ah_txq[q] struct OOB read, not OOB MMIO. - No DF-4520-style EEPROM-driven queue count: ar5210_attach.c:368 sets pCap->halTotalQueues = HAL_NUM_TX_QUEUES (constant 3), and ar5210SetupTxQueue picks fixed q ∈ {0,1,2}. - The 1 << q mask updates in ar5210ReleaseTxQueue:143-147 / ar5210ResetTxQueue:228-247 sit behind real runtime q >= HAL_NUM_TX_QUEUES checks — no UB shift.

Reproduce

Hardware-gated (no Atheros NIC on the guest — verified, pciconf -l shows zero ath devices; kld module compiled into kernel per kldstat -v). Source-only confirmation plus fix build-validation:

# on the guest (from this pack):
scp fix.diff guest:/tmp/df4733.diff
vm.sh run_root 'cd /usr/src && patch -p1 < /tmp/df4733.diff && make nativekernel KERNCONF=X86_64_GENERIC'
# expected: BUILD_RC=0, ar5210_xmit.c compiled -Werror (see build.log)

Fix

fix.diff (7 runtime q-checks: q < 0 || in the two signed props ops; runtime q >= HAL_NUM_TX_QUEUES checks returning 0xffffffff/AH_FALSE/0 in the five ops). Mirrors the accepted DF-4519/DF-4712 fix shape.

VERDICT — DF-4733

Finding

ar5210 twin of the signed-q / inert-HALASSERT family (DF-4519 ar5212 verified, DF-4712 ar5211). ar5210SetTxQueueProps (ar5210_xmit.c:40) and ar5210GetTxQueueProps (:56) take signed int q, check only q >= HAL_NUM_TX_QUEUES; for 5210 that constant is 3 (ar5210.h:96 — not the global 10) and ah_txq[HAL_NUM_TX_QUEUES] (ar5210.h:112) is a 3-element array inside the heap struct ath_hal_5210. Negative q → &ahp->ah_txq[q] → ath_hal_setTxQProps (ah.c:951) reads tqi_type then writes ~15 fields (aifs, cwmin, cwmax, retries, cbr fields, burst/ready times, intFlags) through the OOB pointer; ath_hal_getTxQProps (ah.c:1012) copies ~14 fields back. Five exported ops gate u_int q only via HALASSERT (GetTxDP:262, SetTxDP:287, StartTxDma:372, NumTxPending:406, StopTxDma:433) — HALASSERT is a no-op unless the opt-in kernel option AH_ASSERT is set (sys/conf/options:549; ah_internal.h:669-676). Verified on the guest: grep -c AH_ASSERT /usr/src/sys/config/X86_64_GENERIC → 0, so the production kernel on this config has zero runtime q validation for those five ops (ah_txq[q] struct OOB read of tqi_type). 1 << q shifts (ReleaseTxQueue:143-147, ResetTxQueue:228-247) are behind real checks; 5210 has no computed per-queue MMIO (fixed AR_TXDP0/AR_TXDP1) — the assert-only ops are weaker than the 5211/5212 twins (struct read only).

Verification (runtime: hardware-gated; fix: build-validated)

• No Atheros NIC on the guest (pciconf -l | grep -ci ath → 0). ath_hal is compiled into the kernel; no userland-reachable caller exists without the radio. Runtime reproduction of the OOB itself is therefore not possible on this guest — same honest position as DF-4519/DF-4712 ("hardware-gated", status untested).
• All in-tree callers pass validated indices: if_ath.c:4013-4032 (qnum == -1 checked, qnum >= nitems(sc->sc_txq) checked), if_ath_beacon.c:130-169 (sc_bhalq from setuptxqueue). Latent primitive only.
• Fix build-validated: fix.diff (7 runtime q-checks mirroring the accepted DF-4519/4712 shape) applied to guest /usr/src; make nativekernel KERNCONF=X86_64_GENERIC BUILD_RC=0 with -Werror; ar5210_xmit.c compiled into the kernel (build.log — mkdep at 5505, cc -Werror of ar5210_xmit.c confirmed).

Killed / negative candidates (ar5210_xmit.c, pass 2)

• ar5210SetupTxQueue q from fixed switch {0,1,2} — no input.
• DF-4520 twin (EEPROM halTotalQueues): absent — ar5210_attach.c:368 sets the constant; no unclamped copy.
• ar5210SetupTxDesc unmasked txRate0 << AR_XmitRate_S (line 507): AR_XmitRate is bits 18-21 (ar5210desc.h:43-44) and the 5210 rate tables (ar5210_phy.c:33-70) only contain 4-bit codes 0x08-0x0f, so no in-tree bleed; note isValidTxRate (line 470-474) also accepts 5-bit 11b codes 0x18-0x1e which WOULD bleed bit 22 (AR_RTSCTSEnable) — inert encoding inconsistency, unreachable (5210 is 11a-only; no 11b rate table exists). Not filed.
• ar5210FillTxDesc HALASSERT-only segLen check (line 557): reachable segment lengths are bounded by 802.11 frame size (< 4096 > AR_BufLen's 0xfff); no in-tree path produces a ≥4096-byte segment. Not filed (matches DF-4712 negative).
• ar5210ProcTxDesc (:589-618): every field extraction masked; writes bounded into caller-owned struct ath_tx_status. Clean.
• ar5210UpdateTxTrigLevel (:329-361): register-value arithmetic on curTrigLevel could wrap if HW returns garbage, but there is no memory-safety consequence (register write only); no clamp upstream in ar5416 either (judged clean there). Not filed.
• ar5210StopTxDma bounded 10 ms wait (1000×10 µs, :441-445); beacon path via bounded ath_hal_wait (ah.c:228-233, AH_TIMEOUT=1000). Clean.
• ar5210GetTxIntrQueue stub leaves *txqs untouched — caller pre-inits txqs = 0xffffffff (if_ath.c:2275) before the call; downstream queue processing is guarded by ATH_TXQ_SETUP. Safe stub.
• ar5210SetupXTxDesc returns AH_FALSE (no multi-rate on 5210) — caller-checked.

Bottom line

Source-confirmed latent OOB read/write below ah_txq[3] via negative q, plus five production-unvalidated q indices — the exact family the project has accepted fixes for twice already. Fix build-validated. Runtime hardware-gated.

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.