DragonFlyBSD Kernel Audit
DF-3068 / fix.diff
← back to finding ↓ download raw
--- a/sys/vfs/hammer/hammer_recover.c
+++ b/sys/vfs/hammer/hammer_recover.c
@@ -210,6 +210,7 @@
 	hammer_buffer_t buffer;
 	hammer_off_t scan_offset;
 	hammer_off_t scan_offset_save;
+	hammer_off_t walked;
 	hammer_off_t bytes;
 	hammer_fifo_any_t head;
 	hammer_off_t first_offset;
@@ -257,7 +258,19 @@
 		scan_offset = first_offset;
 		seqno = 0;
 
+		/*
+		 * DF-3067: PAD records are exempt from the seqno check
+		 * below and hammer_recover_scan_rev() wraps at the zone
+		 * base, so an undo FIFO consisting solely of PAD records
+		 * would otherwise make this loop spin forever.  Bound the
+		 * backscan to one lap around the FIFO.
+		 */
+		walked = 0;
 		for (;;) {
+			if (walked >= HAMMER_OFF_LONG_ENCODE(rootmap->alloc_offset)) {
+				error = EIO;
+				break;
+			}
 			head = hammer_recover_scan_rev(hmp, root_volume,
 						       &scan_offset,
 						       &error, &buffer);
@@ -267,6 +280,7 @@
 				seqno = head->head.hdr_seq;
 				break;
 			}
+			walked += head->head.hdr_size;
 		}
 		if (error) {
 			hvkprintf(root_volume,
@@ -398,6 +412,23 @@
 			break;
 
 		/*
+		 * DF-3068: a corrupt record (e.g. a fake tail anchoring
+		 * an oversized head) can claim more bytes than remain in
+		 * the nominal undo range.  Without this check bytes goes
+		 * negative and either KKASSERT(error || bytes == 0) panics
+		 * an INVARIANTS kernel or the mount silently mis-recovers
+		 * on production kernels.
+		 */
+		if ((hammer_off_t)head->head.hdr_size > bytes) {
+			error = EIO;
+			hvkprintf(root_volume,
+				"Corrupt UNDO record size %04x at %016jx\n",
+				head->head.hdr_size,
+				(intmax_t)scan_offset);
+			break;
+		}
+
+		/*
 		 * Normal UNDO
 		 */
 		error = hammer_recover_undo(hmp, root_volume, &head->undo);
@@ -517,6 +548,7 @@
 {
 	hammer_blockmap_t rootmap;
 	hammer_buffer_t buffer;
+	hammer_off_t walked;
 	hammer_off_t scan_offset;
 	hammer_off_t oscan_offset;
 	hammer_off_t bytes;
@@ -649,7 +681,18 @@
 			(intmax_t)ext_offset,
 			(intmax_t)ext_bytes);
 		seqno = hmp->recover_stage2_seqno - 1;
+		/*
+		 * DF-3067 (stage2 variant): the loop below only ends at
+		 * ext_offset, which is the unvalidated redo_offset field
+		 * of a REDO_SYNC record.  PAD records bypass the seqno
+		 * check, so bound the scan to one lap around the FIFO.
+		 */
+		walked = 0;
 		for (;;) {
+			if (walked >= HAMMER_OFF_LONG_ENCODE(rootmap->alloc_offset)) {
+				error = EIO;
+				break;
+			}
 			head = hammer_recover_scan_rev(hmp, root_volume,
 						       &scan_offset,
 						       &error, &buffer);
@@ -665,6 +708,7 @@
 						scan_offset, &head->redo);
 				--seqno;
 			}
+			walked += head->head.hdr_size;
 			if (scan_offset == ext_offset)
 				break;
 		}
@@ -703,6 +747,19 @@
 		if (error)
 			break;
 
+		/*
+		 * DF-3068 (stage2 sibling): don't let one corrupt record
+		 * drive bytes negative before the KKASSERT at loop end.
+		 */
+		if ((hammer_off_t)head->head.hdr_size > bytes) {
+			error = EIO;
+			hvkprintf(root_volume,
+				"Corrupt REDO record size %04x at %016jx\n",
+				head->head.hdr_size,
+				(intmax_t)oscan_offset);
+			break;
+		}
+
 		error = hammer_recover_redo_run(hmp, &rterm_root,
 						oscan_offset, &head->redo);
 		if (error) {