DragonFlyBSD Kernel Audit
DF-2617 / fix.diff
← back to finding ↓ download raw
--- a/sys/vfs/hammer2/hammer2_chain.c
+++ b/sys/vfs/hammer2/hammer2_chain.c
@@ -935,8 +935,38 @@
 			hammer2_io_bkvasync(chain->dio);
 		return;
 	}
-	if ((chain->bref.data_off & ~HAMMER2_OFF_MASK_RADIX) == 0)
+	if ((chain->bref.data_off & ~HAMMER2_OFF_MASK_RADIX) == 0) {
+		/*
+		 * A data_off of 0 (no media offset, no radix) means the
+		 * chain has no media data block.  This is legitimate for
+		 * embedded dirents (names <= 64 bytes live in the blockref
+		 * itself, see hammer2_chain_dirent_test()) and for freshly
+		 * allocated chains that have not been flushed yet
+		 * (HAMMER2_CHAIN_INITIAL, whose data_off is still 0 until
+		 * hammer2_chain_modify() allocates the block).
+		 *
+		 * For every other blockref type (INODE, INDIRECT, DATA,
+		 * FREEMAP_NODE, FREEMAP_LEAF, media DIRENT) a zero data_off
+		 * is on-disk corruption.  Historically this case fell
+		 * through silently: chain->data stayed NULL with
+		 * chain->error == 0, so the first consumer dereferenced
+		 * NULL (hammer2_vfsops.c:1309/1311 super-root ipdata,
+		 * hammer2_vfsops.c:1560-1561 PFS scan, the
+		 * parent->data->ipdata reads in hammer2_chain_lookup) or
+		 * wedged forever in the debugging loop at
+		 * hammer2_chain.c:2524-2529 while(1) tsleep("xxx").
+		 *
+		 * Record a real error so callers fail cleanly instead.
+		 */
+		if ((chain->flags & HAMMER2_CHAIN_INITIAL) == 0 &&
+		    chain->bref.type != HAMMER2_BREF_TYPE_DIRENT) {
+			chain->error = HAMMER2_ERROR_EIO;
+			kprintf("hammer2_chain_load_data: illegal zero "
+				"data_off on bref type %d\n",
+				chain->bref.type);
+		}
 		return;
+	}
 
 	hmp = chain->hmp;
 	KKASSERT(hmp != NULL);
--- a/sys/vfs/hammer2/hammer2_vfsops.c
+++ b/sys/vfs/hammer2/hammer2_vfsops.c
@@ -1388,8 +1388,16 @@
 				     lhc, lhc + HAMMER2_DIRHASH_LOMASK,
 				     &error, 0);
 	while (chain) {
-		if (chain->bref.type == HAMMER2_BREF_TYPE_INODE &&
-		    strcmp(label, (char *)chain->data->ipdata.filename) == 0) {
+		if (chain->error) {
+			/*
+			 * Corrupt label entry (e.g. zero data_off rejected
+			 * by hammer2_chain_load_data()).  Do not touch
+			 * chain->data (it is NULL); keep scanning.
+			 */
+			error = chain->error;
+		} else if (chain->bref.type == HAMMER2_BREF_TYPE_INODE &&
+			   strcmp(label,
+				  (char *)chain->data->ipdata.filename) == 0) {
 			break;
 		}
 		chain = hammer2_chain_next(&parent, chain, &key_next,
--- a/sys/vfs/hammer2/hammer2_iocom.c
+++ b/sys/vfs/hammer2/hammer2_iocom.c
@@ -311,6 +311,17 @@
 				     HAMMER2_KEY_MIN, HAMMER2_KEY_MAX,
 				     &error, 0);
 	while (chain) {
+		if (chain->error) {
+			/*
+			 * Corrupt entry (e.g. zero data_off rejected by
+			 * hammer2_chain_load_data()); chain->data is NULL.
+			 * Skip it, do not dereference.
+			 */
+			chain = hammer2_chain_next(&parent, chain, &key_next,
+						   key_next, HAMMER2_KEY_MAX,
+						   &error, 0);
+			continue;
+		}
 		if (chain->bref.type != HAMMER2_BREF_TYPE_INODE)
 			continue;
 		ripdata = &chain->data->ipdata;