⬢ DragonFlyBSD Kernel Audit
DF-2489 / fix.diff
← back to finding ↓ download raw
From: DragonFly BSD security audit <audit@localhost>
Subject: [PATCH] nata/ata-disk: clamp degenerate CHS geometry from device IDENTIFY data (DF-2489)

ad_get_geometry() copies adp->heads / adp->sectors straight out of the
device-supplied ATA IDENTIFY words (word 3 / word 6) whenever word 53
does not report words 54-58 valid (or current_heads/current_sectors are
zero) -- the else branch at ata-disk.c:476-480.  Neither that branch nor
anything between ata_getparam() (which copies IDENTIFY verbatim and only
requires a printable model byte, ata-all.c:601-663) and ad_attach()
validates the words, so a device reporting heads==0 or sectors==0 makes

    info.d_ncylinders = adp->total_secs/(adp->heads*adp->sectors);

at ata-disk.c:147 divide by zero: x86 #DE, "Fatal trap 18: integer
divide fault", killing the machine at device attach (boot or hot-plug).
The identical divisor is re-evaluated in the bootverbose path of
ad_describe() at ata-disk.c:540, so a site fix at :147 alone would still
panic every bootverbose boot.

Verified on the audit guest by driving the full production funnel
(real ata_identify() -> real ata_getparam() parse of a crafted IDENTIFY
-> real newbus -> the running kernel's own ad_attach): heads==0 (word 3)
and sectors==0 (word 6) each produce Fatal trap 18 at ad_attach+0x24b
(divq), the shipped binary.  IDENTIFY words 3 and 6 are retired in the
ATA spec (devices report legacy 16/63 defaults), which is exactly why a
malicious or simply buggy device / SATA-USB bridge firmware is free to
put zero there unchallenged.

Fix: clamp the geometry at its single source.  With the clamp, both
division sites (and the d_nheads/d_secpertrack values published to the
disk layer at :145-146) always see sane CHS values; total_secs keeps
being driven by the LBA sizes below, so media size is unaffected.  The
16/63 defaults match the classic ATA CHS geometry (what real devices
report today).
--- a/sys/dev/disk/nata/ata-disk.c
+++ b/sys/dev/disk/nata/ata-disk.c
@@ -478,6 +478,21 @@
 	adp->sectors = atadev->param.sectors;
 	adp->total_secs = atadev->param.cylinders * adp->heads * adp->sectors;
     }
+
+    /*
+     * DF-2489: a device reporting degenerate CHS geometry in its
+     * IDENTIFY data (word 3 heads and/or word 6 sectors == 0; the
+     * words are retired but never validated) would make ad_attach()
+     * and the bootverbose ad_describe() divide by heads*sectors == 0
+     * -- integer divide fault (trap 18) at attach.  Clamp to the
+     * classic minimum geometry; total_secs below is driven by the
+     * (device independent) LBA sizes anyway.
+     */
+    if (adp->heads == 0)
+	adp->heads = 16;
+    if (adp->sectors == 0)
+	adp->sectors = 63;
+
     lbasize = (u_int32_t)atadev->param.lba_size_1 |
 	      ((u_int32_t)atadev->param.lba_size_2 << 16);