From: DragonFly BSD security audit 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,