DF-2616 / 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 | --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -908,6 +908,46 @@ #endif /* + * Returns non-zero if the (untrusted, on-disk) bref data_off has illegal + * media geometry: a radix of 0 with a media offset, a radix greater than + * HAMMER2_RADIX_MAX, an offset inside the first 64KB (volume header area), + * or a block that would cross a 64KB DIO window boundary. + * + * NOTE: the freemap allocates at HAMMER2_ALLOC_MIN (1KB) granularity, so a + * radix-N block at a merely-1KB-aligned offset is LEGAL; alignment to the + * radix is NOT an invariant (verified against real filesystems). + * + * The DIO layer only asserts these conditions (hammer2_io.c:122-126, + * hammer2_io.c:565), which is compiled out without INVARIANTS. Without + * this validation a crafted filesystem gives chain->data a pointer whose + * chain->bytes extension runs past the end of the 64KB DIO buffer, + * turning every read (bcopy to the logical buffer / check code) and every + * write (bcopy from the logical buffer, base_insert/base_delete on + * indirect blocks) into a kernel heap out-of-bounds access. + */ +static __inline +int +hammer2_bad_data_off(hammer2_off_t data_off) +{ + hammer2_off_t lbase; + u_int radix; + u_int bytes; + + if ((data_off & ~HAMMER2_OFF_MASK_RADIX) == 0) + return(0); /* embedded, no media block */ + lbase = data_off & ~HAMMER2_OFF_MASK_RADIX; + radix = (u_int)(data_off & HAMMER2_OFF_MASK_RADIX); + if (radix == 0 || radix > HAMMER2_RADIX_MAX) + return(1); /* bad size class */ + bytes = 1U << radix; + if (lbase < HAMMER2_PBUFSIZE64) + return(1); /* volume header / boot area */ + if ((lbase & HAMMER2_PBUFMASK64) + bytes > HAMMER2_PBUFSIZE64) + return(1); /* crosses 64KB DIO window */ + return(0); +} + +/* * Issue I/O and install chain->data. Caller must hold a chain lock, lock * may be of any type. * @@ -938,6 +978,20 @@ if ((chain->bref.data_off & ~HAMMER2_OFF_MASK_RADIX) == 0) return; + /* + * Reject illegal media geometry before any I/O is issued or any + * data pointer is installed. chain->bytes was derived from the + * same untrusted radix in hammer2_chain_alloc(). + */ + if (hammer2_bad_data_off(chain->bref.data_off)) { + chain->error = HAMMER2_ERROR_CHECK; + kprintf("hammer2_chain_load_data: illegal data_off " + "geometry %016jx.%02x\n", + (intmax_t)chain->bref.data_off, + chain->bref.type); + return; + } + hmp = chain->hmp; KKASSERT(hmp != NULL); @@ -1470,10 +1524,27 @@ hammer2_chain_load_data(chain); if (chain->error) return (chain->error); + } else if (chain->data == NULL && chain->bytes != 0 && + (chain->bref.data_off & ~HAMMER2_OFF_MASK_RADIX)) { + /* + * OPTDATA modifications skip the data load, but will still + * issue I/O through the (untrusted) data_off below and in + * hammer2_write_bp(). Reject illegal geometry here too. + */ + if (hammer2_bad_data_off(chain->bref.data_off)) + return (HAMMER2_ERROR_CHECK); } error = 0; /* + * A dedup offset installed from the dedup table also originates + * from on-disk blockrefs (bulkfree scan); never install one with + * illegal geometry (fall back to a normal COW allocation instead). + */ + if (dedup_off && hammer2_bad_data_off(dedup_off)) + dedup_off = 0; + + /* * Set MODIFIED to indicate that the chain has been modified. A new * allocation is required when modifying a chain. * --- a/sys/vfs/hammer2/hammer2_io.c +++ b/sys/vfs/hammer2/hammer2_io.c @@ -119,7 +119,8 @@ lbase = data_off & ~HAMMER2_OFF_MASK_RADIX; pbase = lbase & pmask; - if (pbase == 0 || ((lbase + lsize - 1) & pmask) != pbase) { + if (lsize < 0 || lsize > HAMMER2_PBUFSIZE || + pbase == 0 || ((lbase + lsize - 1) & pmask) != pbase) { kprintf("Illegal: %016jx %016jx+%08x / %016jx\n", pbase, lbase, lsize, pmask); } |