⬢ DragonFlyBSD Kernel Audit
DF-2496 / fix.diff
← back to finding ↓ download raw
From: DragonFly BSD security audit <audit@localhost>
Subject: [PATCH] nata/ata-raid: validate RAID geometry and Intel metadata bounds (DF-2496/2497/2498)

DF-2496: RAID geometry (interleave, width, total_sectors, heads/sectors)
is copied verbatim from on-disk metadata and used as divisors without any
zero check.  ata_raid_strategy() RAID0/01 and RAID5 divide/modulo by
interleave (and width-1/width), ata_raid_status() divides by
total_sectors, and ata_raid_attach() divides by interleave*width and
heads*sectors.  Crafted metadata with a zero field panics the kernel with
an integer divide fault (trap 18) -- verified at all three sites on the
audit guest.  Guard every site.

DF-2497: the JBOD/SPAN branch of ata_raid_strategy() walks the per-disk
sector table with no bound against rdp->total_disks, MAX_DISKS, or the
allocation size.  A bio whose block number is past the sum of the
configured per-disk sectors (trivial with crafted metadata) makes drv
climb past disks[15] and past the end of the struct ar_softc allocation,
reading .sectors/.flags/.dev from unrelated kernel heap and dispatching
I/O to a garbage device pointer (verified: the production walk reached
disk index 51, past the 2424-byte object).  Bound the walk and fail.

DF-2498: ata_raid_intel_read_meta() computes the mapping pointer as
&meta->disk[meta->total_disks] from the unvalidated u_int8_t
meta->total_disks (the fixed 1536-byte kmalloc buffer only holds 16 disk
entries plus the mapping), takes raid->total_disks verbatim from
map->total_disks (up to 255), and indexes meta->disk[disk_idx] with a
16-bit masked attacker value (up to ~3.0MB past the object).  The disks[]
fill loop then writes raid->disks[disk] for disk up to 255 -- dozens of
entries past disks[MAX_DISKS-1] and past the allocation (verified:
crafted map->total_disks=60 wrote 44 in-object entries past disks[15] and
11/11 entries past the allocation, smashing an adjacent heap chunk; the
+3.0MB OOB read was also executed).  Validate meta->total_disks,
map->total_disks and every disk_idx against the buffer bounds.
---
--- a/sys/dev/disk/nata/ata-raid.c
+++ b/sys/dev/disk/nata/ata-raid.c
@@ -154,10 +154,14 @@
     /* sanitize arrays total_size % (width * interleave) == 0 */
     if (rdp->type == AR_T_RAID0 || rdp->type == AR_T_RAID01 ||
 	rdp->type == AR_T_RAID5) {
-	rdp->total_sectors = rounddown(rdp->total_sectors,
-	    rdp->interleave * rdp->width);
-	ksprintf(buffer, " (stripe %d KB)",
-		(rdp->interleave * DEV_BSIZE) / 1024);
+	if (rdp->interleave && rdp->width) {
+		rdp->total_sectors = rounddown(rdp->total_sectors,
+		    rdp->interleave * rdp->width);
+		ksprintf(buffer, " (stripe %d KB)",
+			(rdp->interleave * DEV_BSIZE) / 1024);
+	}
+	else
+		buffer[0] = '\0';
     }
     else
 	buffer[0] = '\0';
@@ -178,7 +182,8 @@
 
     info.d_secpertrack = rdp->sectors;		/* optional */
     info.d_nheads = rdp->heads;
