⬢ DragonFlyBSD Kernel Audit
← triage · dashboard
DF-3752

Uninitialized retrycnt passed to ratectl on TX_FAILURE (amrr consumes it unconditionally)

Summary

rt2860_drain_stats_fifo declares retrycnt and assigns it only in the TXQ_OK branch; the TX_FAILURE branch passes &retrycnt and amrr_tx_complete reads *(int*)arg1 unconditionally, doing amn->amn_retrycnt += <interrupt-stack garbage>. UB that corrupts AMRR retry accounting on every failed-frame stat. Wrong rate adaptation with kernel stack residue; no disclosure, no memory unsafety. One-line fix.

Discussion (0)

No comments yet.

PoC verification

Evidence pack

findings/poc/DF-3752 · 5 files
FileTypeDescriptionSize
README.md — 1.4 KB ↓ raw
fix.diff — 552 B view raw
verdict.json — 2.0 KB view raw
env.txt — 275 B view raw
manifest.json — 823 B view raw
README.md
↓ download raw

DF-3752 — retrycnt used uninitialized on the TX-failure feedback path (CWE-457)

rt2860_drain_stats_fifo (rt2860.c:1125-1166) declares int retrycnt; (:1129) and only ever assigns it inside the TXQ_OK branch (:1153-1156). The failure branch passes its address straight through:

} else {
    ieee80211_ratectl_tx_complete(ni->ni_vap, ni,
        IEEE80211_RATECTL_TX_FAILURE, &retrycnt, NULL);   /* rt2860.c:1160-1161 */

ieee80211_ratectl_tx_complete (ieee80211_ratectl.h:99-103) forwards to amrr_tx_complete, which reads int retries = *(int *)arg1; unconditionally (sys/netproto/802_11/wlan/ieee80211_amrr.c:359-370) and accumulates it: amn->amn_retrycnt += retries;. Every failed-frame stat therefore folds an uninitialized stack slot of the interrupt path into the per-node rate-control state, corrupting AMRR's retry accounting (wrong rate adaptation decisions, e.g. premature rate drops) with kernel stack garbage. No direct disclosure to userspace, but the garbage does influence observable TX behavior; classic undefined behavior.

Fix: fix.diff — retrycnt = 0; in the else branch (a one-liner; the compiler is free to do anything with the current indeterminate value).

Status: source-certain, Low severity. Not runnable on the audit guest (no Ralink NIC — env.txt); would reproduce trivially on hardware by TXing to an unreachable station and observing garbage retry stats via wlanstats.

Fix verification

not_testable
baseline no→ patch + rebuild →patched clean

One-line initialization removes the UB; compile-consistent.

fix.diff (git apply --check passes against pristine sys/)
↓ fix.diffper-fix-DF-3752

Confirmed kernel references

Detail

Evidence (decisive lines)

README.md (the three code sites), fix.diff (retrycnt = 0 in else branch; git apply --check passes)

PoC changes

Analysis-only (hardware precondition).

Verified recommended fix

Initialize retrycnt = 0 before the TX_FAILURE ieee80211_ratectl_tx_complete call.

Verdict

rt2860_drain_stats_fifo declares retrycnt (rt2860.c:1129) and assigns it only in the TXQ_OK branch (rt2860.c:1153-1156); the TX_FAILURE branch passes &retrycnt to ieee80211_ratectl_tx_complete (rt2860.c:1160-1161), and amrr_tx_complete dereferences arg1 unconditionally (ieee80211_amrr.c:364) doing amn->amn_retrycnt += garbage. Uninitialized stack read consumed by the rate controller on every failed-frame stat -- UB per the C standard, corrupts AMRR retry accounting with interrupt-path stack residue. Source-certain; runtime impact is wrong rate adaptation (observable behavior), no direct info leak. Not reproduced: guest has no Ralink NIC (env.txt).