DF-4810 — AHCI command-table stride (ATA_AHCI_CT_SIZE=256) ≠ sizeof(struct ata_ahci_cmd_tab)=1152 — latent OOB of the DMA work area

File: sys/dev/disk/nata/chipsets/ata-ahci.c + sys/dev/disk/nata/ata-all.h (pass 2) Severity: Info (hardening / defense-in-depth — not exploitable in the tree as-is) · CWE-131 (incorrect calculation of buffer size) · confidence: certain

Build / run

Not applicable — this is a latent-hazard finding, proven by arithmetic on the type/constant definitions. No PoC can exercise it today because tag is hardcoded to 0 on every code path.

The mismatch

struct ata_ahci_cmd_tab (sys/dev/disk/nata/ata-all.h:249-255), __packed:

cfis[64] + acmd[32] + reserved[32] = 128  (== ATA_AHCI_CT_SG_OFFSET, ata-all.h:238 ✓)
prd_tab[64] × sizeof(struct ata_ahci_dma_prd = 8+4+4 = 16)  = 1024
total = 1152 bytes

but the stride used to place command tables inside the channel's DMA work area is ATA_AHCI_CT_SIZE = 256 (sys/dev/disk/nata/ata-all.h:239), and every indexing site uses that stride:

• ata-ahci.c:254-255 (ctp = work + ATA_AHCI_CT_OFFSET + ATA_AHCI_CT_SIZE*tag)
• ata-ahci.c:277 (cmd-list placement uses CL_SIZE — consistent)
• ata-ahci.c:286-287 (cmd_table_phys = work_bus + CT_OFFSET + CT_SIZE*tag — this is what the hardware is told)

Why it is safe today (and why it is a landmine)

• tag is int tag = 0 at both sites (ata-ahci.c:250, ata-ahci.c:343); this driver never issues more than one command per port (no NCQ).
• The work area is MAXWSPCSZ = PAGE_SIZE*2 = 8192 bytes (ata-dma.c:55-56, allocated at ata-dma.c:130-145), so the single real table at [1280, 1280+1152) = [1280, 2432) fits.
• If any future change derives tag from the request (the natural NCQ evolution — FreeBSD's later sys/dev/ahci does exactly that):
• tag ≥ 1: tables overlap/alias (stride 256 < struct 1152) — silent corruption of a live command's FIS/PRD by the next one;
• tag ≥ 28: 1280 + 28×256 = 8448 > 8192 — ctp and ctp->prd_tab point past the end of the 8192-byte bus_dmamem_alloc'd work area (heap/DMA-memory OOB write with fully attacker-influenced content: FIS bytes and PRD addresses of the I/O), and cmd_table_phys hands that OOB address to the AHCI engine for real DMA.

Secondary hardening items in the same function family:

• ata_ahci_dmasetprd() (ata-ahci.c:531-546): KASSERT(nsegs <= ATA_AHCI_DMA_ENTRIES) sits after the for (i = 0; i < nsegs; i++) prd[i] = ... write loop, and KASSERT is compiled out without INVARIANTS. Today the bound holds only structurally: max_iosize = 63×PAGE_SIZE (ata-ahci.c:558) caps a transfer at 64 pages-spanned == exactly 64 PRD entries == prd_tab[64] (the dma tag itself would permit up to ATA_DMA_ENTRIES = 256 segments, ata-dma.c:99,112). One careless bump of max_iosize or max_iosize/PAGE_SIZE arithmetic change turns this into a live OOB write with zero margin.

Fix

Make the constant self-consistent and assert it (see fix.diff):

-#define ATA_AHCI_CT_SIZE                256
+/* must equal sizeof(struct ata_ahci_cmd_tab) = 128 + 64*16 = 1152 */
+#define ATA_AHCI_CT_SIZE                1152

plus _Static_assert(ATA_AHCI_CT_SIZE == sizeof(struct ata_ahci_cmd_tab), ...) in ata-ahci.c, and move the KASSERT(nsegs <= ATA_AHCI_DMA_ENTRIES) before the write loop in ata_ahci_dmasetprd. With tag still 0 the address arithmetic is unchanged (CT_OFFSET + 0*CT_SIZE), so the constant fix is a no-op at runtime today and only removes the trap.

DF-4810 — VERDICT

Status: untested (latent hardening finding; Phase V not applicable — the hazard is provably unreachable in the tree as-is, tag hardcoded 0).

Reproduced?

No — by design of the finding: there is no runtime path today that can place a command table at tag != 0. The defect is an arithmetic inconsistency between ATA_AHCI_CT_SIZE (256, ata-all.h:239) and the real size of struct ata_ahci_cmd_tab (64+32+32+64×16 = 1152 bytes, ata-all.h:249-255, __packed, ATA_AHCI_CT_SG_OFFSET=128 confirming the 128-byte header). All indexing and the hardware-visible cmd_table_phys (ata-ahci.c:254-255, 286-287) use the wrong stride, held safe only by int tag = 0 (ata-ahci.c:250, 343) inside the 8192-byte work allocation (MAXWSPCSZ, ata-dma.c:55-56,130-145).

Impact ceiling if the constraint is ever lifted (why filed at all)

• tag ≥ 1 → adjacent command tables overlap (stride < struct): silent cross-command FIS/PRD corruption.
• tag ≥ 28 → 1280 + 28×256 = 8448 > 8192: ctp->prd_tab writes past the end of the bus_dmamem_alloc'd work area (kernel-heap OOB write, content = attacker-influenced DMA addresses), and the same OOB physical address is programmed into the AHCI command list for the engine to DMA through.

This is exactly the change a NCQ enablement patch would make, which is why the constant deserves to be correct before that day. Filed as Info (defense-in-depth, no demonstrated impact today), confidence certain (the mismatch is a compile-time fact).

Recommended fix

ATA_AHCI_CT_SIZE 256 → 1152 + _Static_assert + move the KASSERT(nsegs <= ATA_AHCI_DMA_ENTRIES) in ata_ahci_dmasetprd (ata-ahci.c:544) above the write loop. See fix.diff (authored against the read-only sys/ tree, never applied). Runtime-neutral today because only tag 0 is used.

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.