DF-2498 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 | 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, |