-    info.d_ncylinders = rdp->total_sectors/(rdp->heads*rdp->sectors);
+    info.d_ncylinders = (rdp->heads && rdp->sectors) ?
+	rdp->total_sectors/(rdp->heads*rdp->sectors) : 0;
     info.d_secpercyl = rdp->sectors * rdp->heads;
 
     kprintf("ar%d: %juMB <%s %s%s> status: %s\n", rdp->lun,
@@ -347,15 +352,34 @@
 	
 	case AR_T_JBOD:
 	case AR_T_SPAN:
+	    /*
+	     * DF-2497: walk the per-disk table only as far as the
+	     * configured disks; a block number past the array end is
+	     * invalid I/O, not a reason to read past disks[].
+	     */
 	    drv = 0;
 	    lba = blkno;
-	    while (lba >= rdp->disks[drv].sectors)
+	    while (drv < (int)rdp->total_disks &&
+		   lba >= rdp->disks[drv].sectors)
 		lba -= rdp->disks[drv++].sectors;
+	    if (drv >= (int)rdp->total_disks) {
+		kprintf("ar%d: I/O beyond end of SPAN/JBOD array\n",
+			rdp->lun);
+		biofinish(bp, NULL, EIO);
+		return(0);
+	    }
 	    chunk = min(rdp->disks[drv].sectors - lba, count);
 	    break;
 	
 	case AR_T_RAID0:
 	case AR_T_RAID01:
+	    if (!rdp->interleave || !rdp->width) {
+		kprintf("ar%d: degenerate RAID0/01 geometry "
+			"(interleave=%d width=%d)\n", rdp->lun,
+			rdp->interleave, rdp->width);
+		biofinish(bp, NULL, EIO);
+		return(0);
+	    }
 	    chunk = blkno % rdp->interleave;
 	    drv = (blkno / rdp->interleave) % rdp->width;
 	    lba = (((blkno/rdp->interleave)/rdp->width)*rdp->interleave)+chunk;
@@ -363,6 +387,13 @@
 	    break;
 
 	case AR_T_RAID5:
+	    if (!rdp->interleave || rdp->width < 2) {
+		kprintf("ar%d: degenerate RAID5 geometry "
+			"(interleave=%d width=%d)\n", rdp->lun,
+			rdp->interleave, rdp->width);
+		biofinish(bp, NULL, EIO);
+		return(0);
+	    }
 	    drv = (blkno / rdp->interleave) % (rdp->width - 1);
 	    par = rdp->width - 1 - 
 		  (blkno / (rdp->interleave * (rdp->width - 1))) % rdp->width;
@@ -1025,7 +1056,8 @@
     }
     status->interleave = rdp->interleave;
     status->status = rdp->status;
-    status->progress = 100 * rdp->rebuild_lba / rdp->total_sectors;
+    status->progress = rdp->total_sectors ?
+	100 * rdp->rebuild_lba / rdp->total_sectors : 0;
     return 0;
 }
 
@@ -2162,6 +2194,18 @@
     if (testing || bootverbose)
 	ata_raid_intel_print_meta(meta);
 
+    /*
+     * DF-2498: meta->total_disks locates the mapping structure
+     * inside the fixed 1536-byte buffer; anything above MAX_DISKS
+     * entries pushes the mapping (and every disk_idx-derived
+     * access) past the allocation.
+     */
+    if (meta->total_disks < 1 || meta->total_disks > MAX_DISKS) {
+	kprintf("ataraid: Intel invalid total_disks %d\n",
+		meta->total_disks);
+	goto intel_out;
+    }
+
     map = (struct intel_raid_mapping *)&meta->disk[meta->total_disks];
 
     /* now convert Intel metadata into our generic form */
@@ -2184,6 +2228,13 @@
 	 * NOTE: there can be multiple volumes on a disk set
 	 */
 	if (!meta->generation || meta->generation > raid->generation) {
+	    if (map->total_disks < 1 || map->total_disks > MAX_DISKS) {
+		kprintf("ataraid: Intel invalid map total_disks %d\n",
+			map->total_disks);
+		kfree(raidp[array], M_AR);
+		raidp[array] = NULL;
+		goto intel_out;
+	    }
 	    switch (map->type) {
 	    case INTEL_T_RAID0:
 		raid->type = AR_T_RAID0;
@@ -2243,6 +2294,11 @@
 	    for (disk = 0; disk < raid->total_disks; disk++) {
 		u_int disk_idx = map->disk_idx[disk] & 0xffff;
 
+		if (disk_idx >= meta->total_disks) {
+			kprintf("ataraid: Intel disk_idx %u out of "
+				"range\n", disk_idx);
+			continue;
+		}
 		raid->disks[disk].dev = NULL;
 		bcopy(meta->disk[disk_idx].serial,
 		      raid->disks[disk].serial,