DragonFlyBSD Kernel Audit
DF-2618 / fix.diff
← back to finding ↓ download raw
--- a/sys/vfs/hammer2/hammer2_chain.c
+++ b/sys/vfs/hammer2/hammer2_chain.c
@@ -292,7 +292,7 @@
 hammer2_chain_insert(hammer2_chain_t *parent, hammer2_chain_t *chain,
 		     int flags, int generation)
 {
-	hammer2_chain_t *xchain __debugvar;
+	hammer2_chain_t *xchain;
 	int error = 0;
 
 	if (flags & HAMMER2_CHAIN_INSERT_SPIN)
@@ -311,9 +311,23 @@
 	 * Insert chain
 	 */
 	xchain = RB_INSERT(hammer2_chain_tree, &parent->core.rbtree, chain);
-	KASSERT(xchain == NULL,
-		("hammer2_chain_insert: collision %p %p (key=%016jx)",
-		chain, xchain, chain->bref.key));
+	if (xchain != NULL) {
+		/*
+		 * Overlapping key range (hammer2_chain_cmp() returns 0 on
+		 * overlap).  This generally indicates a corrupted blockref
+		 * array on media.  The chain was NOT linked into the tree,
+		 * so do not set ONRBTREE or chain->parent and do not mess
+		 * with the parent's accounting.  Fail the insertion; the
+		 * caller must skip the entry (leaving the flags set would
+		 * later cause RB_REMOVE to wipe the parent's tree root).
+		 */
+		krateprintf(&krate_h2chk,
+			    "hammer2_chain_insert: collision %p %p "
+			    "(key=%016jx)\n",
+			    chain, xchain, chain->bref.key);
+		error = HAMMER2_ERROR_CHECK;
+		goto failed;
+	}
 	atomic_set_int(&chain->flags, HAMMER2_CHAIN_ONRBTREE);
 	chain->parent = parent;
 	++parent->core.chain_count;
@@ -2092,10 +2106,21 @@
 				     generation);
 	if (error) {
 		KKASSERT((chain->flags & HAMMER2_CHAIN_ONRBTREE) == 0);
-		/*kprintf("chain %p get race\n", chain);*/
-		hammer2_chain_unlock(chain);
-		hammer2_chain_drop(chain);
-		chain = NULL;
+		if (error == HAMMER2_ERROR_EAGAIN) {
+			/*kprintf("chain %p get race\n", chain);*/
+			hammer2_chain_unlock(chain);
+			hammer2_chain_drop(chain);
+			chain = NULL;
+		} else {
+			/*
+			 * Overlapping blockref on media (corruption): the
+			 * chain is NOT in the rbtree.  Return it locked with
+			 * the error set so callers can detect the failure
+			 * and skip past the entry instead of retrying the
+			 * same blockref forever.
+			 */
+			chain->error = error;
+		}
 	} else {
 		KKASSERT(chain->flags & HAMMER2_CHAIN_ONRBTREE);
 	}
@@ -2615,6 +2640,21 @@
 		}
 		if (chain == NULL)
 			goto again;
+		if (chain->error == HAMMER2_ERROR_CHECK &&
+		    (chain->flags & HAMMER2_CHAIN_ONRBTREE) == 0) {
+			/*
+			 * Overlapping blockref on media (corruption):
+			 * skip past the bad entry's key range.
+			 */
+			key_beg = bsave.key +
+				  ((hammer2_key_t)1 << bsave.keybits);
+			hammer2_chain_unlock(chain);
+			hammer2_chain_drop(chain);
+			chain = NULL;
+			if (key_beg == 0 || key_beg > key_end)
+				return (NULL);
+			goto again;
+		}
 	} else {
 		hammer2_chain_ref(chain);
 		hammer2_spin_unex(&parent->core.spin);
@@ -2986,6 +3026,23 @@
 						  bref, how);
 			if (chain == NULL)
 				goto again;
+			if (chain->error == HAMMER2_ERROR_CHECK &&
+			    (chain->flags & HAMMER2_CHAIN_ONRBTREE) == 0) {
+				/*
+				 * Overlapping blockref on media (corruption):
+				 * skip past the bad entry's key range.
+				 */
+				key = bref->key +
+				       ((hammer2_key_t)1 << bref->keybits);
+				hammer2_chain_unlock(chain);
+				hammer2_chain_drop(chain);
+				chain = NULL;
+				if (key == 0) {
+					error |= HAMMER2_ERROR_EOF;
+					goto done;
+				}
+				goto again;
+			}
 			break;
 		default:
 			/*
@@ -3322,10 +3379,19 @@
 	KKASSERT(chain->parent == NULL);
 	if (parent) {
 		KKASSERT(parent->core.live_count < count);
-		hammer2_chain_insert(parent, chain,
-				     HAMMER2_CHAIN_INSERT_SPIN |
-				     HAMMER2_CHAIN_INSERT_LIVE,
-				     0);
+		if (hammer2_chain_insert(parent, chain,
+					 HAMMER2_CHAIN_INSERT_SPIN |
+					 HAMMER2_CHAIN_INSERT_LIVE,
+					 0) != 0) {
+			/*
+			 * Overlapping key range (media corruption or a
+			 * software bug): the chain is not linked into the
+			 * parent.  Mark the chain errored so callers can
+			 * fail cleanly; do not set ONRBTREE (the chain will
+			 * be destroyed on drop since it was never linked).
+			 */
+			chain->error = HAMMER2_ERROR_CHECK;
+		}
 	}
 
 	if (allocated) {
@@ -3941,6 +4007,17 @@
 			hammer2_spin_unex(&parent->core.spin);
 			chain = hammer2_chain_get(parent, generation, &bsave,
 						  HAMMER2_RESOLVE_NEVER);
+			if (chain && chain->error == HAMMER2_ERROR_CHECK &&
+			    (chain->flags & HAMMER2_CHAIN_ONRBTREE) == 0) {
+				/*
+				 * Overlapping blockref on media: skip it.
+				 */
+				hammer2_chain_unlock(chain);
+				hammer2_chain_drop(chain);
+				chain = NULL;
+				hammer2_spin_ex(&parent->core.spin);
+				goto next_key_spinlocked;
+			}
 			if (chain == NULL) {
 				reason = 1;
 				hammer2_spin_ex(&parent->core.spin);
@@ -4194,6 +4271,20 @@
 			hammer2_spin_unex(&chain->core.spin);
 			sub = hammer2_chain_get(chain, generation, &bsave,
 						HAMMER2_RESOLVE_NEVER);
+			if (sub && sub->error == HAMMER2_ERROR_CHECK &&
+			    (sub->flags & HAMMER2_CHAIN_ONRBTREE) == 0) {
+				/*
+				 * Overlapping blockref on media: skip it.
+				 */
+				hammer2_chain_unlock(sub);
+				hammer2_chain_drop(sub);
+				sub = NULL;
+				hammer2_spin_ex(&chain->core.spin);
+				if (key_next == 0)
+					break;
+				key_beg = key_next;
+				continue;
+			}
 			if (sub == NULL) {
 				hammer2_spin_ex(&chain->core.spin);
 				continue;