# DF-1211 — VERDICT

**Finding:** OOB read+write in AHCI NCQ error recovery: a device-controlled
READ LOG EXT page-10h tag indexes `ap_ccbs[]` without a bounds check
(`sys/dev/disk/ahci/ahci.c`).
**Status:** NOT TESTABLE on this audit guest. **Confidence (bug is real):** certain.
**Impact ceiling:** OOB array index → 20-byte OOB write + 2 single-byte OOB
writes of device-controlled data, plus a caller OOB write; **device-controlled**.
**Fix:** authored in `fix.diff`, applied clean, compile-validated into GENERIC.

## Mechanism (confirmed line-by-line in `sys/`)

1. `ahci_port_read_ncq_error()` (`ahci.c`) is called from the TFES error path
   (`ahci.c:2718`) when NCQ commands (`ap_sactive`) were active.
2. It issues a READ LOG EXT - 0x10, DMA'd into `ap->ap_err_scratch`
   (`ahci.c:3480`, `log = (struct ata_log_address_10h *)ap->ap_err_scratch`).
3. `ahci.c:3488` — `err_slot = log->err_regs.type & ATA_LOG_10H_TYPE_TAG_MASK;`
   The tag field is the low 5 bits (0-31) of a **device-supplied** byte.
4. `ahci.c:3490` — `ccb2 = &ap->ap_ccbs[err_slot];` **NO bounds check.**
   `ap_ccbs[]` holds `sc->sc_ncmds` (4-32) entries.
5. `ahci.c:3491` — `if (ccb2->ccb_xa.state == ATA_S_ONCHIP)` reads the OOB
   ccb's state (OOB read).
6. `ahci.c:3495-3498` — on match: `memcpy(&ccb2->ccb_xa.rfis, &log->err_regs,
   sizeof(struct ata_fis_d2h))` writes **20 bytes of device data** into the
   OOB ccb's rfis, then `rfis.type = ATA_FIS_TYPE_D2H; rfis.flags = 0;`
   (two 1-byte OOB writes).
7. Caller `ahci.c:2805` — `ccb = &ap->ap_ccbs[err_slot];` (same bad slot) and
   `ahci.c:2807-2808` — `KKASSERT(...); ccb->ccb_xa.state = ATA_S_ERROR;`
   another OOB write.

The only mitigation today is that `ap_ccbs[]` is sometimes over-allocated
relative to the real NCQ depth, so some out-of-range tags land in unused tail
slots — but the array is exactly `sc_ncmds` long, so any tag in
`[sc_ncmds .. 31]` is a genuine OOB.

## Why it is NOT TESTABLE on this guest

- No AHCI/SATA controller (`pciconf -l` clean; root disk is virtio-blk), so
  `ahci(4)` does not attach; the NCQ error-recovery path is dead code here.
- The malicious tag originates in device firmware / a malicious disk device
  model (DMA into `ap_err_scratch`), not in any userspace syscall. A benign
  device always echoes a tag the host issued (< sc_ncmds).

Valid "device-controlled, not reachable from an unprivileged user on this
guest" case. Bug is genuine (missing bounds check at the only tag-decode
site); threat model = malicious/buggy NCQ device / malicious VM disk model.

## Exploit chain
None developed: device-controlled primitive with no unprivileged syscall
path on this guest (and no AHCI device at all here). Documented impact
ceiling: OOB read + up to 22 bytes of OOB write (device-controlled content)
per NCQ error.

## Fix
`fix.diff` adds the bounds check right after the tag is decoded:
```c
err_slot = log->err_regs.type & ATA_LOG_10H_TYPE_TAG_MASK;
if (err_slot < 0 || err_slot >= ap->ap_sc->sc_ncmds) {
	kprintf("%s: NCQ error log bad tag %d\n", PORTNAME(ap), err_slot);
	err_slot = -1;
	goto err;
}
ccb2 = &ap->ap_ccbs[err_slot];
```
`err:` (`ahci.c:3507`) is the function's existing cleanup label; it returns
`err_slot = -1`, and the caller's `err_slot < 0` handling (`ahci.c:2791`)
then resets the port (`goto failall`). Minimal, targeted at the only
missing-check site, symmetric with the TFES guard.

## Build / run on this guest
`./run.sh` is a reachability probe; on this guest it reports "no AHCI
controller → NCQ error path unreachable". Bug confirmed by the trace above;
fix compile-validated into GENERIC.