[PATCH 0/3] btrfs: add missing NULL checks when searching for roots

public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] btrfs: add missing NULL checks when searching for roots
@ 2026-02-08 20:00 fdmanana
  2026-02-08 20:00 ` [PATCH 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: fdmanana @ 2026-02-08 20:00 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

Remove a wrong but harmless expression before loading an extent root and
add missing error handling for some root search functions, which can
return NULL when using the extent tree v2 feature. Some places have the
NULL checks already, but many others completely ignore it. Chris recently
reported this with his AI generated reviews.

Filipe Manana (3):
  btrfs: remove bogus root search condition in load_extent_tree_free()
  btrfs: check for NULL root after calls to btrfs_extent_root()
  btrfs: check for NULL root after calls to btrfs_csum_root()

 fs/btrfs/backref.c         | 28 +++++++++++
 fs/btrfs/block-group.c     | 39 ++++++++++++++-
 fs/btrfs/disk-io.c         | 17 ++++++-
 fs/btrfs/extent-tree.c     | 97 ++++++++++++++++++++++++++++++++++++--
 fs/btrfs/file-item.c       |  7 +++
 fs/btrfs/free-space-tree.c |  7 +++
 fs/btrfs/inode.c           | 18 ++++++-
 fs/btrfs/qgroup.c          |  8 ++++
 fs/btrfs/raid56.c          | 12 ++++-
 fs/btrfs/relocation.c      | 30 +++++++++++-
 fs/btrfs/tree-log.c        | 21 +++++++++
 fs/btrfs/zoned.c           |  7 +++
 12 files changed, 276 insertions(+), 15 deletions(-)

-- 
2.47.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] btrfs: remove bogus root search condition in load_extent_tree_free()
  2026-02-08 20:00 [PATCH 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
@ 2026-02-08 20:00 ` fdmanana
  2026-02-08 20:00 ` [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: fdmanana @ 2026-02-08 20:00 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

There's no need to pass the maximum between the block group's start offset
and BTRFS_SUPER_INFO_OFFSET (64K) since we can't have any block groups
allocated in the first megabyte, as that's reserved space. Furthermore,
even if we could, the correct thing to do was to pass the block group's
start offset anyway - and that's precisely what we do for block groups
hat happen to contain a superblock mirror (the range for the super block
is never marked as free and it's marked as dirty in the
fs_info->excluded_extents io tree).

So simplify this and get rid of that maximum expression.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/block-group.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 764caaf1d8b2..17a18f17538d 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -728,7 +728,7 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 	u64 total_found = 0;
-	u64 last = 0;
+	u64 last = block_group->start;
 	u32 nritems;
 	int ret;
 	bool wakeup = true;
@@ -737,7 +737,6 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	if (!path)
 		return -ENOMEM;
 
-	last = max_t(u64, block_group->start, BTRFS_SUPER_INFO_OFFSET);
 	extent_root = btrfs_extent_root(fs_info, last);
 
 #ifdef CONFIG_BTRFS_DEBUG
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root()
  2026-02-08 20:00 [PATCH 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  2026-02-08 20:00 ` [PATCH 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
@ 2026-02-08 20:00 ` fdmanana
  2026-02-09 15:41   ` Filipe Manana
  2026-02-08 20:00 ` [PATCH 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  3 siblings, 1 reply; 14+ messages in thread
From: fdmanana @ 2026-02-08 20:00 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

btrfs_extent_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error. The same applies to callers of btrfs_block_group_root(),
since it calls btrfs_extent_root().

Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/backref.c         | 28 ++++++++++++++
 fs/btrfs/block-group.c     | 36 ++++++++++++++++++
 fs/btrfs/disk-io.c         | 13 ++++++-
 fs/btrfs/extent-tree.c     | 78 ++++++++++++++++++++++++++++++++++++--
 fs/btrfs/free-space-tree.c |  7 ++++
 fs/btrfs/qgroup.c          |  8 ++++
 fs/btrfs/relocation.c      | 22 ++++++++++-
 fs/btrfs/zoned.c           |  7 ++++
 8 files changed, 192 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 9bb406f7dd30..7921a926f676 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1393,6 +1393,13 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
 		.indirect_missing_keys = PREFTREE_INIT
 	};
 
+	if (unlikely(!root)) {
+		btrfs_err(ctx->fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  ctx->bytenr);
+		return -EUCLEAN;
+	}
+
 	/* Roots ulist is not needed when using a sharedness check context. */
 	if (sc)
 		ASSERT(ctx->roots == NULL);
@@ -2204,6 +2211,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
 	struct btrfs_extent_item *ei;
 	struct btrfs_key key;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  logical);
+		return -EUCLEAN;
+	}
+
 	key.objectid = logical;
 	if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
 		key.type = BTRFS_METADATA_ITEM_KEY;
@@ -2851,6 +2865,13 @@ int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_METADATA_ITEM_KEY;
 	key.offset = (u64)-1;
@@ -2987,6 +3008,13 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
 
 	/* We're at keyed items, there is no inline item, go to the next one */
 	extent_root = btrfs_extent_root(iter->fs_info, iter->bytenr);
+	if (unlikely(!extent_root)) {
+		btrfs_err(iter->fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  iter->bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_next_item(extent_root, iter->path);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 17a18f17538d..36ff6c5a8f51 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -738,6 +738,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 		return -ENOMEM;
 
 	extent_root = btrfs_extent_root(fs_info, last);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  block_group->start);
+		return -EUCLEAN;
+	}
 
 #ifdef CONFIG_BTRFS_DEBUG
 	/*
@@ -1060,6 +1066,11 @@ static int remove_block_group_item(struct btrfs_trans_handle *trans,
 	int ret;
 
 	root = btrfs_block_group_root(fs_info);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	key.objectid = block_group->start;
 	key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
 	key.offset = block_group->length;
@@ -1348,6 +1359,11 @@ struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
 	struct btrfs_chunk_map *map;
 	unsigned int num_items;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return ERR_PTR(-EUCLEAN);
+	}
+
 	map = btrfs_find_chunk_map(fs_info, chunk_offset, 1);
 	ASSERT(map != NULL);
 	ASSERT(map->start == chunk_offset);
@@ -2139,6 +2155,11 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
 	int ret;
 	struct btrfs_key found_key;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	btrfs_for_each_slot(root, key, &found_key, path, ret) {
 		if (found_key.objectid >= key->objectid &&
 		    found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
@@ -2713,6 +2734,11 @@ static int insert_block_group_item(struct btrfs_trans_handle *trans,
 	size_t size;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	spin_lock(&block_group->lock);
 	btrfs_set_stack_block_group_v2_used(&bgi, block_group->used);
 	btrfs_set_stack_block_group_v2_chunk_objectid(&bgi, block_group->global_root_id);
@@ -3048,6 +3074,11 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
 	int ret;
 	bool dirty_bg_running;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	/*
 	 * This can only happen when we are doing read-only scrub on read-only
 	 * mount.
@@ -3192,6 +3223,11 @@ static int update_block_group_item(struct btrfs_trans_handle *trans,
 	u64 used, remap_bytes;
 	u32 identity_remap_count;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	/*
 	 * Block group items update can be triggered out of commit transaction
 	 * critical section, thus we need a consistent view of used bytes.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 67117e7516bf..1194cb182a91 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1591,7 +1591,7 @@ static int find_newest_super_backup(struct btrfs_fs_info *info)
  * this will bump the backup pointer by one when it is
  * done
  */
-static void backup_super_roots(struct btrfs_fs_info *info)
+static int backup_super_roots(struct btrfs_fs_info *info)
 {
 	const int next_backup = info->backup_root_index;
 	struct btrfs_root_backup *root_backup;
@@ -1623,6 +1623,11 @@ static void backup_super_roots(struct btrfs_fs_info *info)
 		struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
 		struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
 
+		if (unlikely(!extent_root)) {
+			btrfs_err(info, "missing extent root for extent at bytenr 0");
+			return -EUCLEAN;
+		}
+
 		btrfs_set_backup_extent_root(root_backup,
 					     extent_root->node->start);
 		btrfs_set_backup_extent_root_gen(root_backup,
@@ -1670,6 +1675,8 @@ static void backup_super_roots(struct btrfs_fs_info *info)
 	memcpy(&info->super_copy->super_roots,
 	       &info->super_for_commit->super_roots,
 	       sizeof(*root_backup) * BTRFS_NUM_BACKUP_ROOTS);
+
+	return 0;
 }
 
 /*
@@ -4032,7 +4039,9 @@ int write_all_supers(struct btrfs_trans_handle *trans)
 	} else {
 		/* We are called from transaction commit. */
 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
-		backup_super_roots(fs_info);
+		ret = backup_super_roots(fs_info);
+		if (ret < 0)
+			return ret;
 	}
 
 	sb = fs_info->super_for_commit;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 01bbe3cae5c2..331263c206af 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -75,6 +75,12 @@ int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
 	struct btrfs_key key;
 	BTRFS_PATH_AUTO_FREE(path);
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", start);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
@@ -131,6 +137,12 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 	key.offset = offset;
 
 	extent_root = btrfs_extent_root(fs_info, bytenr);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
 	if (ret < 0)
 		return ret;
@@ -436,6 +448,12 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
 	int recow;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (parent) {
 		key.type = BTRFS_SHARED_DATA_REF_KEY;
@@ -510,6 +528,12 @@ static noinline int insert_extent_data_ref(struct btrfs_trans_handle *trans,
 	u32 num_refs;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (node->parent) {
 		key.type = BTRFS_SHARED_DATA_REF_KEY;
@@ -668,6 +692,12 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (parent) {
 		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
@@ -692,6 +722,12 @@ static noinline int insert_tree_block_ref(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (node->parent) {
 		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
@@ -782,6 +818,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
 	bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
 	int needed;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_EXTENT_ITEM_KEY;
 	key.offset = num_bytes;
@@ -1680,6 +1722,12 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
 	}
 
 	root = btrfs_extent_root(fs_info, key.objectid);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  key.objectid);
+		return -EUCLEAN;
+	}
 again:
 	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
 	if (ret < 0) {
@@ -2379,6 +2427,12 @@ static noinline int check_committed_ref(struct btrfs_inode *inode,
 	int type;
 	int ret;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_EXTENT_ITEM_KEY;
 	key.offset = (u64)-1;
@@ -3222,7 +3276,11 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 	u64 delayed_ref_root = href->owning_root;
 
 	extent_root = btrfs_extent_root(info, bytenr);
-	ASSERT(extent_root);
+	if (unlikely(!extent_root)) {
+		btrfs_err(info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -4935,11 +4993,18 @@ static int alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
 		size += btrfs_extent_inline_ref_size(BTRFS_EXTENT_OWNER_REF_KEY);
 	size += btrfs_extent_inline_ref_size(type);
 
+	extent_root = btrfs_extent_root(fs_info, ins->objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  ins->objectid);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
-	extent_root = btrfs_extent_root(fs_info, ins->objectid);
 	ret = btrfs_insert_empty_item(trans, extent_root, path, ins, size);
 	if (ret) {
 		btrfs_free_path(path);
@@ -5015,11 +5080,18 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
 		size += sizeof(*block_info);
 	}
 
+	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  extent_key.objectid);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
-	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
 	ret = btrfs_insert_empty_item(trans, extent_root, path, &extent_key,
 				      size);
 	if (ret) {
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index ecddfca92b2b..f5caa0da5214 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1087,6 +1087,13 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
 	key.offset = 0;
 
 	extent_root = btrfs_extent_root(trans->fs_info, key.objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for block group at offset %llu",
+			  key.objectid);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot_for_read(extent_root, &key, path, 1, 0);
 	if (ret < 0)
 		goto out_locked;
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index f53c313ab6e4..8eb7739a381f 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3737,6 +3737,14 @@ static int qgroup_rescan_leaf(struct btrfs_trans_handle *trans,
 	mutex_lock(&fs_info->qgroup_rescan_lock);
 	extent_root = btrfs_extent_root(fs_info,
 				fs_info->qgroup_rescan_progress.objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  fs_info->qgroup_rescan_progress.objectid);
+		mutex_unlock(&fs_info->qgroup_rescan_lock);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot_for_read(extent_root,
 					 &fs_info->qgroup_rescan_progress,
 					 path, 1, 0);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index fcd0a2ba3554..14de065e79fc 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4949,6 +4949,12 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
 	struct btrfs_space_info *sinfo = src_bg->space_info;
 
 	extent_root = btrfs_extent_root(fs_info, src_bg->start);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  src_bg->start);
+		return -EUCLEAN;
+	}
 
 	trans = btrfs_start_transaction(extent_root, 0);
 	if (IS_ERR(trans))
@@ -5301,6 +5307,13 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start,
 	int ret;
 	bool bg_is_ro = false;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  group_start);
+		return -EUCLEAN;
+	}
+
 	/*
 	 * This only gets set if we had a half-deleted snapshot on mount.  We
 	 * cannot allow relocation to start while we're still trying to clean up
@@ -5531,12 +5544,17 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
 		goto out;
 	}
 
+	rc->extent_root = btrfs_extent_root(fs_info, 0);
+	if (unlikely(!rc->extent_root)) {
+		btrfs_err(fs_info, "missing extent root for extent at bytenr 0");
+		ret = -EUCLEAN;
+		goto out;
+	}
+
 	ret = reloc_chunk_start(fs_info);
 	if (ret < 0)
 		goto out_end;
 
-	rc->extent_root = btrfs_extent_root(fs_info, 0);
-
 	set_reloc_control(rc);
 
 	trans = btrfs_join_transaction(rc->extent_root);
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index ae6f61bcefca..4a1dabeea531 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1259,6 +1259,13 @@ static int calculate_alloc_pointer(struct btrfs_block_group *cache,
 	key.offset = 0;
 
 	root = btrfs_extent_root(fs_info, key.objectid);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  key.objectid);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	/* We should not find the exact match */
 	if (unlikely(!ret))
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root()
  2026-02-08 20:00 ` [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
@ 2026-02-09 15:41   ` Filipe Manana
  0 siblings, 0 replies; 14+ messages in thread
From: Filipe Manana @ 2026-02-09 15:41 UTC (permalink / raw)
  To: linux-btrfs

On Sun, Feb 8, 2026 at 8:01 PM <fdmanana@kernel.org> wrote:
>
> From: Filipe Manana <fdmanana@suse.com>
>
> btrfs_extent_root() can return a NULL pointer in case the root we are
> looking for is not in the rb tree that tracks roots. So add checks to
> every caller that is missing such check to log a message and return
> an error. The same applies to callers of btrfs_block_group_root(),
> since it calls btrfs_extent_root().
>
> Reported-by: Chris Mason <clm@meta.com>
> Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/backref.c         | 28 ++++++++++++++
>  fs/btrfs/block-group.c     | 36 ++++++++++++++++++
>  fs/btrfs/disk-io.c         | 13 ++++++-
>  fs/btrfs/extent-tree.c     | 78 ++++++++++++++++++++++++++++++++++++--
>  fs/btrfs/free-space-tree.c |  7 ++++
>  fs/btrfs/qgroup.c          |  8 ++++
>  fs/btrfs/relocation.c      | 22 ++++++++++-
>  fs/btrfs/zoned.c           |  7 ++++
>  8 files changed, 192 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 9bb406f7dd30..7921a926f676 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1393,6 +1393,13 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
>                 .indirect_missing_keys = PREFTREE_INIT
>         };
>
> +       if (unlikely(!root)) {
> +               btrfs_err(ctx->fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         ctx->bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         /* Roots ulist is not needed when using a sharedness check context. */
>         if (sc)
>                 ASSERT(ctx->roots == NULL);
> @@ -2204,6 +2211,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
>         struct btrfs_extent_item *ei;
>         struct btrfs_key key;
>
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         logical);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = logical;
>         if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
>                 key.type = BTRFS_METADATA_ITEM_KEY;
> @@ -2851,6 +2865,13 @@ int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
>         struct btrfs_key key;
>         int ret;
>
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         key.type = BTRFS_METADATA_ITEM_KEY;
>         key.offset = (u64)-1;
> @@ -2987,6 +3008,13 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
>
>         /* We're at keyed items, there is no inline item, go to the next one */
>         extent_root = btrfs_extent_root(iter->fs_info, iter->bytenr);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(iter->fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         iter->bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         ret = btrfs_next_item(extent_root, iter->path);
>         if (ret)
>                 return ret;
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 17a18f17538d..36ff6c5a8f51 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -738,6 +738,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
>                 return -ENOMEM;
>
>         extent_root = btrfs_extent_root(fs_info, last);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for block group at offset %llu",
> +                         block_group->start);
> +               return -EUCLEAN;
> +       }
>
>  #ifdef CONFIG_BTRFS_DEBUG
>         /*
> @@ -1060,6 +1066,11 @@ static int remove_block_group_item(struct btrfs_trans_handle *trans,
>         int ret;
>
>         root = btrfs_block_group_root(fs_info);
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = block_group->start;
>         key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
>         key.offset = block_group->length;
> @@ -1348,6 +1359,11 @@ struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
>         struct btrfs_chunk_map *map;
>         unsigned int num_items;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return ERR_PTR(-EUCLEAN);
> +       }
> +
>         map = btrfs_find_chunk_map(fs_info, chunk_offset, 1);
>         ASSERT(map != NULL);
>         ASSERT(map->start == chunk_offset);
> @@ -2139,6 +2155,11 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
>         int ret;
>         struct btrfs_key found_key;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return -EUCLEAN;
> +       }
> +
>         btrfs_for_each_slot(root, key, &found_key, path, ret) {
>                 if (found_key.objectid >= key->objectid &&
>                     found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
> @@ -2713,6 +2734,11 @@ static int insert_block_group_item(struct btrfs_trans_handle *trans,
>         size_t size;
>         int ret;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return -EUCLEAN;
> +       }
> +
>         spin_lock(&block_group->lock);
>         btrfs_set_stack_block_group_v2_used(&bgi, block_group->used);
>         btrfs_set_stack_block_group_v2_chunk_objectid(&bgi, block_group->global_root_id);
> @@ -3048,6 +3074,11 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
>         int ret;
>         bool dirty_bg_running;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return -EUCLEAN;
> +       }
> +
>         /*
>          * This can only happen when we are doing read-only scrub on read-only
>          * mount.
> @@ -3192,6 +3223,11 @@ static int update_block_group_item(struct btrfs_trans_handle *trans,
>         u64 used, remap_bytes;
>         u32 identity_remap_count;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info, "missing block group root");
> +               return -EUCLEAN;
> +       }
> +
>         /*
>          * Block group items update can be triggered out of commit transaction
>          * critical section, thus we need a consistent view of used bytes.
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 67117e7516bf..1194cb182a91 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1591,7 +1591,7 @@ static int find_newest_super_backup(struct btrfs_fs_info *info)
>   * this will bump the backup pointer by one when it is
>   * done
>   */
> -static void backup_super_roots(struct btrfs_fs_info *info)
> +static int backup_super_roots(struct btrfs_fs_info *info)
>  {
>         const int next_backup = info->backup_root_index;
>         struct btrfs_root_backup *root_backup;
> @@ -1623,6 +1623,11 @@ static void backup_super_roots(struct btrfs_fs_info *info)
>                 struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
>                 struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
>
> +               if (unlikely(!extent_root)) {
> +                       btrfs_err(info, "missing extent root for extent at bytenr 0");
> +                       return -EUCLEAN;
> +               }
> +
>                 btrfs_set_backup_extent_root(root_backup,
>                                              extent_root->node->start);
>                 btrfs_set_backup_extent_root_gen(root_backup,
> @@ -1670,6 +1675,8 @@ static void backup_super_roots(struct btrfs_fs_info *info)
>         memcpy(&info->super_copy->super_roots,
>                &info->super_for_commit->super_roots,
>                sizeof(*root_backup) * BTRFS_NUM_BACKUP_ROOTS);
> +
> +       return 0;
>  }
>
>  /*
> @@ -4032,7 +4039,9 @@ int write_all_supers(struct btrfs_trans_handle *trans)
>         } else {
>                 /* We are called from transaction commit. */
>                 max_mirrors = BTRFS_SUPER_MIRROR_MAX;
> -               backup_super_roots(fs_info);
> +               ret = backup_super_roots(fs_info);
> +               if (ret < 0)
> +                       return ret;
>         }
>
>         sb = fs_info->super_for_commit;
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 01bbe3cae5c2..331263c206af 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -75,6 +75,12 @@ int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
>         struct btrfs_key key;
>         BTRFS_PATH_AUTO_FREE(path);
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu", start);
> +               return -EUCLEAN;
> +       }
> +
>         path = btrfs_alloc_path();
>         if (!path)
>                 return -ENOMEM;
> @@ -131,6 +137,12 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>         key.offset = offset;
>
>         extent_root = btrfs_extent_root(fs_info, bytenr);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
>         if (ret < 0)
>                 return ret;
> @@ -436,6 +448,12 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
>         int recow;
>         int ret;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(trans->fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         if (parent) {
>                 key.type = BTRFS_SHARED_DATA_REF_KEY;
> @@ -510,6 +528,12 @@ static noinline int insert_extent_data_ref(struct btrfs_trans_handle *trans,
>         u32 num_refs;
>         int ret;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(trans->fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         if (node->parent) {
>                 key.type = BTRFS_SHARED_DATA_REF_KEY;
> @@ -668,6 +692,12 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
>         struct btrfs_key key;
>         int ret;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(trans->fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         if (parent) {
>                 key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> @@ -692,6 +722,12 @@ static noinline int insert_tree_block_ref(struct btrfs_trans_handle *trans,
>         struct btrfs_key key;
>         int ret;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(trans->fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         if (node->parent) {
>                 key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> @@ -782,6 +818,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
>         bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
>         int needed;
>
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         key.type = BTRFS_EXTENT_ITEM_KEY;
>         key.offset = num_bytes;
> @@ -1680,6 +1722,12 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
>         }
>
>         root = btrfs_extent_root(fs_info, key.objectid);
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         key.objectid);
> +               return -EUCLEAN;
> +       }
>  again:
>         ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
>         if (ret < 0) {
> @@ -2379,6 +2427,12 @@ static noinline int check_committed_ref(struct btrfs_inode *inode,
>         int type;
>         int ret;
>
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
> +
>         key.objectid = bytenr;
>         key.type = BTRFS_EXTENT_ITEM_KEY;
>         key.offset = (u64)-1;
> @@ -3222,7 +3276,11 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>         u64 delayed_ref_root = href->owning_root;
>
>         extent_root = btrfs_extent_root(info, bytenr);
> -       ASSERT(extent_root);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(info,
> +                         "missing extent root for extent at bytenr %llu", bytenr);
> +               return -EUCLEAN;
> +       }
>
>         path = btrfs_alloc_path();
>         if (!path)
> @@ -4935,11 +4993,18 @@ static int alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
>                 size += btrfs_extent_inline_ref_size(BTRFS_EXTENT_OWNER_REF_KEY);
>         size += btrfs_extent_inline_ref_size(type);
>
> +       extent_root = btrfs_extent_root(fs_info, ins->objectid);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         ins->objectid);
> +               return -EUCLEAN;
> +       }
> +
>         path = btrfs_alloc_path();
>         if (!path)
>                 return -ENOMEM;
>
> -       extent_root = btrfs_extent_root(fs_info, ins->objectid);
>         ret = btrfs_insert_empty_item(trans, extent_root, path, ins, size);
>         if (ret) {
>                 btrfs_free_path(path);
> @@ -5015,11 +5080,18 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>                 size += sizeof(*block_info);
>         }
>
> +       extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         extent_key.objectid);
> +               return -EUCLEAN;
> +       }
> +
>         path = btrfs_alloc_path();
>         if (!path)
>                 return -ENOMEM;
>
> -       extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
>         ret = btrfs_insert_empty_item(trans, extent_root, path, &extent_key,
>                                       size);
>         if (ret) {
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index ecddfca92b2b..f5caa0da5214 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -1087,6 +1087,13 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
>         key.offset = 0;
>
>         extent_root = btrfs_extent_root(trans->fs_info, key.objectid);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(trans->fs_info,
> +                         "missing extent root for block group at offset %llu",
> +                         key.objectid);
> +               return -EUCLEAN;

Misses a:

mutex_unlock(&block_group->free_space_lock);

Will add to v2 or update before picking into for-next.

> +       }
> +
>         ret = btrfs_search_slot_for_read(extent_root, &key, path, 1, 0);
>         if (ret < 0)
>                 goto out_locked;
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index f53c313ab6e4..8eb7739a381f 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -3737,6 +3737,14 @@ static int qgroup_rescan_leaf(struct btrfs_trans_handle *trans,
>         mutex_lock(&fs_info->qgroup_rescan_lock);
>         extent_root = btrfs_extent_root(fs_info,
>                                 fs_info->qgroup_rescan_progress.objectid);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         fs_info->qgroup_rescan_progress.objectid);
> +               mutex_unlock(&fs_info->qgroup_rescan_lock);
> +               return -EUCLEAN;
> +       }
> +
>         ret = btrfs_search_slot_for_read(extent_root,
>                                          &fs_info->qgroup_rescan_progress,
>                                          path, 1, 0);
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index fcd0a2ba3554..14de065e79fc 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4949,6 +4949,12 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
>         struct btrfs_space_info *sinfo = src_bg->space_info;
>
>         extent_root = btrfs_extent_root(fs_info, src_bg->start);
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for block group at offset %llu",
> +                         src_bg->start);
> +               return -EUCLEAN;
> +       }
>
>         trans = btrfs_start_transaction(extent_root, 0);
>         if (IS_ERR(trans))
> @@ -5301,6 +5307,13 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start,
>         int ret;
>         bool bg_is_ro = false;
>
> +       if (unlikely(!extent_root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for block group at offset %llu",
> +                         group_start);
> +               return -EUCLEAN;
> +       }
> +
>         /*
>          * This only gets set if we had a half-deleted snapshot on mount.  We
>          * cannot allow relocation to start while we're still trying to clean up
> @@ -5531,12 +5544,17 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
>                 goto out;
>         }
>
> +       rc->extent_root = btrfs_extent_root(fs_info, 0);
> +       if (unlikely(!rc->extent_root)) {
> +               btrfs_err(fs_info, "missing extent root for extent at bytenr 0");
> +               ret = -EUCLEAN;
> +               goto out;
> +       }
> +
>         ret = reloc_chunk_start(fs_info);
>         if (ret < 0)
>                 goto out_end;
>
> -       rc->extent_root = btrfs_extent_root(fs_info, 0);
> -
>         set_reloc_control(rc);
>
>         trans = btrfs_join_transaction(rc->extent_root);
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index ae6f61bcefca..4a1dabeea531 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1259,6 +1259,13 @@ static int calculate_alloc_pointer(struct btrfs_block_group *cache,
>         key.offset = 0;
>
>         root = btrfs_extent_root(fs_info, key.objectid);
> +       if (unlikely(!root)) {
> +               btrfs_err(fs_info,
> +                         "missing extent root for extent at bytenr %llu",
> +                         key.objectid);
> +               return -EUCLEAN;
> +       }
> +
>         ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>         /* We should not find the exact match */
>         if (unlikely(!ret))
> --
> 2.47.2
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] btrfs: check for NULL root after calls to btrfs_csum_root()
  2026-02-08 20:00 [PATCH 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  2026-02-08 20:00 ` [PATCH 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
  2026-02-08 20:00 ` [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
@ 2026-02-08 20:00 ` fdmanana
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  3 siblings, 0 replies; 14+ messages in thread
From: fdmanana @ 2026-02-08 20:00 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

btrfs_csum_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error.

Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/disk-io.c     |  4 ++++
 fs/btrfs/extent-tree.c | 19 +++++++++++++++++--
 fs/btrfs/file-item.c   |  7 +++++++
 fs/btrfs/inode.c       | 18 ++++++++++++++++--
 fs/btrfs/raid56.c      | 12 ++++++++++--
 fs/btrfs/relocation.c  |  8 ++++++++
 fs/btrfs/tree-log.c    | 21 +++++++++++++++++++++
 7 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1194cb182a91..53237ec14d49 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1627,6 +1627,10 @@ static int backup_super_roots(struct btrfs_fs_info *info)
 			btrfs_err(info, "missing extent root for extent at bytenr 0");
 			return -EUCLEAN;
 		}
+		if (unlikely(!csum_root)) {
+			btrfs_err(info, "missing csum root for extent at bytenr 0");
+			return -EUCLEAN;
+		}
 
 		btrfs_set_backup_extent_root(root_backup,
 					     extent_root->node->start);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 331263c206af..aa83436ac2be 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1974,8 +1974,14 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans,
 			struct btrfs_root *csum_root;
 
 			csum_root = btrfs_csum_root(fs_info, head->bytenr);
-			ret = btrfs_del_csums(trans, csum_root, head->bytenr,
-					      head->num_bytes);
+			if (unlikely(!csum_root)) {
+				btrfs_err(fs_info,
+					  "missing csum root for extent at bytenr 0");
+				ret = -EUCLEAN;
+			} else {
+				ret = btrfs_del_csums(trans, csum_root, head->bytenr,
+						      head->num_bytes);
+			}
 		}
 	}
 
