DragonFlyBSD Kernel Audit
DF-2616 / fix.diff
← back to finding ↓ download raw
--- 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);
 	}