ar5212GetNfAdjust sentinel-terminator bug: OOB read when channel == 0

• File: sys/dev/netif/ath/ath_hal/ar5212/ar5212_misc.c
• Lines: 769, 787, 789
• Severity: Info
• CVSS: CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U:C:L/I:N/A:L
• CWE: CWE-1284 Improper Validation of Specified Quantity in Input
• Confidence: speculative

Summary

The linear-scan lookup table in ar5212GetNfAdjust uses a sentinel entry {0,0} as the terminator, but the loop condition is c->channel <= adjustDef[i].freqLow (<=).

For any c->channel == 0 the comparison at the sentinel is 0 <= 0 which is TRUE, so i advances past the end of the static const array and adjustDef[11] is read out-of-bounds.

The loop will keep walking into adjacent rodata until it either finds a word that happens to read as 0 or faults across a page boundary into unmapped kernel address space.

Root cause

ar5212_misc.c:787

for (i = 0; c->channel <= adjustDef[i].freqLow; i++)
    ;

with the table declared at lines 769–784 ending in sentinel { 0, 0 }.

The intended contract is "return the adjustment for the highest freqLow strictly below c->channel" but the <= makes channel==0 fall through every entry including the sentinel.

c->channel is uint16_t (ah_internal.h:180) so it cannot be negative; the only bad input value is 0.

Either change <= to <, or change the sentinel freqLow to a value larger than any plausible channel (e.g. 0xffff).

Threat

There is no currently-reachable path that calls ar5212GetNfAdjust with c->channel == 0. All five callers in ah.c (1065, 1116, 1134-1135, 1174, 1179, 1185, 1196) either pass an ichan returned by ath_hal_checkchannel (ah_regdomain.c:776, which validates c->ic_freq == cc->channel against the constructed channel table) or iterate AH_PRIVATE(ah)->ah_channels[0..ah_nchan-1] whose entries are populated by ath_hal_setup_channel_table with real MHz frequencies.

The bug is therefore a latent defense-in-depth defect: any future change that lets a zero-frequency slot enter ah_channels[], or any new caller that passes an uninitialized/zero HAL_CHANNEL_INTERNAL, turns this into a kernel OOB read of adjacent rodata (info leak of kernel .rodata bytes via the returned int16_t adjust value, or kernel panic if the walk crosses into an unmapped page).

Marked speculative because no exploit path was traced.

Exploit / PoC

Not currently reproducible on a default DragonFlyBSD build.

To demonstrate the latent bug in isolation, a developer would need to

1. build a kernel that lets ah_channels[] contain an entry with channel==0, or
2. add a unit-test harness that invokes the AH5212(ah)->ah_getNfAdjust() vtable entry with a stack-zeroed HAL_CHANNEL_INTERNAL.

Under either setup: a single call with c->channel == 0 walks past adjustDef[10]; depending on what rodata immediately follows, the return value is either a stable garbage int16_t (silent wrong NF adjustment) or the loop eventually hits an unmapped page and the kernel panics with a fatal page fault in ar5212GetNfAdjust.

No PoC source is provided because the trigger is not reachable from userspace in the shipping tree.

Recommended fix

Make the sentinel unconditionally terminating regardless of channel value. Two equivalent options:

--- a/sys/dev/netif/ath/ath_hal/ar5212/ar5212_misc.c
+++ b/sys/dev/netif/ath/ath_hal/ar5212/ar5212_misc.c
@@ -780,7 +780,7 @@ ar5212GetNfAdjust(struct ath_hal *ah, const HAL_CHANNEL_INTERNAL *c)
        { 5209,  0 },
        { 3000,  1 },
-       {    0,  0 },
+       { 0xffff, 0 },  /* NB: sentinel must exceed any uint16_t channel */
    };
    int i;

or, equivalently, change the comparator from <= to < so a sentinel of 0 terminates for channel >= 0:

--- a/sys/dev/netif/ath/ath_hal/ar5212/ar5212_misc.c
+++ b/sys/dev/netif/ath/ath_hal/ar5212/ar5212_misc.c
@@ -785,7 +785,7 @@ ar5212GetNfAdjust(struct ath_hal *ah, const HAL_CHANNEL_INTERNAL *c)
    };
    int i;

-   for (i = 0; c->channel <= adjustDef[i].freqLow; i++)
+   for (i = 0; adjustDef[i].freqLow != 0xffff && c->channel <= adjustDef[i].freqLow; i++)
        ;
    return adjustDef[i].adjust;
 }

The first option (widen sentinel) is preferred: it is a one-line change with no behavioral difference for any in-range channel and is robust against c->channel == 0.