@@ -3147,6 +3153,15 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
 		struct btrfs_root *csum_root;
 
 		csum_root = btrfs_csum_root(trans->fs_info, bytenr);
+		if (unlikely(!csum_root)) {
+			ret = -EUCLEAN;
+			btrfs_abort_transaction(trans, ret);
+			btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  bytenr);
+			return ret;
+		}
+
 		ret = btrfs_del_csums(trans, csum_root, bytenr, num_bytes);
 		if (unlikely(ret)) {
 			btrfs_abort_transaction(trans, ret);
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 7bd715442f3e..f585ddfa8440 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -308,6 +308,13 @@ static int search_csum_tree(struct btrfs_fs_info *fs_info,
 	/* Current item doesn't contain the desired range, search again */
 	btrfs_release_path(path);
 	csum_root = btrfs_csum_root(fs_info, disk_bytenr);
+	if (unlikely(!csum_root)) {
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  disk_bytenr);
+		return -EUCLEAN;
+	}
+
 	item = btrfs_lookup_csum(NULL, csum_root, path, disk_bytenr, 0);
 	if (IS_ERR(item)) {
 		ret = PTR_ERR(item);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b6c763a17406..4753bfb7a15a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1997,6 +1997,13 @@ static int can_nocow_file_extent(struct btrfs_path *path,
 	 */
 
 	csum_root = btrfs_csum_root(root->fs_info, io_start);
+	if (unlikely(!csum_root)) {
+		btrfs_err(root->fs_info,
+			  "missing csum root for extent at bytenr %llu", io_start);
+		ret = -EUCLEAN;
+		goto out;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, io_start,
 				      io_start + args->file_extent.num_bytes - 1,
 				      NULL, nowait);
@@ -2734,10 +2741,17 @@ static int add_pending_csums(struct btrfs_trans_handle *trans,
 	int ret;
 
 	list_for_each_entry(sum, list, list) {
-		trans->adding_csums = true;
-		if (!csum_root)
+		if (!csum_root) {
 			csum_root = btrfs_csum_root(trans->fs_info,
 						    sum->logical);
+			if (unlikely(!csum_root)) {
+				btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+					  sum->logical);
+				return -EUCLEAN;
+			}
+		}
+		trans->adding_csums = true;
 		ret = btrfs_csum_file_blocks(trans, csum_root, sum);
 		trans->adding_csums = false;
 		if (ret)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index baadaaa189c0..230dd93dad6e 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2295,8 +2295,7 @@ void raid56_parity_recover(struct bio *bio, struct btrfs_io_context *bioc,
 static void fill_data_csums(struct btrfs_raid_bio *rbio)
 {
 	struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
-	struct btrfs_root *csum_root = btrfs_csum_root(fs_info,
-						       rbio->bioc->full_stripe_logical);
+	struct btrfs_root *csum_root;
 	const u64 start = rbio->bioc->full_stripe_logical;
 	const u32 len = (rbio->nr_data * rbio->stripe_nsectors) <<
 			fs_info->sectorsize_bits;
@@ -2329,6 +2328,15 @@ static void fill_data_csums(struct btrfs_raid_bio *rbio)
 		goto error;
 	}
 
+	csum_root = btrfs_csum_root(fs_info, rbio->bioc->full_stripe_logical);
+	if (unlikely(!csum_root)) {
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  rbio->bioc->full_stripe_logical);
+		ret = -EUCLEAN;
+		goto error;
+	}
+
 	ret = btrfs_lookup_csums_bitmap(csum_root, NULL, start, start + len - 1,
 					rbio->csum_buf, rbio->csum_bitmap);
 	if (ret < 0)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 14de065e79fc..64d140bea1fb 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -5649,6 +5649,14 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
 	LIST_HEAD(list);
 	int ret;
 
+	if (unlikely(!csum_root)) {
+		btrfs_mark_ordered_extent_error(ordered);
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  disk_bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 				      disk_bytenr + ordered->num_bytes - 1,
 				      &list, false);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index e2806ca410f6..e9655095ba4c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -984,6 +984,13 @@ static noinline int replay_one_extent(struct walk_control *wc)
 
 		sums = list_first_entry(&ordered_sums, struct btrfs_ordered_sum, list);
 		csum_root = btrfs_csum_root(fs_info, sums->logical);
+		if (unlikely(!csum_root)) {
+			btrfs_err(fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  sums->logical);
+			ret = -EUCLEAN;
+		}
+
 		if (!ret) {
 			ret = btrfs_del_csums(trans, csum_root, sums->logical,
 					      sums->len);
@@ -4890,6 +4897,13 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
 		}
 
 		csum_root = btrfs_csum_root(trans->fs_info, disk_bytenr);
+		if (unlikely(!csum_root)) {
+			btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  disk_bytenr);
+			return -EUCLEAN;
+		}
+
 		disk_bytenr += extent_offset;
 		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 					      disk_bytenr + extent_num_bytes - 1,
@@ -5086,6 +5100,13 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
 	/* block start is already adjusted for the file extent offset. */
 	block_start = btrfs_extent_map_block_start(em);
 	csum_root = btrfs_csum_root(trans->fs_info, block_start);
+	if (unlikely(!csum_root)) {
+		btrfs_err(trans->fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  block_start);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, block_start + csum_offset,
 				      block_start + csum_offset + csum_len - 1,
 				      &ordered_sums, false);
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots
  2026-02-08 20:00 [PATCH 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
                   ` (2 preceding siblings ...)
  2026-02-08 20:00 ` [PATCH 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
@ 2026-02-10 11:57 ` fdmanana
  2026-02-10 11:57   ` [PATCH v2 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
                     ` (3 more replies)
  3 siblings, 4 replies; 14+ messages in thread
From: fdmanana @ 2026-02-10 11:57 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

Remove a wrong but harmless expression before loading an extent root and
add missing error handling for some root search functions, which can
return NULL when using the extent tree v2 feature. Some places have the
NULL checks already, but many others completely ignore it. Chris recently
reported this with his AI generated reviews.

V2: Fixed a return without mutex unlock in patch 2/3, updated patch 3/3
    to log the logical bytenr of a delayed ref in case we can't find
    the csum root.

Filipe Manana (3):
  btrfs: remove bogus root search condition in load_extent_tree_free()
  btrfs: check for NULL root after calls to btrfs_extent_root()
  btrfs: check for NULL root after calls to btrfs_csum_root()

 fs/btrfs/backref.c         | 28 +++++++++++
 fs/btrfs/block-group.c     | 39 ++++++++++++++-
 fs/btrfs/disk-io.c         | 17 ++++++-
 fs/btrfs/extent-tree.c     | 98 ++++++++++++++++++++++++++++++++++++--
 fs/btrfs/file-item.c       |  7 +++
 fs/btrfs/free-space-tree.c |  9 +++-
 fs/btrfs/inode.c           | 18 ++++++-
 fs/btrfs/qgroup.c          |  8 ++++
 fs/btrfs/raid56.c          | 12 ++++-
 fs/btrfs/relocation.c      | 30 +++++++++++-
 fs/btrfs/tree-log.c        | 21 ++++++++
 fs/btrfs/zoned.c           |  7 +++
 12 files changed, 278 insertions(+), 16 deletions(-)

-- 
2.47.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/3] btrfs: remove bogus root search condition in load_extent_tree_free()
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
@ 2026-02-10 11:57   ` fdmanana
  2026-02-10 11:57   ` [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: fdmanana @ 2026-02-10 11:57 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

There's no need to pass the maximum between the block group's start offset
and BTRFS_SUPER_INFO_OFFSET (64K) since we can't have any block groups
allocated in the first megabyte, as that's reserved space. Furthermore,
even if we could, the correct thing to do was to pass the block group's
start offset anyway - and that's precisely what we do for block groups
hat happen to contain a superblock mirror (the range for the super block
is never marked as free and it's marked as dirty in the
fs_info->excluded_extents io tree).

So simplify this and get rid of that maximum expression.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/block-group.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 764caaf1d8b2..17a18f17538d 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -728,7 +728,7 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	struct extent_buffer *leaf;
 	struct btrfs_key key;
 	u64 total_found = 0;
-	u64 last = 0;
+	u64 last = block_group->start;
 	u32 nritems;
 	int ret;
 	bool wakeup = true;
@@ -737,7 +737,6 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 	if (!path)
 		return -ENOMEM;
 
-	last = max_t(u64, block_group->start, BTRFS_SUPER_INFO_OFFSET);
 	extent_root = btrfs_extent_root(fs_info, last);
 
 #ifdef CONFIG_BTRFS_DEBUG
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root()
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  2026-02-10 11:57   ` [PATCH v2 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
@ 2026-02-10 11:57   ` fdmanana
  2026-02-11 17:34     ` Boris Burkov
  2026-02-10 11:57   ` [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
  2026-02-11 17:40   ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots Boris Burkov
  3 siblings, 1 reply; 14+ messages in thread
From: fdmanana @ 2026-02-10 11:57 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

btrfs_extent_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error. The same applies to callers of btrfs_block_group_root(),
since it calls btrfs_extent_root().

Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/backref.c         | 28 ++++++++++++++
 fs/btrfs/block-group.c     | 36 ++++++++++++++++++
 fs/btrfs/disk-io.c         | 13 ++++++-
 fs/btrfs/extent-tree.c     | 78 ++++++++++++++++++++++++++++++++++++--
 fs/btrfs/free-space-tree.c |  9 ++++-
 fs/btrfs/qgroup.c          |  8 ++++
 fs/btrfs/relocation.c      | 22 ++++++++++-
 fs/btrfs/zoned.c           |  7 ++++
 8 files changed, 193 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index cf10e10a40fa..8a119c505e32 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1388,6 +1388,13 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
 		.indirect_missing_keys = PREFTREE_INIT
 	};
 
+	if (unlikely(!root)) {
+		btrfs_err(ctx->fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  ctx->bytenr);
+		return -EUCLEAN;
+	}
+
 	/* Roots ulist is not needed when using a sharedness check context. */
 	if (sc)
 		ASSERT(ctx->roots == NULL);
@@ -2194,6 +2201,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
 	struct btrfs_extent_item *ei;
 	struct btrfs_key key;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  logical);
+		return -EUCLEAN;
+	}
+
 	key.objectid = logical;
 	if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
 		key.type = BTRFS_METADATA_ITEM_KEY;
@@ -2841,6 +2855,13 @@ int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_METADATA_ITEM_KEY;
 	key.offset = (u64)-1;
@@ -2977,6 +2998,13 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
 
 	/* We're at keyed items, there is no inline item, go to the next one */
 	extent_root = btrfs_extent_root(iter->fs_info, iter->bytenr);
+	if (unlikely(!extent_root)) {
+		btrfs_err(iter->fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  iter->bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_next_item(extent_root, iter->path);
 	if (ret)
 		return ret;
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 17a18f17538d..36ff6c5a8f51 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -738,6 +738,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
 		return -ENOMEM;
 
 	extent_root = btrfs_extent_root(fs_info, last);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  block_group->start);
+		return -EUCLEAN;
+	}
 
 #ifdef CONFIG_BTRFS_DEBUG
 	/*
@@ -1060,6 +1066,11 @@ static int remove_block_group_item(struct btrfs_trans_handle *trans,
 	int ret;
 
 	root = btrfs_block_group_root(fs_info);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	key.objectid = block_group->start;
 	key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
 	key.offset = block_group->length;
@@ -1348,6 +1359,11 @@ struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
 	struct btrfs_chunk_map *map;
 	unsigned int num_items;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return ERR_PTR(-EUCLEAN);
+	}
+
 	map = btrfs_find_chunk_map(fs_info, chunk_offset, 1);
 	ASSERT(map != NULL);
 	ASSERT(map->start == chunk_offset);
@@ -2139,6 +2155,11 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
 	int ret;
 	struct btrfs_key found_key;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	btrfs_for_each_slot(root, key, &found_key, path, ret) {
 		if (found_key.objectid >= key->objectid &&
 		    found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
@@ -2713,6 +2734,11 @@ static int insert_block_group_item(struct btrfs_trans_handle *trans,
 	size_t size;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	spin_lock(&block_group->lock);
 	btrfs_set_stack_block_group_v2_used(&bgi, block_group->used);
 	btrfs_set_stack_block_group_v2_chunk_objectid(&bgi, block_group->global_root_id);
@@ -3048,6 +3074,11 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
 	int ret;
 	bool dirty_bg_running;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	/*
 	 * This can only happen when we are doing read-only scrub on read-only
 	 * mount.
@@ -3192,6 +3223,11 @@ static int update_block_group_item(struct btrfs_trans_handle *trans,
 	u64 used, remap_bytes;
 	u32 identity_remap_count;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info, "missing block group root");
+		return -EUCLEAN;
+	}
+
 	/*
 	 * Block group items update can be triggered out of commit transaction
 	 * critical section, thus we need a consistent view of used bytes.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index e2a4d4dbf056..4b1e67f176a3 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1590,7 +1590,7 @@ static int find_newest_super_backup(struct btrfs_fs_info *info)
  * this will bump the backup pointer by one when it is
  * done
  */
-static void backup_super_roots(struct btrfs_fs_info *info)
+static int backup_super_roots(struct btrfs_fs_info *info)
 {
 	const int next_backup = info->backup_root_index;
 	struct btrfs_root_backup *root_backup;
@@ -1622,6 +1622,11 @@ static void backup_super_roots(struct btrfs_fs_info *info)
 		struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
 		struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
 
+		if (unlikely(!extent_root)) {
+			btrfs_err(info, "missing extent root for extent at bytenr 0");
+			return -EUCLEAN;
+		}
+
 		btrfs_set_backup_extent_root(root_backup,
 					     extent_root->node->start);
 		btrfs_set_backup_extent_root_gen(root_backup,
@@ -1669,6 +1674,8 @@ static void backup_super_roots(struct btrfs_fs_info *info)
 	memcpy(&info->super_copy->super_roots,
 	       &info->super_for_commit->super_roots,
 	       sizeof(*root_backup) * BTRFS_NUM_BACKUP_ROOTS);
+
+	return 0;
 }
 
 /*
@@ -4019,7 +4026,9 @@ int write_all_supers(struct btrfs_trans_handle *trans)
 	} else {
 		/* We are called from transaction commit. */
 		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
-		backup_super_roots(fs_info);
+		ret = backup_super_roots(fs_info);
+		if (ret < 0)
+			return ret;
 	}
 
 	sb = fs_info->super_for_commit;
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 01bbe3cae5c2..331263c206af 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -75,6 +75,12 @@ int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
 	struct btrfs_key key;
 	BTRFS_PATH_AUTO_FREE(path);
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", start);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
@@ -131,6 +137,12 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
 	key.offset = offset;
 
 	extent_root = btrfs_extent_root(fs_info, bytenr);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
 	if (ret < 0)
 		return ret;
@@ -436,6 +448,12 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
 	int recow;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (parent) {
 		key.type = BTRFS_SHARED_DATA_REF_KEY;
@@ -510,6 +528,12 @@ static noinline int insert_extent_data_ref(struct btrfs_trans_handle *trans,
 	u32 num_refs;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (node->parent) {
 		key.type = BTRFS_SHARED_DATA_REF_KEY;
@@ -668,6 +692,12 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (parent) {
 		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
@@ -692,6 +722,12 @@ static noinline int insert_tree_block_ref(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	int ret;
 
+	if (unlikely(!root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	if (node->parent) {
 		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
@@ -782,6 +818,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
 	bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
 	int needed;
 
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_EXTENT_ITEM_KEY;
 	key.offset = num_bytes;
@@ -1680,6 +1722,12 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
 	}
 
 	root = btrfs_extent_root(fs_info, key.objectid);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  key.objectid);
+		return -EUCLEAN;
+	}
 again:
 	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
 	if (ret < 0) {
@@ -2379,6 +2427,12 @@ static noinline int check_committed_ref(struct btrfs_inode *inode,
 	int type;
 	int ret;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
+
 	key.objectid = bytenr;
 	key.type = BTRFS_EXTENT_ITEM_KEY;
 	key.offset = (u64)-1;
@@ -3222,7 +3276,11 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 	u64 delayed_ref_root = href->owning_root;
 
 	extent_root = btrfs_extent_root(info, bytenr);
-	ASSERT(extent_root);
+	if (unlikely(!extent_root)) {
+		btrfs_err(info,
+			  "missing extent root for extent at bytenr %llu", bytenr);
+		return -EUCLEAN;
+	}
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -4935,11 +4993,18 @@ static int alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
 		size += btrfs_extent_inline_ref_size(BTRFS_EXTENT_OWNER_REF_KEY);
 	size += btrfs_extent_inline_ref_size(type);
 
+	extent_root = btrfs_extent_root(fs_info, ins->objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  ins->objectid);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
-	extent_root = btrfs_extent_root(fs_info, ins->objectid);
 	ret = btrfs_insert_empty_item(trans, extent_root, path, ins, size);
 	if (ret) {
 		btrfs_free_path(path);
@@ -5015,11 +5080,18 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
 		size += sizeof(*block_info);
 	}
 
+	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  extent_key.objectid);
+		return -EUCLEAN;
+	}
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
-	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
 	ret = btrfs_insert_empty_item(trans, extent_root, path, &extent_key,
 				      size);
 	if (ret) {
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index ecddfca92b2b..9efd1ec90f03 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1073,6 +1073,14 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
 	if (ret)
 		return ret;
 
+	extent_root = btrfs_extent_root(trans->fs_info, block_group->start);
+	if (unlikely(!extent_root)) {
+		btrfs_err(trans->fs_info,
+			  "missing extent root for block group at offset %llu",
+			  block_group->start);
+		return -EUCLEAN;
+	}
+
 	mutex_lock(&block_group->free_space_lock);
 
 	/*
@@ -1086,7 +1094,6 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
 	key.type = BTRFS_EXTENT_ITEM_KEY;
 	key.offset = 0;
 
-	extent_root = btrfs_extent_root(trans->fs_info, key.objectid);
 	ret = btrfs_search_slot_for_read(extent_root, &key, path, 1, 0);
 	if (ret < 0)
 		goto out_locked;
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9dfd6632c8cc..aaa47b63ffb1 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3740,6 +3740,14 @@ static int qgroup_rescan_leaf(struct btrfs_trans_handle *trans,
 	mutex_lock(&fs_info->qgroup_rescan_lock);
 	extent_root = btrfs_extent_root(fs_info,
 				fs_info->qgroup_rescan_progress.objectid);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  fs_info->qgroup_rescan_progress.objectid);
+		mutex_unlock(&fs_info->qgroup_rescan_lock);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot_for_read(extent_root,
 					 &fs_info->qgroup_rescan_progress,
 					 path, 1, 0);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 95f7c3a20691..3906e457d2ef 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4941,6 +4941,12 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
 	struct btrfs_space_info *sinfo = src_bg->space_info;
 
 	extent_root = btrfs_extent_root(fs_info, src_bg->start);
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  src_bg->start);
+		return -EUCLEAN;
+	}
 
 	trans = btrfs_start_transaction(extent_root, 0);
 	if (IS_ERR(trans))
@@ -5293,6 +5299,13 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start,
 	int ret;
 	bool bg_is_ro = false;
 
+	if (unlikely(!extent_root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for block group at offset %llu",
+			  group_start);
+		return -EUCLEAN;
+	}
+
 	/*
 	 * This only gets set if we had a half-deleted snapshot on mount.  We
 	 * cannot allow relocation to start while we're still trying to clean up
@@ -5523,12 +5536,17 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
 		goto out;
 	}
 
+	rc->extent_root = btrfs_extent_root(fs_info, 0);
+	if (unlikely(!rc->extent_root)) {
+		btrfs_err(fs_info, "missing extent root for extent at bytenr 0");
+		ret = -EUCLEAN;
+		goto out;
+	}
+
 	ret = reloc_chunk_start(fs_info);
 	if (ret < 0)
 		goto out_end;
 
-	rc->extent_root = btrfs_extent_root(fs_info, 0);
-
 	set_reloc_control(rc);
 
 	trans = btrfs_join_transaction(rc->extent_root);
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index ae6f61bcefca..4a1dabeea531 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1259,6 +1259,13 @@ static int calculate_alloc_pointer(struct btrfs_block_group *cache,
 	key.offset = 0;
 
 	root = btrfs_extent_root(fs_info, key.objectid);
+	if (unlikely(!root)) {
+		btrfs_err(fs_info,
+			  "missing extent root for extent at bytenr %llu",
+			  key.objectid);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 	/* We should not find the exact match */
 	if (unlikely(!ret))
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root()
  2026-02-10 11:57   ` [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
@ 2026-02-11 17:34     ` Boris Burkov
  2026-02-11 17:50       ` Filipe Manana
  0 siblings, 1 reply; 14+ messages in thread
From: Boris Burkov @ 2026-02-11 17:34 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Tue, Feb 10, 2026 at 11:57:49AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> btrfs_extent_root() can return a NULL pointer in case the root we are
> looking for is not in the rb tree that tracks roots. So add checks to
> every caller that is missing such check to log a message and return
> an error. The same applies to callers of btrfs_block_group_root(),
> since it calls btrfs_extent_root().
> 

LGTM, thanks.

Would it also make sense to add
Fixes: 29cbcf401793 ("btrfs: stop accessing ->extent_root directly")

Or was it technically possible fs_info->extent_root was NULL before?

> Reported-by: Chris Mason <clm@meta.com>
> Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/backref.c         | 28 ++++++++++++++
>  fs/btrfs/block-group.c     | 36 ++++++++++++++++++
>  fs/btrfs/disk-io.c         | 13 ++++++-
>  fs/btrfs/extent-tree.c     | 78 ++++++++++++++++++++++++++++++++++++--
>  fs/btrfs/free-space-tree.c |  9 ++++-
>  fs/btrfs/qgroup.c          |  8 ++++
>  fs/btrfs/relocation.c      | 22 ++++++++++-
>  fs/btrfs/zoned.c           |  7 ++++
>  8 files changed, 193 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index cf10e10a40fa..8a119c505e32 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1388,6 +1388,13 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
>  		.indirect_missing_keys = PREFTREE_INIT
>  	};
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(ctx->fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  ctx->bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	/* Roots ulist is not needed when using a sharedness check context. */
>  	if (sc)
>  		ASSERT(ctx->roots == NULL);
> @@ -2194,6 +2201,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
>  	struct btrfs_extent_item *ei;
>  	struct btrfs_key key;
>  
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  logical);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = logical;
>  	if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
>  		key.type = BTRFS_METADATA_ITEM_KEY;
> @@ -2841,6 +2855,13 @@ int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
>  	struct btrfs_key key;
>  	int ret;
>  
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	key.type = BTRFS_METADATA_ITEM_KEY;
>  	key.offset = (u64)-1;
> @@ -2977,6 +2998,13 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
>  
>  	/* We're at keyed items, there is no inline item, go to the next one */
>  	extent_root = btrfs_extent_root(iter->fs_info, iter->bytenr);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(iter->fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  iter->bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_next_item(extent_root, iter->path);
>  	if (ret)
>  		return ret;
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 17a18f17538d..36ff6c5a8f51 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -738,6 +738,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
>  		return -ENOMEM;
>  
>  	extent_root = btrfs_extent_root(fs_info, last);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for block group at offset %llu",
> +			  block_group->start);
> +		return -EUCLEAN;
> +	}
>  
>  #ifdef CONFIG_BTRFS_DEBUG
>  	/*
> @@ -1060,6 +1066,11 @@ static int remove_block_group_item(struct btrfs_trans_handle *trans,
>  	int ret;
>  
>  	root = btrfs_block_group_root(fs_info);
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = block_group->start;
>  	key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
>  	key.offset = block_group->length;
> @@ -1348,6 +1359,11 @@ struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
>  	struct btrfs_chunk_map *map;
>  	unsigned int num_items;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return ERR_PTR(-EUCLEAN);
> +	}
> +
>  	map = btrfs_find_chunk_map(fs_info, chunk_offset, 1);
>  	ASSERT(map != NULL);
>  	ASSERT(map->start == chunk_offset);
> @@ -2139,6 +2155,11 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
>  	int ret;
>  	struct btrfs_key found_key;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return -EUCLEAN;
> +	}
> +
>  	btrfs_for_each_slot(root, key, &found_key, path, ret) {
>  		if (found_key.objectid >= key->objectid &&
>  		    found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
> @@ -2713,6 +2734,11 @@ static int insert_block_group_item(struct btrfs_trans_handle *trans,
>  	size_t size;
>  	int ret;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return -EUCLEAN;
> +	}
> +
>  	spin_lock(&block_group->lock);
>  	btrfs_set_stack_block_group_v2_used(&bgi, block_group->used);
>  	btrfs_set_stack_block_group_v2_chunk_objectid(&bgi, block_group->global_root_id);
> @@ -3048,6 +3074,11 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
>  	int ret;
>  	bool dirty_bg_running;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return -EUCLEAN;
> +	}
> +
>  	/*
>  	 * This can only happen when we are doing read-only scrub on read-only
>  	 * mount.
> @@ -3192,6 +3223,11 @@ static int update_block_group_item(struct btrfs_trans_handle *trans,
>  	u64 used, remap_bytes;
>  	u32 identity_remap_count;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info, "missing block group root");
> +		return -EUCLEAN;
> +	}
> +
>  	/*
>  	 * Block group items update can be triggered out of commit transaction
>  	 * critical section, thus we need a consistent view of used bytes.
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index e2a4d4dbf056..4b1e67f176a3 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1590,7 +1590,7 @@ static int find_newest_super_backup(struct btrfs_fs_info *info)
>   * this will bump the backup pointer by one when it is
>   * done
>   */
> -static void backup_super_roots(struct btrfs_fs_info *info)
> +static int backup_super_roots(struct btrfs_fs_info *info)
>  {
>  	const int next_backup = info->backup_root_index;
>  	struct btrfs_root_backup *root_backup;
> @@ -1622,6 +1622,11 @@ static void backup_super_roots(struct btrfs_fs_info *info)
>  		struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
>  		struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
>  
> +		if (unlikely(!extent_root)) {
> +			btrfs_err(info, "missing extent root for extent at bytenr 0");
> +			return -EUCLEAN;
> +		}
> +
>  		btrfs_set_backup_extent_root(root_backup,
>  					     extent_root->node->start);
>  		btrfs_set_backup_extent_root_gen(root_backup,
> @@ -1669,6 +1674,8 @@ static void backup_super_roots(struct btrfs_fs_info *info)
>  	memcpy(&info->super_copy->super_roots,
>  	       &info->super_for_commit->super_roots,
>  	       sizeof(*root_backup) * BTRFS_NUM_BACKUP_ROOTS);
> +
> +	return 0;
>  }
>  
>  /*
> @@ -4019,7 +4026,9 @@ int write_all_supers(struct btrfs_trans_handle *trans)
>  	} else {
>  		/* We are called from transaction commit. */
>  		max_mirrors = BTRFS_SUPER_MIRROR_MAX;
> -		backup_super_roots(fs_info);
> +		ret = backup_super_roots(fs_info);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
>  	sb = fs_info->super_for_commit;
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 01bbe3cae5c2..331263c206af 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -75,6 +75,12 @@ int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
>  	struct btrfs_key key;
>  	BTRFS_PATH_AUTO_FREE(path);
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu", start);
> +		return -EUCLEAN;
> +	}
> +
>  	path = btrfs_alloc_path();
>  	if (!path)
>  		return -ENOMEM;
> @@ -131,6 +137,12 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>  	key.offset = offset;
>  
>  	extent_root = btrfs_extent_root(fs_info, bytenr);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
>  	if (ret < 0)
>  		return ret;
> @@ -436,6 +448,12 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
>  	int recow;
>  	int ret;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	if (parent) {
>  		key.type = BTRFS_SHARED_DATA_REF_KEY;
> @@ -510,6 +528,12 @@ static noinline int insert_extent_data_ref(struct btrfs_trans_handle *trans,
>  	u32 num_refs;
>  	int ret;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	if (node->parent) {
>  		key.type = BTRFS_SHARED_DATA_REF_KEY;
> @@ -668,6 +692,12 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
>  	struct btrfs_key key;
>  	int ret;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	if (parent) {
>  		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> @@ -692,6 +722,12 @@ static noinline int insert_tree_block_ref(struct btrfs_trans_handle *trans,
>  	struct btrfs_key key;
>  	int ret;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	if (node->parent) {
>  		key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> @@ -782,6 +818,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
>  	bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
>  	int needed;
>  
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	key.type = BTRFS_EXTENT_ITEM_KEY;
>  	key.offset = num_bytes;
> @@ -1680,6 +1722,12 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
>  	}
>  
>  	root = btrfs_extent_root(fs_info, key.objectid);
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  key.objectid);
> +		return -EUCLEAN;
> +	}
>  again:
>  	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
>  	if (ret < 0) {
> @@ -2379,6 +2427,12 @@ static noinline int check_committed_ref(struct btrfs_inode *inode,
>  	int type;
>  	int ret;
>  
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	key.objectid = bytenr;
>  	key.type = BTRFS_EXTENT_ITEM_KEY;
>  	key.offset = (u64)-1;
> @@ -3222,7 +3276,11 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>  	u64 delayed_ref_root = href->owning_root;
>  
>  	extent_root = btrfs_extent_root(info, bytenr);
> -	ASSERT(extent_root);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(info,
> +			  "missing extent root for extent at bytenr %llu", bytenr);
> +		return -EUCLEAN;
> +	}
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> @@ -4935,11 +4993,18 @@ static int alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
>  		size += btrfs_extent_inline_ref_size(BTRFS_EXTENT_OWNER_REF_KEY);
>  	size += btrfs_extent_inline_ref_size(type);
>  
> +	extent_root = btrfs_extent_root(fs_info, ins->objectid);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  ins->objectid);
> +		return -EUCLEAN;
> +	}
> +
>  	path = btrfs_alloc_path();
>  	if (!path)
>  		return -ENOMEM;
>  
> -	extent_root = btrfs_extent_root(fs_info, ins->objectid);
>  	ret = btrfs_insert_empty_item(trans, extent_root, path, ins, size);
>  	if (ret) {
>  		btrfs_free_path(path);
> @@ -5015,11 +5080,18 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>  		size += sizeof(*block_info);
>  	}
>  
> +	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  extent_key.objectid);
> +		return -EUCLEAN;
> +	}
> +
>  	path = btrfs_alloc_path();
>  	if (!path)
>  		return -ENOMEM;
>  
> -	extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
>  	ret = btrfs_insert_empty_item(trans, extent_root, path, &extent_key,
>  				      size);
>  	if (ret) {
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index ecddfca92b2b..9efd1ec90f03 100644
> --- a/fs/btrfs/free-space-tree.c
> +++ b/fs/btrfs/free-space-tree.c
> @@ -1073,6 +1073,14 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
>  	if (ret)
>  		return ret;
>  
> +	extent_root = btrfs_extent_root(trans->fs_info, block_group->start);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing extent root for block group at offset %llu",
> +			  block_group->start);
> +		return -EUCLEAN;
> +	}
> +
>  	mutex_lock(&block_group->free_space_lock);
>  
>  	/*
> @@ -1086,7 +1094,6 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
>  	key.type = BTRFS_EXTENT_ITEM_KEY;
>  	key.offset = 0;
>  
> -	extent_root = btrfs_extent_root(trans->fs_info, key.objectid);
>  	ret = btrfs_search_slot_for_read(extent_root, &key, path, 1, 0);
>  	if (ret < 0)
>  		goto out_locked;
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9dfd6632c8cc..aaa47b63ffb1 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -3740,6 +3740,14 @@ static int qgroup_rescan_leaf(struct btrfs_trans_handle *trans,
>  	mutex_lock(&fs_info->qgroup_rescan_lock);
>  	extent_root = btrfs_extent_root(fs_info,
>  				fs_info->qgroup_rescan_progress.objectid);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  fs_info->qgroup_rescan_progress.objectid);
> +		mutex_unlock(&fs_info->qgroup_rescan_lock);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_search_slot_for_read(extent_root,
>  					 &fs_info->qgroup_rescan_progress,
>  					 path, 1, 0);
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 95f7c3a20691..3906e457d2ef 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4941,6 +4941,12 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
>  	struct btrfs_space_info *sinfo = src_bg->space_info;
>  
>  	extent_root = btrfs_extent_root(fs_info, src_bg->start);
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for block group at offset %llu",
> +			  src_bg->start);
> +		return -EUCLEAN;
> +	}
>  
>  	trans = btrfs_start_transaction(extent_root, 0);
>  	if (IS_ERR(trans))
> @@ -5293,6 +5299,13 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start,
>  	int ret;
>  	bool bg_is_ro = false;
>  
> +	if (unlikely(!extent_root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for block group at offset %llu",
> +			  group_start);
> +		return -EUCLEAN;
> +	}
> +
>  	/*
>  	 * This only gets set if we had a half-deleted snapshot on mount.  We
>  	 * cannot allow relocation to start while we're still trying to clean up
> @@ -5523,12 +5536,17 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
>  		goto out;
>  	}
>  
> +	rc->extent_root = btrfs_extent_root(fs_info, 0);
> +	if (unlikely(!rc->extent_root)) {
> +		btrfs_err(fs_info, "missing extent root for extent at bytenr 0");
> +		ret = -EUCLEAN;
> +		goto out;
> +	}
> +
>  	ret = reloc_chunk_start(fs_info);
>  	if (ret < 0)
>  		goto out_end;
>  
> -	rc->extent_root = btrfs_extent_root(fs_info, 0);
> -
>  	set_reloc_control(rc);
>  
>  	trans = btrfs_join_transaction(rc->extent_root);
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index ae6f61bcefca..4a1dabeea531 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1259,6 +1259,13 @@ static int calculate_alloc_pointer(struct btrfs_block_group *cache,
>  	key.offset = 0;
>  
>  	root = btrfs_extent_root(fs_info, key.objectid);
> +	if (unlikely(!root)) {
> +		btrfs_err(fs_info,
> +			  "missing extent root for extent at bytenr %llu",
> +			  key.objectid);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>  	/* We should not find the exact match */
>  	if (unlikely(!ret))
> -- 
> 2.47.2
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root()
  2026-02-11 17:34     ` Boris Burkov
@ 2026-02-11 17:50       ` Filipe Manana
  0 siblings, 0 replies; 14+ messages in thread
From: Filipe Manana @ 2026-02-11 17:50 UTC (permalink / raw)
  To: Boris Burkov; +Cc: linux-btrfs

On Wed, Feb 11, 2026 at 5:35 PM Boris Burkov <boris@bur.io> wrote:
>
> On Tue, Feb 10, 2026 at 11:57:49AM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > btrfs_extent_root() can return a NULL pointer in case the root we are
> > looking for is not in the rb tree that tracks roots. So add checks to
> > every caller that is missing such check to log a message and return
> > an error. The same applies to callers of btrfs_block_group_root(),
> > since it calls btrfs_extent_root().
> >
>
> LGTM, thanks.
>
> Would it also make sense to add
> Fixes: 29cbcf401793 ("btrfs: stop accessing ->extent_root directly")
>
> Or was it technically possible fs_info->extent_root was NULL before?

I'm not sure about adding a Fixes tag here, and even if that would be
the correct commit (it could also be the one that introduces rescue
mount options, see below).

Apart from a bug, the only reason we may not find the extent root is
on a corrupt fs mounted with the rescue options (see for example
6aecd91a5c5b6 ("btrfs: avoid NULL pointer dereference if no valid
extent tree")). The same thing applies to the csum root searches (we
had a few commits that introduced the NULL checks for rescue mounts of
damaged filesystems).

Thanks.

>
> > Reported-by: Chris Mason <clm@meta.com>
> > Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  fs/btrfs/backref.c         | 28 ++++++++++++++
> >  fs/btrfs/block-group.c     | 36 ++++++++++++++++++
> >  fs/btrfs/disk-io.c         | 13 ++++++-
> >  fs/btrfs/extent-tree.c     | 78 ++++++++++++++++++++++++++++++++++++--
> >  fs/btrfs/free-space-tree.c |  9 ++++-
> >  fs/btrfs/qgroup.c          |  8 ++++
> >  fs/btrfs/relocation.c      | 22 ++++++++++-
> >  fs/btrfs/zoned.c           |  7 ++++
> >  8 files changed, 193 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index cf10e10a40fa..8a119c505e32 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -1388,6 +1388,13 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
> >               .indirect_missing_keys = PREFTREE_INIT
> >       };
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(ctx->fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       ctx->bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       /* Roots ulist is not needed when using a sharedness check context. */
> >       if (sc)
> >               ASSERT(ctx->roots == NULL);
> > @@ -2194,6 +2201,13 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
> >       struct btrfs_extent_item *ei;
> >       struct btrfs_key key;
> >
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       logical);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = logical;
> >       if (btrfs_fs_incompat(fs_info, SKINNY_METADATA))
> >               key.type = BTRFS_METADATA_ITEM_KEY;
> > @@ -2841,6 +2855,13 @@ int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
> >       struct btrfs_key key;
> >       int ret;
> >
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       key.type = BTRFS_METADATA_ITEM_KEY;
> >       key.offset = (u64)-1;
> > @@ -2977,6 +2998,13 @@ int btrfs_backref_iter_next(struct btrfs_backref_iter *iter)
> >
> >       /* We're at keyed items, there is no inline item, go to the next one */
> >       extent_root = btrfs_extent_root(iter->fs_info, iter->bytenr);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(iter->fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       iter->bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_next_item(extent_root, iter->path);
> >       if (ret)
> >               return ret;
> > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> > index 17a18f17538d..36ff6c5a8f51 100644
> > --- a/fs/btrfs/block-group.c
> > +++ b/fs/btrfs/block-group.c
> > @@ -738,6 +738,12 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
> >               return -ENOMEM;
> >
> >       extent_root = btrfs_extent_root(fs_info, last);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for block group at offset %llu",
> > +                       block_group->start);
> > +             return -EUCLEAN;
> > +     }
> >
> >  #ifdef CONFIG_BTRFS_DEBUG
> >       /*
> > @@ -1060,6 +1066,11 @@ static int remove_block_group_item(struct btrfs_trans_handle *trans,
> >       int ret;
> >
> >       root = btrfs_block_group_root(fs_info);
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = block_group->start;
> >       key.type = BTRFS_BLOCK_GROUP_ITEM_KEY;
> >       key.offset = block_group->length;
> > @@ -1348,6 +1359,11 @@ struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
> >       struct btrfs_chunk_map *map;
> >       unsigned int num_items;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return ERR_PTR(-EUCLEAN);
> > +     }
> > +
> >       map = btrfs_find_chunk_map(fs_info, chunk_offset, 1);
> >       ASSERT(map != NULL);
> >       ASSERT(map->start == chunk_offset);
> > @@ -2139,6 +2155,11 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
> >       int ret;
> >       struct btrfs_key found_key;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return -EUCLEAN;
> > +     }
> > +
> >       btrfs_for_each_slot(root, key, &found_key, path, ret) {
> >               if (found_key.objectid >= key->objectid &&
> >                   found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
> > @@ -2713,6 +2734,11 @@ static int insert_block_group_item(struct btrfs_trans_handle *trans,
> >       size_t size;
> >       int ret;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return -EUCLEAN;
> > +     }
> > +
> >       spin_lock(&block_group->lock);
> >       btrfs_set_stack_block_group_v2_used(&bgi, block_group->used);
> >       btrfs_set_stack_block_group_v2_chunk_objectid(&bgi, block_group->global_root_id);
> > @@ -3048,6 +3074,11 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
> >       int ret;
> >       bool dirty_bg_running;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return -EUCLEAN;
> > +     }
> > +
> >       /*
> >        * This can only happen when we are doing read-only scrub on read-only
> >        * mount.
> > @@ -3192,6 +3223,11 @@ static int update_block_group_item(struct btrfs_trans_handle *trans,
> >       u64 used, remap_bytes;
> >       u32 identity_remap_count;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info, "missing block group root");
> > +             return -EUCLEAN;
> > +     }
> > +
> >       /*
> >        * Block group items update can be triggered out of commit transaction
> >        * critical section, thus we need a consistent view of used bytes.
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index e2a4d4dbf056..4b1e67f176a3 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -1590,7 +1590,7 @@ static int find_newest_super_backup(struct btrfs_fs_info *info)
> >   * this will bump the backup pointer by one when it is
> >   * done
> >   */
> > -static void backup_super_roots(struct btrfs_fs_info *info)
> > +static int backup_super_roots(struct btrfs_fs_info *info)
> >  {
> >       const int next_backup = info->backup_root_index;
> >       struct btrfs_root_backup *root_backup;
> > @@ -1622,6 +1622,11 @@ static void backup_super_roots(struct btrfs_fs_info *info)
> >               struct btrfs_root *extent_root = btrfs_extent_root(info, 0);
> >               struct btrfs_root *csum_root = btrfs_csum_root(info, 0);
> >
> > +             if (unlikely(!extent_root)) {
> > +                     btrfs_err(info, "missing extent root for extent at bytenr 0");
> > +                     return -EUCLEAN;
> > +             }
> > +
> >               btrfs_set_backup_extent_root(root_backup,
> >                                            extent_root->node->start);
> >               btrfs_set_backup_extent_root_gen(root_backup,
> > @@ -1669,6 +1674,8 @@ static void backup_super_roots(struct btrfs_fs_info *info)
> >       memcpy(&info->super_copy->super_roots,
> >              &info->super_for_commit->super_roots,
> >              sizeof(*root_backup) * BTRFS_NUM_BACKUP_ROOTS);
> > +
> > +     return 0;
> >  }
> >
> >  /*
> > @@ -4019,7 +4026,9 @@ int write_all_supers(struct btrfs_trans_handle *trans)
> >       } else {
> >               /* We are called from transaction commit. */
> >               max_mirrors = BTRFS_SUPER_MIRROR_MAX;
> > -             backup_super_roots(fs_info);
> > +             ret = backup_super_roots(fs_info);
> > +             if (ret < 0)
> > +                     return ret;
> >       }
> >
> >       sb = fs_info->super_for_commit;
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 01bbe3cae5c2..331263c206af 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -75,6 +75,12 @@ int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64 len)
> >       struct btrfs_key key;
> >       BTRFS_PATH_AUTO_FREE(path);
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu", start);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       path = btrfs_alloc_path();
> >       if (!path)
> >               return -ENOMEM;
> > @@ -131,6 +137,12 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
> >       key.offset = offset;
> >
> >       extent_root = btrfs_extent_root(fs_info, bytenr);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
> >       if (ret < 0)
> >               return ret;
> > @@ -436,6 +448,12 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
> >       int recow;
> >       int ret;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       if (parent) {
> >               key.type = BTRFS_SHARED_DATA_REF_KEY;
> > @@ -510,6 +528,12 @@ static noinline int insert_extent_data_ref(struct btrfs_trans_handle *trans,
> >       u32 num_refs;
> >       int ret;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       if (node->parent) {
> >               key.type = BTRFS_SHARED_DATA_REF_KEY;
> > @@ -668,6 +692,12 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
> >       struct btrfs_key key;
> >       int ret;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       if (parent) {
> >               key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> > @@ -692,6 +722,12 @@ static noinline int insert_tree_block_ref(struct btrfs_trans_handle *trans,
> >       struct btrfs_key key;
> >       int ret;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       if (node->parent) {
> >               key.type = BTRFS_SHARED_BLOCK_REF_KEY;
> > @@ -782,6 +818,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
> >       bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
> >       int needed;
> >
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       key.type = BTRFS_EXTENT_ITEM_KEY;
> >       key.offset = num_bytes;
> > @@ -1680,6 +1722,12 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
> >       }
> >
> >       root = btrfs_extent_root(fs_info, key.objectid);
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       key.objectid);
> > +             return -EUCLEAN;
> > +     }
> >  again:
> >       ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> >       if (ret < 0) {
> > @@ -2379,6 +2427,12 @@ static noinline int check_committed_ref(struct btrfs_inode *inode,
> >       int type;
> >       int ret;
> >
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       key.objectid = bytenr;
> >       key.type = BTRFS_EXTENT_ITEM_KEY;
> >       key.offset = (u64)-1;
> > @@ -3222,7 +3276,11 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
> >       u64 delayed_ref_root = href->owning_root;
> >
> >       extent_root = btrfs_extent_root(info, bytenr);
> > -     ASSERT(extent_root);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(info,
> > +                       "missing extent root for extent at bytenr %llu", bytenr);
> > +             return -EUCLEAN;
> > +     }
> >
> >       path = btrfs_alloc_path();
> >       if (!path)
> > @@ -4935,11 +4993,18 @@ static int alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
> >               size += btrfs_extent_inline_ref_size(BTRFS_EXTENT_OWNER_REF_KEY);
> >       size += btrfs_extent_inline_ref_size(type);
> >
> > +     extent_root = btrfs_extent_root(fs_info, ins->objectid);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       ins->objectid);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       path = btrfs_alloc_path();
> >       if (!path)
> >               return -ENOMEM;
> >
> > -     extent_root = btrfs_extent_root(fs_info, ins->objectid);
> >       ret = btrfs_insert_empty_item(trans, extent_root, path, ins, size);
> >       if (ret) {
> >               btrfs_free_path(path);
> > @@ -5015,11 +5080,18 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
> >               size += sizeof(*block_info);
> >       }
> >
> > +     extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       extent_key.objectid);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       path = btrfs_alloc_path();
> >       if (!path)
> >               return -ENOMEM;
> >
> > -     extent_root = btrfs_extent_root(fs_info, extent_key.objectid);
> >       ret = btrfs_insert_empty_item(trans, extent_root, path, &extent_key,
> >                                     size);
> >       if (ret) {
> > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> > index ecddfca92b2b..9efd1ec90f03 100644
> > --- a/fs/btrfs/free-space-tree.c
> > +++ b/fs/btrfs/free-space-tree.c
> > @@ -1073,6 +1073,14 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
> >       if (ret)
> >               return ret;
> >
> > +     extent_root = btrfs_extent_root(trans->fs_info, block_group->start);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing extent root for block group at offset %llu",
> > +                       block_group->start);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       mutex_lock(&block_group->free_space_lock);
> >
> >       /*
> > @@ -1086,7 +1094,6 @@ static int populate_free_space_tree(struct btrfs_trans_handle *trans,
> >       key.type = BTRFS_EXTENT_ITEM_KEY;
> >       key.offset = 0;
> >
> > -     extent_root = btrfs_extent_root(trans->fs_info, key.objectid);
> >       ret = btrfs_search_slot_for_read(extent_root, &key, path, 1, 0);
> >       if (ret < 0)
> >               goto out_locked;
> > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > index 9dfd6632c8cc..aaa47b63ffb1 100644
> > --- a/fs/btrfs/qgroup.c
> > +++ b/fs/btrfs/qgroup.c
> > @@ -3740,6 +3740,14 @@ static int qgroup_rescan_leaf(struct btrfs_trans_handle *trans,
> >       mutex_lock(&fs_info->qgroup_rescan_lock);
> >       extent_root = btrfs_extent_root(fs_info,
> >                               fs_info->qgroup_rescan_progress.objectid);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       fs_info->qgroup_rescan_progress.objectid);
> > +             mutex_unlock(&fs_info->qgroup_rescan_lock);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_search_slot_for_read(extent_root,
> >                                        &fs_info->qgroup_rescan_progress,
> >                                        path, 1, 0);
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 95f7c3a20691..3906e457d2ef 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -4941,6 +4941,12 @@ static int do_remap_reloc_trans(struct btrfs_fs_info *fs_info,
> >       struct btrfs_space_info *sinfo = src_bg->space_info;
> >
> >       extent_root = btrfs_extent_root(fs_info, src_bg->start);
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for block group at offset %llu",
> > +                       src_bg->start);
> > +             return -EUCLEAN;
> > +     }
> >
> >       trans = btrfs_start_transaction(extent_root, 0);
> >       if (IS_ERR(trans))
> > @@ -5293,6 +5299,13 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start,
> >       int ret;
> >       bool bg_is_ro = false;
> >
> > +     if (unlikely(!extent_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for block group at offset %llu",
> > +                       group_start);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       /*
> >        * This only gets set if we had a half-deleted snapshot on mount.  We
> >        * cannot allow relocation to start while we're still trying to clean up
> > @@ -5523,12 +5536,17 @@ int btrfs_recover_relocation(struct btrfs_fs_info *fs_info)
> >               goto out;
> >       }
> >
> > +     rc->extent_root = btrfs_extent_root(fs_info, 0);
> > +     if (unlikely(!rc->extent_root)) {
> > +             btrfs_err(fs_info, "missing extent root for extent at bytenr 0");
> > +             ret = -EUCLEAN;
> > +             goto out;
> > +     }
> > +
> >       ret = reloc_chunk_start(fs_info);
> >       if (ret < 0)
> >               goto out_end;
> >
> > -     rc->extent_root = btrfs_extent_root(fs_info, 0);
> > -
> >       set_reloc_control(rc);
> >
> >       trans = btrfs_join_transaction(rc->extent_root);
> > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> > index ae6f61bcefca..4a1dabeea531 100644
> > --- a/fs/btrfs/zoned.c
> > +++ b/fs/btrfs/zoned.c
> > @@ -1259,6 +1259,13 @@ static int calculate_alloc_pointer(struct btrfs_block_group *cache,
> >       key.offset = 0;
> >
> >       root = btrfs_extent_root(fs_info, key.objectid);
> > +     if (unlikely(!root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing extent root for extent at bytenr %llu",
> > +                       key.objectid);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> >       /* We should not find the exact match */
> >       if (unlikely(!ret))
> > --
> > 2.47.2
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root()
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
  2026-02-10 11:57   ` [PATCH v2 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
  2026-02-10 11:57   ` [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
@ 2026-02-10 11:57   ` fdmanana
  2026-02-11 17:39     ` Boris Burkov
  2026-02-11 17:40   ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots Boris Burkov
  3 siblings, 1 reply; 14+ messages in thread
From: fdmanana @ 2026-02-10 11:57 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

btrfs_csum_root() can return a NULL pointer in case the root we are
looking for is not in the rb tree that tracks roots. So add checks to
every caller that is missing such check to log a message and return
an error.

Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/disk-io.c     |  4 ++++
 fs/btrfs/extent-tree.c | 20 ++++++++++++++++++--
 fs/btrfs/file-item.c   |  7 +++++++
 fs/btrfs/inode.c       | 18 ++++++++++++++++--
 fs/btrfs/raid56.c      | 12 ++++++++++--
 fs/btrfs/relocation.c  |  8 ++++++++
 fs/btrfs/tree-log.c    | 21 +++++++++++++++++++++
 7 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4b1e67f176a3..99ce7c1ca53a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1626,6 +1626,10 @@ static int backup_super_roots(struct btrfs_fs_info *info)
 			btrfs_err(info, "missing extent root for extent at bytenr 0");
 			return -EUCLEAN;
 		}
+		if (unlikely(!csum_root)) {
+			btrfs_err(info, "missing csum root for extent at bytenr 0");
+			return -EUCLEAN;
+		}
 
 		btrfs_set_backup_extent_root(root_backup,
 					     extent_root->node->start);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 331263c206af..d1bfbad5adc6 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1974,8 +1974,15 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans,
 			struct btrfs_root *csum_root;
 
 			csum_root = btrfs_csum_root(fs_info, head->bytenr);
-			ret = btrfs_del_csums(trans, csum_root, head->bytenr,
-					      head->num_bytes);
+			if (unlikely(!csum_root)) {
+				btrfs_err(fs_info,
+					  "missing csum root for extent at bytenr %llu",
+					  head->bytenr);
+				ret = -EUCLEAN;
+			} else {
+				ret = btrfs_del_csums(trans, csum_root, head->bytenr,
+						      head->num_bytes);
+			}
 		}
 	}
 
@@ -3147,6 +3154,15 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
 		struct btrfs_root *csum_root;
 
 		csum_root = btrfs_csum_root(trans->fs_info, bytenr);
+		if (unlikely(!csum_root)) {
+			ret = -EUCLEAN;
+			btrfs_abort_transaction(trans, ret);
+			btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  bytenr);
+			return ret;
+		}
+
 		ret = btrfs_del_csums(trans, csum_root, bytenr, num_bytes);
 		if (unlikely(ret)) {
 			btrfs_abort_transaction(trans, ret);
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 7bd715442f3e..f585ddfa8440 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -308,6 +308,13 @@ static int search_csum_tree(struct btrfs_fs_info *fs_info,
 	/* Current item doesn't contain the desired range, search again */
 	btrfs_release_path(path);
 	csum_root = btrfs_csum_root(fs_info, disk_bytenr);
+	if (unlikely(!csum_root)) {
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  disk_bytenr);
+		return -EUCLEAN;
+	}
+
 	item = btrfs_lookup_csum(NULL, csum_root, path, disk_bytenr, 0);
 	if (IS_ERR(item)) {
 		ret = PTR_ERR(item);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 826809977df5..1cbaaf7a7230 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2013,6 +2013,13 @@ static int can_nocow_file_extent(struct btrfs_path *path,
 	 */
 
 	csum_root = btrfs_csum_root(root->fs_info, io_start);
+	if (unlikely(!csum_root)) {
+		btrfs_err(root->fs_info,
+			  "missing csum root for extent at bytenr %llu", io_start);
+		ret = -EUCLEAN;
+		goto out;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, io_start,
 				      io_start + args->file_extent.num_bytes - 1,
 				      NULL, nowait);
@@ -2750,10 +2757,17 @@ static int add_pending_csums(struct btrfs_trans_handle *trans,
 	int ret;
 
 	list_for_each_entry(sum, list, list) {
-		trans->adding_csums = true;
-		if (!csum_root)
+		if (!csum_root) {
 			csum_root = btrfs_csum_root(trans->fs_info,
 						    sum->logical);
+			if (unlikely(!csum_root)) {
+				btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+					  sum->logical);
+				return -EUCLEAN;
+			}
+		}
+		trans->adding_csums = true;
 		ret = btrfs_csum_file_blocks(trans, csum_root, sum);
 		trans->adding_csums = false;
 		if (ret)
diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index baadaaa189c0..230dd93dad6e 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2295,8 +2295,7 @@ void raid56_parity_recover(struct bio *bio, struct btrfs_io_context *bioc,
 static void fill_data_csums(struct btrfs_raid_bio *rbio)
 {
 	struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
-	struct btrfs_root *csum_root = btrfs_csum_root(fs_info,
-						       rbio->bioc->full_stripe_logical);
+	struct btrfs_root *csum_root;
 	const u64 start = rbio->bioc->full_stripe_logical;
 	const u32 len = (rbio->nr_data * rbio->stripe_nsectors) <<
 			fs_info->sectorsize_bits;
@@ -2329,6 +2328,15 @@ static void fill_data_csums(struct btrfs_raid_bio *rbio)
 		goto error;
 	}
 
+	csum_root = btrfs_csum_root(fs_info, rbio->bioc->full_stripe_logical);
+	if (unlikely(!csum_root)) {
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  rbio->bioc->full_stripe_logical);
+		ret = -EUCLEAN;
+		goto error;
+	}
+
 	ret = btrfs_lookup_csums_bitmap(csum_root, NULL, start, start + len - 1,
 					rbio->csum_buf, rbio->csum_bitmap);
 	if (ret < 0)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 3906e457d2ef..8a8a66112d42 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -5641,6 +5641,14 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
 	LIST_HEAD(list);
 	int ret;
 
+	if (unlikely(!csum_root)) {
+		btrfs_mark_ordered_extent_error(ordered);
+		btrfs_err(fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  disk_bytenr);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 				      disk_bytenr + ordered->num_bytes - 1,
 				      &list, false);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index e2806ca410f6..e9655095ba4c 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -984,6 +984,13 @@ static noinline int replay_one_extent(struct walk_control *wc)
 
 		sums = list_first_entry(&ordered_sums, struct btrfs_ordered_sum, list);
 		csum_root = btrfs_csum_root(fs_info, sums->logical);
+		if (unlikely(!csum_root)) {
+			btrfs_err(fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  sums->logical);
+			ret = -EUCLEAN;
+		}
+
 		if (!ret) {
 			ret = btrfs_del_csums(trans, csum_root, sums->logical,
 					      sums->len);
@@ -4890,6 +4897,13 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
 		}
 
 		csum_root = btrfs_csum_root(trans->fs_info, disk_bytenr);
+		if (unlikely(!csum_root)) {
+			btrfs_err(trans->fs_info,
+				  "missing csum root for extent at bytenr %llu",
+				  disk_bytenr);
+			return -EUCLEAN;
+		}
+
 		disk_bytenr += extent_offset;
 		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
 					      disk_bytenr + extent_num_bytes - 1,
@@ -5086,6 +5100,13 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
 	/* block start is already adjusted for the file extent offset. */
 	block_start = btrfs_extent_map_block_start(em);
 	csum_root = btrfs_csum_root(trans->fs_info, block_start);
+	if (unlikely(!csum_root)) {
+		btrfs_err(trans->fs_info,
+			  "missing csum root for extent at bytenr %llu",
+			  block_start);
+		return -EUCLEAN;
+	}
+
 	ret = btrfs_lookup_csums_list(csum_root, block_start + csum_offset,
 				      block_start + csum_offset + csum_len - 1,
 				      &ordered_sums, false);
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root()
  2026-02-10 11:57   ` [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
@ 2026-02-11 17:39     ` Boris Burkov
  2026-02-11 17:53       ` Filipe Manana
  0 siblings, 1 reply; 14+ messages in thread
From: Boris Burkov @ 2026-02-11 17:39 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Tue, Feb 10, 2026 at 11:57:50AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> btrfs_csum_root() can return a NULL pointer in case the root we are
> looking for is not in the rb tree that tracks roots. So add checks to
> every caller that is missing such check to log a message and return
> an error.

Similar question on if you think this one needs a Fixes.

> 
> Reported-by: Chris Mason <clm@meta.com>
> Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/disk-io.c     |  4 ++++
>  fs/btrfs/extent-tree.c | 20 ++++++++++++++++++--
>  fs/btrfs/file-item.c   |  7 +++++++
>  fs/btrfs/inode.c       | 18 ++++++++++++++++--
>  fs/btrfs/raid56.c      | 12 ++++++++++--
>  fs/btrfs/relocation.c  |  8 ++++++++
>  fs/btrfs/tree-log.c    | 21 +++++++++++++++++++++
>  7 files changed, 84 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 4b1e67f176a3..99ce7c1ca53a 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1626,6 +1626,10 @@ static int backup_super_roots(struct btrfs_fs_info *info)
>  			btrfs_err(info, "missing extent root for extent at bytenr 0");
>  			return -EUCLEAN;
>  		}
> +		if (unlikely(!csum_root)) {
> +			btrfs_err(info, "missing csum root for extent at bytenr 0");
> +			return -EUCLEAN;
> +		}
>  
>  		btrfs_set_backup_extent_root(root_backup,
>  					     extent_root->node->start);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 331263c206af..d1bfbad5adc6 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -1974,8 +1974,15 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans,
>  			struct btrfs_root *csum_root;
>  
>  			csum_root = btrfs_csum_root(fs_info, head->bytenr);
> -			ret = btrfs_del_csums(trans, csum_root, head->bytenr,
> -					      head->num_bytes);
> +			if (unlikely(!csum_root)) {
> +				btrfs_err(fs_info,
> +					  "missing csum root for extent at bytenr %llu",
> +					  head->bytenr);
> +				ret = -EUCLEAN;
> +			} else {
> +				ret = btrfs_del_csums(trans, csum_root, head->bytenr,
> +						      head->num_bytes);
> +			}

nit: I sort of prefer early-exit/goto-cleanup as a general pattern rather
than nesting the work behind conditions, but I assume you did it on purpose
since it's the only early exit that cleans up?

I think it's nice that it makes all the code look predictable and the
same when checking the root.

>  		}
>  	}
>  
> @@ -3147,6 +3154,15 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
>  		struct btrfs_root *csum_root;
>  
>  		csum_root = btrfs_csum_root(trans->fs_info, bytenr);
> +		if (unlikely(!csum_root)) {
> +			ret = -EUCLEAN;
> +			btrfs_abort_transaction(trans, ret);
> +			btrfs_err(trans->fs_info,
> +				  "missing csum root for extent at bytenr %llu",
> +				  bytenr);
> +			return ret;
> +		}
> +
>  		ret = btrfs_del_csums(trans, csum_root, bytenr, num_bytes);
>  		if (unlikely(ret)) {
>  			btrfs_abort_transaction(trans, ret);
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 7bd715442f3e..f585ddfa8440 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -308,6 +308,13 @@ static int search_csum_tree(struct btrfs_fs_info *fs_info,
>  	/* Current item doesn't contain the desired range, search again */
>  	btrfs_release_path(path);
>  	csum_root = btrfs_csum_root(fs_info, disk_bytenr);
> +	if (unlikely(!csum_root)) {
> +		btrfs_err(fs_info,
> +			  "missing csum root for extent at bytenr %llu",
> +			  disk_bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	item = btrfs_lookup_csum(NULL, csum_root, path, disk_bytenr, 0);
>  	if (IS_ERR(item)) {
>  		ret = PTR_ERR(item);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 826809977df5..1cbaaf7a7230 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2013,6 +2013,13 @@ static int can_nocow_file_extent(struct btrfs_path *path,
>  	 */
>  
>  	csum_root = btrfs_csum_root(root->fs_info, io_start);
> +	if (unlikely(!csum_root)) {
> +		btrfs_err(root->fs_info,
> +			  "missing csum root for extent at bytenr %llu", io_start);
> +		ret = -EUCLEAN;
> +		goto out;
> +	}
> +
>  	ret = btrfs_lookup_csums_list(csum_root, io_start,
>  				      io_start + args->file_extent.num_bytes - 1,
>  				      NULL, nowait);
> @@ -2750,10 +2757,17 @@ static int add_pending_csums(struct btrfs_trans_handle *trans,
>  	int ret;
>  
>  	list_for_each_entry(sum, list, list) {
> -		trans->adding_csums = true;
> -		if (!csum_root)
> +		if (!csum_root) {
>  			csum_root = btrfs_csum_root(trans->fs_info,
>  						    sum->logical);
> +			if (unlikely(!csum_root)) {
> +				btrfs_err(trans->fs_info,
> +				  "missing csum root for extent at bytenr %llu",
> +					  sum->logical);
> +				return -EUCLEAN;
> +			}
> +		}
> +		trans->adding_csums = true;
>  		ret = btrfs_csum_file_blocks(trans, csum_root, sum);
>  		trans->adding_csums = false;
>  		if (ret)
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index baadaaa189c0..230dd93dad6e 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -2295,8 +2295,7 @@ void raid56_parity_recover(struct bio *bio, struct btrfs_io_context *bioc,
>  static void fill_data_csums(struct btrfs_raid_bio *rbio)
>  {
>  	struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
> -	struct btrfs_root *csum_root = btrfs_csum_root(fs_info,
> -						       rbio->bioc->full_stripe_logical);
> +	struct btrfs_root *csum_root;
>  	const u64 start = rbio->bioc->full_stripe_logical;
>  	const u32 len = (rbio->nr_data * rbio->stripe_nsectors) <<
>  			fs_info->sectorsize_bits;
> @@ -2329,6 +2328,15 @@ static void fill_data_csums(struct btrfs_raid_bio *rbio)
>  		goto error;
>  	}
>  
> +	csum_root = btrfs_csum_root(fs_info, rbio->bioc->full_stripe_logical);
> +	if (unlikely(!csum_root)) {
> +		btrfs_err(fs_info,
> +			  "missing csum root for extent at bytenr %llu",
> +			  rbio->bioc->full_stripe_logical);
> +		ret = -EUCLEAN;
> +		goto error;
> +	}
> +
>  	ret = btrfs_lookup_csums_bitmap(csum_root, NULL, start, start + len - 1,
>  					rbio->csum_buf, rbio->csum_bitmap);
>  	if (ret < 0)
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 3906e457d2ef..8a8a66112d42 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -5641,6 +5641,14 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
>  	LIST_HEAD(list);
>  	int ret;
>  
> +	if (unlikely(!csum_root)) {
> +		btrfs_mark_ordered_extent_error(ordered);
> +		btrfs_err(fs_info,
> +			  "missing csum root for extent at bytenr %llu",
> +			  disk_bytenr);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>  				      disk_bytenr + ordered->num_bytes - 1,
>  				      &list, false);
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index e2806ca410f6..e9655095ba4c 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -984,6 +984,13 @@ static noinline int replay_one_extent(struct walk_control *wc)
>  
>  		sums = list_first_entry(&ordered_sums, struct btrfs_ordered_sum, list);
>  		csum_root = btrfs_csum_root(fs_info, sums->logical);
> +		if (unlikely(!csum_root)) {
> +			btrfs_err(fs_info,
> +				  "missing csum root for extent at bytenr %llu",
> +				  sums->logical);
> +			ret = -EUCLEAN;
> +		}
> +
>  		if (!ret) {
>  			ret = btrfs_del_csums(trans, csum_root, sums->logical,
>  					      sums->len);
> @@ -4890,6 +4897,13 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
>  		}
>  
>  		csum_root = btrfs_csum_root(trans->fs_info, disk_bytenr);
> +		if (unlikely(!csum_root)) {
> +			btrfs_err(trans->fs_info,
> +				  "missing csum root for extent at bytenr %llu",
> +				  disk_bytenr);
> +			return -EUCLEAN;
> +		}
> +
>  		disk_bytenr += extent_offset;
>  		ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
>  					      disk_bytenr + extent_num_bytes - 1,
> @@ -5086,6 +5100,13 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
>  	/* block start is already adjusted for the file extent offset. */
>  	block_start = btrfs_extent_map_block_start(em);
>  	csum_root = btrfs_csum_root(trans->fs_info, block_start);
> +	if (unlikely(!csum_root)) {
> +		btrfs_err(trans->fs_info,
> +			  "missing csum root for extent at bytenr %llu",
> +			  block_start);
> +		return -EUCLEAN;
> +	}
> +
>  	ret = btrfs_lookup_csums_list(csum_root, block_start + csum_offset,
>  				      block_start + csum_offset + csum_len - 1,
>  				      &ordered_sums, false);
> -- 
> 2.47.2
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root()
  2026-02-11 17:39     ` Boris Burkov
@ 2026-02-11 17:53       ` Filipe Manana
  0 siblings, 0 replies; 14+ messages in thread
From: Filipe Manana @ 2026-02-11 17:53 UTC (permalink / raw)
  To: Boris Burkov; +Cc: linux-btrfs

On Wed, Feb 11, 2026 at 5:40 PM Boris Burkov <boris@bur.io> wrote:
>
> On Tue, Feb 10, 2026 at 11:57:50AM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > btrfs_csum_root() can return a NULL pointer in case the root we are
> > looking for is not in the rb tree that tracks roots. So add checks to
> > every caller that is missing such check to log a message and return
> > an error.
>
> Similar question on if you think this one needs a Fixes.
>
> >
> > Reported-by: Chris Mason <clm@meta.com>
> > Link: https://lore.kernel.org/linux-btrfs/20260208161657.3972997-1-clm@meta.com/
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  fs/btrfs/disk-io.c     |  4 ++++
> >  fs/btrfs/extent-tree.c | 20 ++++++++++++++++++--
> >  fs/btrfs/file-item.c   |  7 +++++++
> >  fs/btrfs/inode.c       | 18 ++++++++++++++++--
> >  fs/btrfs/raid56.c      | 12 ++++++++++--
> >  fs/btrfs/relocation.c  |  8 ++++++++
> >  fs/btrfs/tree-log.c    | 21 +++++++++++++++++++++
> >  7 files changed, 84 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 4b1e67f176a3..99ce7c1ca53a 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -1626,6 +1626,10 @@ static int backup_super_roots(struct btrfs_fs_info *info)
> >                       btrfs_err(info, "missing extent root for extent at bytenr 0");
> >                       return -EUCLEAN;
> >               }
> > +             if (unlikely(!csum_root)) {
> > +                     btrfs_err(info, "missing csum root for extent at bytenr 0");
> > +                     return -EUCLEAN;
> > +             }
> >
> >               btrfs_set_backup_extent_root(root_backup,
> >                                            extent_root->node->start);
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 331263c206af..d1bfbad5adc6 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -1974,8 +1974,15 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans,
> >                       struct btrfs_root *csum_root;
> >
> >                       csum_root = btrfs_csum_root(fs_info, head->bytenr);
> > -                     ret = btrfs_del_csums(trans, csum_root, head->bytenr,
> > -                                           head->num_bytes);
> > +                     if (unlikely(!csum_root)) {
> > +                             btrfs_err(fs_info,
> > +                                       "missing csum root for extent at bytenr %llu",
> > +                                       head->bytenr);
> > +                             ret = -EUCLEAN;
> > +                     } else {
> > +                             ret = btrfs_del_csums(trans, csum_root, head->bytenr,
> > +                                                   head->num_bytes);
> > +                     }
>
> nit: I sort of prefer early-exit/goto-cleanup as a general pattern rather
> than nesting the work behind conditions, but I assume you did it on purpose
> since it's the only early exit that cleans up?

Yes, it was on purpose, because pinning the extent must come before
the failure and everything after should be executed on error too (just
like when btrfs_del_csums() fails).

Thanks.

>
> I think it's nice that it makes all the code look predictable and the
> same when checking the root.
>
> >               }
> >       }
> >
> > @@ -3147,6 +3154,15 @@ static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
> >               struct btrfs_root *csum_root;
> >
> >               csum_root = btrfs_csum_root(trans->fs_info, bytenr);
> > +             if (unlikely(!csum_root)) {
> > +                     ret = -EUCLEAN;
> > +                     btrfs_abort_transaction(trans, ret);
> > +                     btrfs_err(trans->fs_info,
> > +                               "missing csum root for extent at bytenr %llu",
> > +                               bytenr);
> > +                     return ret;
> > +             }
> > +
> >               ret = btrfs_del_csums(trans, csum_root, bytenr, num_bytes);
> >               if (unlikely(ret)) {
> >                       btrfs_abort_transaction(trans, ret);
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 7bd715442f3e..f585ddfa8440 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -308,6 +308,13 @@ static int search_csum_tree(struct btrfs_fs_info *fs_info,
> >       /* Current item doesn't contain the desired range, search again */
> >       btrfs_release_path(path);
> >       csum_root = btrfs_csum_root(fs_info, disk_bytenr);
> > +     if (unlikely(!csum_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing csum root for extent at bytenr %llu",
> > +                       disk_bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       item = btrfs_lookup_csum(NULL, csum_root, path, disk_bytenr, 0);
> >       if (IS_ERR(item)) {
> >               ret = PTR_ERR(item);
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 826809977df5..1cbaaf7a7230 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -2013,6 +2013,13 @@ static int can_nocow_file_extent(struct btrfs_path *path,
> >        */
> >
> >       csum_root = btrfs_csum_root(root->fs_info, io_start);
> > +     if (unlikely(!csum_root)) {
> > +             btrfs_err(root->fs_info,
> > +                       "missing csum root for extent at bytenr %llu", io_start);
> > +             ret = -EUCLEAN;
> > +             goto out;
> > +     }
> > +
> >       ret = btrfs_lookup_csums_list(csum_root, io_start,
> >                                     io_start + args->file_extent.num_bytes - 1,
> >                                     NULL, nowait);
> > @@ -2750,10 +2757,17 @@ static int add_pending_csums(struct btrfs_trans_handle *trans,
> >       int ret;
> >
> >       list_for_each_entry(sum, list, list) {
> > -             trans->adding_csums = true;
> > -             if (!csum_root)
> > +             if (!csum_root) {
> >                       csum_root = btrfs_csum_root(trans->fs_info,
> >                                                   sum->logical);
> > +                     if (unlikely(!csum_root)) {
> > +                             btrfs_err(trans->fs_info,
> > +                               "missing csum root for extent at bytenr %llu",
> > +                                       sum->logical);
> > +                             return -EUCLEAN;
> > +                     }
> > +             }
> > +             trans->adding_csums = true;
> >               ret = btrfs_csum_file_blocks(trans, csum_root, sum);
> >               trans->adding_csums = false;
> >               if (ret)
> > diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> > index baadaaa189c0..230dd93dad6e 100644
> > --- a/fs/btrfs/raid56.c
> > +++ b/fs/btrfs/raid56.c
> > @@ -2295,8 +2295,7 @@ void raid56_parity_recover(struct bio *bio, struct btrfs_io_context *bioc,
> >  static void fill_data_csums(struct btrfs_raid_bio *rbio)
> >  {
> >       struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
> > -     struct btrfs_root *csum_root = btrfs_csum_root(fs_info,
> > -                                                    rbio->bioc->full_stripe_logical);
> > +     struct btrfs_root *csum_root;
> >       const u64 start = rbio->bioc->full_stripe_logical;
> >       const u32 len = (rbio->nr_data * rbio->stripe_nsectors) <<
> >                       fs_info->sectorsize_bits;
> > @@ -2329,6 +2328,15 @@ static void fill_data_csums(struct btrfs_raid_bio *rbio)
> >               goto error;
> >       }
> >
> > +     csum_root = btrfs_csum_root(fs_info, rbio->bioc->full_stripe_logical);
> > +     if (unlikely(!csum_root)) {
> > +             btrfs_err(fs_info,
> > +                       "missing csum root for extent at bytenr %llu",
> > +                       rbio->bioc->full_stripe_logical);
> > +             ret = -EUCLEAN;
> > +             goto error;
> > +     }
> > +
> >       ret = btrfs_lookup_csums_bitmap(csum_root, NULL, start, start + len - 1,
> >                                       rbio->csum_buf, rbio->csum_bitmap);
> >       if (ret < 0)
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 3906e457d2ef..8a8a66112d42 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -5641,6 +5641,14 @@ int btrfs_reloc_clone_csums(struct btrfs_ordered_extent *ordered)
> >       LIST_HEAD(list);
> >       int ret;
> >
> > +     if (unlikely(!csum_root)) {
> > +             btrfs_mark_ordered_extent_error(ordered);
> > +             btrfs_err(fs_info,
> > +                       "missing csum root for extent at bytenr %llu",
> > +                       disk_bytenr);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
> >                                     disk_bytenr + ordered->num_bytes - 1,
> >                                     &list, false);
> > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> > index e2806ca410f6..e9655095ba4c 100644
> > --- a/fs/btrfs/tree-log.c
> > +++ b/fs/btrfs/tree-log.c
> > @@ -984,6 +984,13 @@ static noinline int replay_one_extent(struct walk_control *wc)
> >
> >               sums = list_first_entry(&ordered_sums, struct btrfs_ordered_sum, list);
> >               csum_root = btrfs_csum_root(fs_info, sums->logical);
> > +             if (unlikely(!csum_root)) {
> > +                     btrfs_err(fs_info,
> > +                               "missing csum root for extent at bytenr %llu",
> > +                               sums->logical);
> > +                     ret = -EUCLEAN;
> > +             }
> > +
> >               if (!ret) {
> >                       ret = btrfs_del_csums(trans, csum_root, sums->logical,
> >                                             sums->len);
> > @@ -4890,6 +4897,13 @@ static noinline int copy_items(struct btrfs_trans_handle *trans,
> >               }
> >
> >               csum_root = btrfs_csum_root(trans->fs_info, disk_bytenr);
> > +             if (unlikely(!csum_root)) {
> > +                     btrfs_err(trans->fs_info,
> > +                               "missing csum root for extent at bytenr %llu",
> > +                               disk_bytenr);
> > +                     return -EUCLEAN;
> > +             }
> > +
> >               disk_bytenr += extent_offset;
> >               ret = btrfs_lookup_csums_list(csum_root, disk_bytenr,
> >                                             disk_bytenr + extent_num_bytes - 1,
> > @@ -5086,6 +5100,13 @@ static int log_extent_csums(struct btrfs_trans_handle *trans,
> >       /* block start is already adjusted for the file extent offset. */
> >       block_start = btrfs_extent_map_block_start(em);
> >       csum_root = btrfs_csum_root(trans->fs_info, block_start);
> > +     if (unlikely(!csum_root)) {
> > +             btrfs_err(trans->fs_info,
> > +                       "missing csum root for extent at bytenr %llu",
> > +                       block_start);
> > +             return -EUCLEAN;
> > +     }
> > +
> >       ret = btrfs_lookup_csums_list(csum_root, block_start + csum_offset,
> >                                     block_start + csum_offset + csum_len - 1,
> >                                     &ordered_sums, false);
> > --
> > 2.47.2
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots
  2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
                     ` (2 preceding siblings ...)
  2026-02-10 11:57   ` [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
@ 2026-02-11 17:40   ` Boris Burkov
  3 siblings, 0 replies; 14+ messages in thread
From: Boris Burkov @ 2026-02-11 17:40 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Tue, Feb 10, 2026 at 11:57:47AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Remove a wrong but harmless expression before loading an extent root and
> add missing error handling for some root search functions, which can
> return NULL when using the extent tree v2 feature. Some places have the
> NULL checks already, but many others completely ignore it. Chris recently
> reported this with his AI generated reviews.

A few nits/questions on the patches, but please feel free to add

Reviewed-by: Boris Burkov <boris@bur.io>

regardless of how you choose to respond to those.

Thanks,
Boris

> 
> V2: Fixed a return without mutex unlock in patch 2/3, updated patch 3/3
>     to log the logical bytenr of a delayed ref in case we can't find
>     the csum root.
> 
> Filipe Manana (3):
>   btrfs: remove bogus root search condition in load_extent_tree_free()
>   btrfs: check for NULL root after calls to btrfs_extent_root()
>   btrfs: check for NULL root after calls to btrfs_csum_root()
> 
>  fs/btrfs/backref.c         | 28 +++++++++++
>  fs/btrfs/block-group.c     | 39 ++++++++++++++-
>  fs/btrfs/disk-io.c         | 17 ++++++-
>  fs/btrfs/extent-tree.c     | 98 ++++++++++++++++++++++++++++++++++++--
>  fs/btrfs/file-item.c       |  7 +++
>  fs/btrfs/free-space-tree.c |  9 +++-
>  fs/btrfs/inode.c           | 18 ++++++-
>  fs/btrfs/qgroup.c          |  8 ++++
>  fs/btrfs/raid56.c          | 12 ++++-
>  fs/btrfs/relocation.c      | 30 +++++++++++-
>  fs/btrfs/tree-log.c        | 21 ++++++++
>  fs/btrfs/zoned.c           |  7 +++
>  12 files changed, 278 insertions(+), 16 deletions(-)
> 
> -- 
> 2.47.2
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-02-11 17:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-08 20:00 [PATCH 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
2026-02-08 20:00 ` [PATCH 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
2026-02-08 20:00 ` [PATCH 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
2026-02-09 15:41   ` Filipe Manana
2026-02-08 20:00 ` [PATCH 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
2026-02-10 11:57 ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots fdmanana
2026-02-10 11:57   ` [PATCH v2 1/3] btrfs: remove bogus root search condition in load_extent_tree_free() fdmanana
2026-02-10 11:57   ` [PATCH v2 2/3] btrfs: check for NULL root after calls to btrfs_extent_root() fdmanana
2026-02-11 17:34     ` Boris Burkov
2026-02-11 17:50       ` Filipe Manana
2026-02-10 11:57   ` [PATCH v2 3/3] btrfs: check for NULL root after calls to btrfs_csum_root() fdmanana
2026-02-11 17:39     ` Boris Burkov
2026-02-11 17:53       ` Filipe Manana
2026-02-11 17:40   ` [PATCH v2 0/3] btrfs: add missing NULL checks when searching for roots Boris Burkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox