• File: sys/dev/netif/ath/ath_hal/ar5416/ar5416_xmit.c
• Lines: 583, 589
• Severity: Low
• CVSS: CVSS:3.1/AV:L/AC:H/PR:L/UI:N/S:U:C:N/I:N/A:L
• CWE: CWE-125 Out-of-bounds Read
• Confidence: speculative

Summary

In ar5416ChainTxDesc(), the if (keyIx != HAL_TXKEYIX_INVALID) block guards the DestIdx/Valid writes (ar5416_xmit.c:583-587) but the per-key encryption-type lookup SM(ahp->ah_keytype[keyIx], AR_EncrType) at ar5416_xmit.c:589 sits OUTSIDE that block.

When a caller passes HAL_TXKEYIX_INVALID (defined as ((u_int)-1) in ah_desc.h:222) to indicate "no encryption", the code reads ahp->ah_keytype[0xFFFFFFFF] — a wild one-byte OOB read whose result is fed into the TX descriptor's AR_EncrType field.

The sibling function ar5416SetupTxDesc() places the identical lookup correctly INSIDE the INVALID guard (ar5416_xmit.c:390-395), confirming this is a misplaced-statement defect rather than an intentional idiom.

Root cause

ar5416_xmit.c:583-589:

if (keyIx != HAL_TXKEYIX_INVALID) {            /* guards DestIdx only */
    /* XXX validate key index */
    ads->ds_ctl1 |= SM(keyIx, AR_DestIdx);
    ads->ds_ctl0 |= AR_DestIdxValid;
}

ads->ds_ctl6 |= SM(ahp->ah_keytype[keyIx], AR_EncrType);   /* line 589: UNCONDITIONAL */

ah_keytype is uint8_t ah_keytype[AR5416_KEYTABLE_SIZE] with AR5416_KEYTABLE_SIZE == 128 (ar5416.h:56,116).

HAL_TXKEYIX_INVALID is ((u_int)-1) == 0xFFFFFFFF (ah_desc.h:222).

For unencrypted frames the driver convention is to pass INVALID, so keyIx == 0xFFFFFFFF and the array access is wild.

The XXX comment two lines up ("validate key index") acknowledges that no bounds check exists; the misplacement compounds it by removing even the INVALID sentinel check.

ar5416SetupTxDesc shows the intended structure: the same SM(ahp->ah_keytype[keyIx], AR_EncrType) write lives inside the if (keyIx != HAL_TXKEYIX_INVALID) body (ar5416_xmit.c:390-395).

Threat

The bug is real by inspection but currently DORMANT in-tree: ar5416ChainTxDesc is registered as the HAL method ah_chainTxDesc (ar5416_attach.c:184) and the public macro ath_hal_chaintxdesc is exposed (if_athvar.h:1438-1442), BUT no code path in sys/dev/netif/ath/ath/ invokes it — the modern EDMA TX path explicitly uses ah_fillTxDesc instead (ah.h:1586-1588 comment: "Descriptors are chained together by using filltxdesc (not ChainTxDesc)").

Confirmed by grep: 'ath_hal_chaintxdesc' appears only in the macro definition, never at a call site. So exploitability through the stock driver is currently zero.

The finding is recorded because (a) the method is part of the public HAL vtable and any out-of-tree net80211/HAL consumer (or a future driver refactor) that calls ah_chainTxDesc with the standard INVALID sentinel for plaintext frames would trigger it on every TX, and (b) it is a one-line fix.

If/when reachable: trigger is trivial (transmit any unencrypted frame), impact is a kernel OOB read of 1 byte at offset ~4GB from ah_keytype[] — almost certainly an unmapped page → kernel panic (local DoS).

The read byte is written into the descriptor's 2-bit AR_EncrType field that is DMA-consumed by the NIC, so it does not return to userspace and is not a clean info leak; net effect is denial of service via panic on the wild dereference.

Attacker position: unprivileged local user with transmit access to an ath(4) interface that has been switched onto the ChainTxDesc path. No special caps required beyond network access.

Exploit / PoC

Cannot be reproduced against the unmodified in-tree driver because ah_chainTxDesc has no caller.

To make it reproducible (proof-of-concept of the latent bug), one would write a small loadable kernel module that obtains the ath_hal handle from a live ath(4) softc and invokes the method directly via the vtable:

/* kld: trigger ar5416ChainTxDesc OOB */
HAL_DMA_ADDR ba = 0; uint32_t sl = 64;
ah->ah_chainTxDesc(ah, some_desc, &ba, &sl,
    64,        /* pktLen */
    0,         /* hdrLen */
    HAL_PKT_TYPE_NORMAL,
    HAL_TXKEYIX_INVALID,   /* <-- triggers ah_keytype[0xFFFFFFFF] */
    HAL_CIPHER_CLR, 0,
    AH_TRUE, AH_TRUE, AH_FALSE);

Building on DragonFlyBSD: place in a module with DECLARE_MODULE, make against /usr/src/sys/modules/ath or a standalone klddev.

Running: kldload ./chain_oob.ko on a host with ath(4) attached.

Expected result: panic in ar5416ChainTxDesc at the wild array index (likely fatal trap 12: page fault while in kernel mode on the dereference of ah_keytype+0xFFFFFFFF).

Success criterion = deterministic panic proving the OOB read.

Because the path is dead in stock sys/, no trigger program that uses only the public if_ath driver interface can reach it; the PoC must poke the HAL vtable directly, hence "speculative" confidence on real-world exploitability.

Recommended fix

Move the ah_keytype[] lookup inside the INVALID guard so it matches ar5416SetupTxDesc, and add the missing bounds check the XXX comment asks for.

--- a/sys/dev/netif/ath/ath_hal/ar5416/ar5416_xmit.c
+++ b/sys/dev/netif/ath/ath_hal/ar5416/ar5416_xmit.c
@@ -580,13 +580,15 @@ ar5416ChainTxDesc(struct ath_hal *ah, struct ath_desc *ds,

    ads->ds_ctl2 = 0;
    ads->ds_ctl3 = 0;
-   if (keyIx != HAL_TXKEYIX_INVALID) {
-       /* XXX validate key index */
-       ads->ds_ctl1 |= SM(keyIx, AR_DestIdx);
-       ads->ds_ctl0 |= AR_DestIdxValid;
-   }
-
-   ads->ds_ctl6 |= SM(ahp->ah_keytype[keyIx], AR_EncrType);
+   if (keyIx != HAL_TXKEYIX_INVALID &&
+       keyIx < AR5416_KEYTABLE_SIZE) {
+       ads->ds_ctl1 |= SM(keyIx, AR_DestIdx);
+       ads->ds_ctl0 |= AR_DestIdxValid;
+       ads->ds_ctl6 |= SM(ahp->ah_keytype[keyIx], AR_EncrType);
+   } else {
+       /* No/invalid key index: encryption type = clear. */
+       ads->ds_ctl6 &= ~AR_EncrType;
+   }
    if (isaggr) {
        ads->ds_ctl6 |= SM(delims, AR_PadDelim);
    }

This mirrors ar5416SetupTxDesc's structure (ar5416_xmit.c:390-395), bounds the index against AR5416_KEYTABLE_SIZE (128) as the XXX comment requests, and explicitly clears the AR_EncrType field for plaintext so the descriptor is not left with stale encryption-type bits.

The same explicit keyIx < AR5416_KEYTABLE_SIZE check should also be retro-fitted into ar5416SetupTxDesc at ar5416_xmit.c:390 to harden the reachable path.