DragonFlyBSD Kernel Audit
DF-2651 / fix_all_three_findings.diff
← back to finding ↓ download raw
--- a/sys/vfs/hammer2/hammer2_freemap.c
+++ b/sys/vfs/hammer2/hammer2_freemap.c
@@ -673,10 +673,15 @@
 		default:
 			break;
 		}
-		if (i >= 0) {
+		/*
+		 * DF-2653: reject sub_key-derived indexes that do not leave
+		 * room for bmradix bits inside the 64-bit element (a crafted
+		 * DATA bref key that is not radix-aligned gives j=62 with
+		 * bmradix=8).  Fall back to the general scan.
+		 */
+		if (i >= 0 && j + bmradix <= HAMMER2_BMAP_BITS_PER_ELEMENT) {
 			KKASSERT(i < HAMMER2_BMAP_ELEMENTS &&
 				 j < 2 * HAMMER2_BMAP_BLOCKS_PER_ELEMENT);
-			KKASSERT(j + bmradix <= HAMMER2_BMAP_BITS_PER_ELEMENT);
 			bmmask = (bmradix == HAMMER2_BMAP_BITS_PER_ELEMENT) ?
 				 HAMMER2_BMAP_ALLONES :
 				 ((hammer2_bitmap_t)1 << bmradix) - 1;
@@ -1013,12 +1018,18 @@
 	/*
 	 * Stop early if we are trying to free something but no leaf exists.
 	 */
-	if (chain == NULL && how != HAMMER2_FREEMAP_DORECOVER) {
-		kprintf("hammer2_freemap_adjust: %016jx: no chain\n",
-			(intmax_t)bref->data_off);
-		goto done;
-	}
-	if (chain->error) {
+	/*
+	 * DF-2651: chain can legitimately be NULL here (missing freemap
+	 * leaf); only DORECOVER creates it further down.  Do not touch
+	 * chain->error until NULL has been ruled out.
+	 */
+	if (chain == NULL) {
+		if (how != HAMMER2_FREEMAP_DORECOVER) {
+			kprintf("hammer2_freemap_adjust: %016jx: no chain\n",
+				(intmax_t)bref->data_off);
+			goto done;
+		}
+	} else if (chain->error) {
 		kprintf("hammer2_freemap_adjust: %016jx: error %s\n",
 			(intmax_t)bref->data_off,
 			hammer2_error_str(chain->error));
@@ -1070,7 +1081,15 @@
 	/*
 	 * Calculate the bitmask (runs in 2-bit pairs).
 	 */
-	start = ((int)(data_off >> HAMMER2_FREEMAP_BLOCK_RADIX) & 15) * 2;
+	/*
+	 * DF-2652: each bitmapq[] element covers 32 x 16KB blocks
+	 * (HAMMER2_BMAP_BLOCKS_PER_ELEMENT), so the block number within
+	 * the element is 5 bits, not 4.  The old '& 15' made every
+	 * recovery/dedup fixup for blocks 16..31 of a 512KB element mark
+	 * the wrong 16KB chunk, leaving live blocks marked free.
+	 */
+	start = ((int)(data_off >> HAMMER2_FREEMAP_BLOCK_RADIX) &
+		 (HAMMER2_BMAP_BLOCKS_PER_ELEMENT - 1)) * 2;
 	//bmmask01 = (hammer2_bitmap_t)1 << start;
 	//bmmask10 = (hammer2_bitmap_t)2 << start;
 	bmmask11 = (hammer2_bitmap_t)3 << start;