[PATCH] btrfs: free mapping node on duplicate reloc root insert

[PATCH] btrfs: free mapping node on duplicate reloc root insert

[Guanghui Yang]

__add_reloc_root() allocates a mapping_node before inserting it into
rc->reloc_root_tree.  If rb_simple_insert() finds an existing entry, it
returns the existing rb_node and leaves the newly allocated node unlinked.

The error path then returns -EEXIST without freeing the new node.  Since
the node was never inserted into reloc_root_tree, the later cleanup in
put_reloc_control() cannot find it either.

Free the newly allocated node before returning -EEXIST.

The callers currently assert that -EEXIST should not happen, so this is a
defensive cleanup for an unexpected duplicate insert path.  If the path is
ever reached, the local allocation should still be released.

Signed-off-by: Guanghui Yang <3497809730@qq.com>
---
 fs/btrfs/relocation.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index fb85bc8b345c..7e451e587726 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -588,6 +588,7 @@ static int __add_reloc_root(struct btrfs_root *root, struct reloc_control *rc)
 		btrfs_err(fs_info,
 			    "Duplicate root found for start=%llu while inserting into relocation tree",
 			    node->bytenr);
+		kfree(node);
 		return -EEXIST;
 	}
 
-- 
2.34.1

Re: [PATCH] btrfs: free mapping node on duplicate reloc root insert

[Qu Wenruo]

在 2026/7/12 01:11, Guanghui Yang 写道:
> __add_reloc_root() allocates a mapping_node before inserting it into
> rc->reloc_root_tree.  If rb_simple_insert() finds an existing entry, it
> returns the existing rb_node and leaves the newly allocated node unlinked.
> 
> The error path then returns -EEXIST without freeing the new node.  Since
> the node was never inserted into reloc_root_tree, the later cleanup in
> put_reloc_control() cannot find it either.
> 
> Free the newly allocated node before returning -EEXIST.
> 
> The callers currently assert that -EEXIST should not happen, so this is a
> defensive cleanup for an unexpected duplicate insert path.  If the path is
> ever reached, the local allocation should still be released.
> 
> Signed-off-by: Guanghui Yang <3497809730@qq.com>

Missing fixes tag.

In commit ffd7b33944f4 ("btrfs: __add_reloc_root error push-up") there 
is still the proper kfree(), but now it's not there.

There must be a commit removing that kfree().

> ---
>   fs/btrfs/relocation.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index fb85bc8b345c..7e451e587726 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -588,6 +588,7 @@ static int __add_reloc_root(struct btrfs_root *root, struct reloc_control *rc)
>   		btrfs_err(fs_info,
>   			    "Duplicate root found for start=%llu while inserting into relocation tree",
>   			    node->bytenr);
> +		kfree(node);
>   		return -EEXIST;
>   	}
>   

[PATCH v2] ext4: propagate errors from fast commit range replay

[Guanghui Yang]

ext4_fc_replay() stops replaying fast commit tags only when a tag
handler returns a negative error. However, ext4_fc_replay_add_range()
and ext4_fc_replay_del_range() currently return 0 from their common
exit paths even after internal failures.

This hides errors from ext4_fc_record_modified_inode(),
ext4_map_blocks(), ext4_find_extent(), ext4_ext_insert_extent(),
ext4_ext_replay_update_ex(), and ext4_ext_remove_space(). As a result,
a failed ADD_RANGE or DEL_RANGE replay can be treated as successful and
the replay code may continue with subsequent fast commit tags.

This is particularly problematic for DEL_RANGE because it may already
have marked blocks as free before ext4_ext_remove_space() fails. If the
error is swallowed, replay may continue from a partially applied range
operation.

Return the saved error from the common exit paths and make the
ERR_PTR() cases in ADD_RANGE store PTR_ERR() before jumping to out.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@vger.kernel.org
Fixes: 57a304cfd43b ("btrfs: do not panic in __add_reloc_root")
Signed-off-by: Guanghui Yang <3497809730@qq.com>
---
Changes in v2:
- Add Fixes tag for the commit that made the duplicate-insert error path reachable.

 fs/ext4/fast_commit.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 8e2259799614..fbb486d917b0 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2196,8 +2196,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
 		if (ret == 0) {
 			/* Range is not mapped */
 			path = ext4_find_extent(inode, cur, path, 0);
-			if (IS_ERR(path))
+			if (IS_ERR(path)) {
+				ret = PTR_ERR(path);
+				path = NULL;
 				goto out;
+			}
 			memset(&newex, 0, sizeof(newex));
 			newex.ee_block = cpu_to_le32(cur);
 			ext4_ext_store_pblock(
@@ -2209,8 +2212,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
 			path = ext4_ext_insert_extent(NULL, inode,
 						      path, &newex, 0);
 			up_write((&EXT4_I(inode)->i_data_sem));
-			if (IS_ERR(path))
+			if (IS_ERR(path)) {
+				ret = PTR_ERR(path);
+				path = NULL;
 				goto out;
+			}
 			goto next;
 		}
 
@@ -2257,10 +2263,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
 	}
 	ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >>
 					sb->s_blocksize_bits);
+	ret = 0;
 out:
 	ext4_free_ext_path(path);
 	iput(inode);
-	return 0;
+	return ret;
 }
 
 /* Replay DEL_RANGE tag */
@@ -2320,9 +2327,10 @@ ext4_fc_replay_del_range(struct super_block *sb, u8 *val)
 	ext4_ext_replay_shrink_inode(inode,
 		i_size_read(inode) >> sb->s_blocksize_bits);
 	ext4_mark_inode_dirty(NULL, inode);
+	ret = 0;
 out:
 	iput(inode);
-	return 0;
+	return ret;
 }
 
 static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)
-- 
2.34.1

Re: [PATCH v2] ext4: propagate errors from fast commit range replay

[Qu Wenruo]

在 2026/7/12 12:41, Guanghui Yang 写道:
> ext4_fc_replay() stops replaying fast commit tags only when a tag
> handler returns a negative error. However, ext4_fc_replay_add_range()
> and ext4_fc_replay_del_range() currently return 0 from their common
> exit paths even after internal failures.
> 
> This hides errors from ext4_fc_record_modified_inode(),
> ext4_map_blocks(), ext4_find_extent(), ext4_ext_insert_extent(),
> ext4_ext_replay_update_ex(), and ext4_ext_remove_space(). As a result,
> a failed ADD_RANGE or DEL_RANGE replay can be treated as successful and
> the replay code may continue with subsequent fast commit tags.
> 
> This is particularly problematic for DEL_RANGE because it may already
> have marked blocks as free before ext4_ext_remove_space() fails. If the
> error is swallowed, replay may continue from a partially applied range
> operation.
> 
> Return the saved error from the common exit paths and make the
> ERR_PTR() cases in ADD_RANGE store PTR_ERR() before jumping to out.
> 
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Cc: stable@vger.kernel.org
> Fixes: 57a304cfd43b ("btrfs: do not panic in __add_reloc_root")

WTF? Stop hallucinate, no matter if it's from LLM or yourself.

How could a ext4 commit related to btrfs?

> Signed-off-by: Guanghui Yang <3497809730@qq.com>
> ---
> Changes in v2:
> - Add Fixes tag for the commit that made the duplicate-insert error path reachable.
> 
>   fs/ext4/fast_commit.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 8e2259799614..fbb486d917b0 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2196,8 +2196,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>   		if (ret == 0) {
>   			/* Range is not mapped */
>   			path = ext4_find_extent(inode, cur, path, 0);
> -			if (IS_ERR(path))
> +			if (IS_ERR(path)) {
> +				ret = PTR_ERR(path);
> +				path = NULL;
>   				goto out;
> +			}
>   			memset(&newex, 0, sizeof(newex));
>   			newex.ee_block = cpu_to_le32(cur);
>   			ext4_ext_store_pblock(
> @@ -2209,8 +2212,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>   			path = ext4_ext_insert_extent(NULL, inode,
>   						      path, &newex, 0);
>   			up_write((&EXT4_I(inode)->i_data_sem));
> -			if (IS_ERR(path))
> +			if (IS_ERR(path)) {
> +				ret = PTR_ERR(path);
> +				path = NULL;
>   				goto out;
> +			}
>   			goto next;
>   		}
>   
> @@ -2257,10 +2263,11 @@ static int ext4_fc_replay_add_range(struct super_block *sb, u8 *val)
>   	}
>   	ext4_ext_replay_shrink_inode(inode, i_size_read(inode) >>
>   					sb->s_blocksize_bits);
> +	ret = 0;
>   out:
>   	ext4_free_ext_path(path);
>   	iput(inode);
> -	return 0;
> +	return ret;
>   }
>   
>   /* Replay DEL_RANGE tag */
> @@ -2320,9 +2327,10 @@ ext4_fc_replay_del_range(struct super_block *sb, u8 *val)
>   	ext4_ext_replay_shrink_inode(inode,
>   		i_size_read(inode) >> sb->s_blocksize_bits);
>   	ext4_mark_inode_dirty(NULL, inode);
> +	ret = 0;
>   out:
>   	iput(inode);
> -	return 0;
> +	return ret;
>   }
>   
>   static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)

Re: [PATCH v2] ext4: propagate errors from fast commit range replay

[Guanghui Yang]

You are right. This was my mistake.

I accidentally mixed the btrfs v2 Fixes tag into the unrelated ext4
patch while preparing two patches at the same time.

Please disregard the ext4 v2 email entirely. The correct btrfs v2 was
sent separately and is the only patch relevant to the btrfs thread.

Sorry for the confusion.

Guanghui

[PATCH v2] btrfs: free mapping node on duplicate reloc root insert

[Guanghui Yang]

__add_reloc_root() allocates a mapping_node before inserting it into
rc->reloc_root_tree.  If rb_simple_insert() finds an existing entry, it
returns the existing rb_node and leaves the newly allocated node unlinked.

The error path then returns -EEXIST without freeing the new node.  Since
the node was never inserted into reloc_root_tree, the later cleanup in
put_reloc_control() cannot find it either.

Free the newly allocated node before returning -EEXIST.

The callers currently assert that -EEXIST should not happen, so this is a
defensive cleanup for an unexpected duplicate insert path.  If the path is
ever reached, the local allocation should still be released.

Fixes: 57a304cfd43b ("btrfs: do not panic in __add_reloc_root")
Signed-off-by: Guanghui Yang <3497809730@qq.com>
---
Changes in v2:
- Add Fixes tag as requested by Qu Wenruo.

 fs/btrfs/relocation.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index fb85bc8b345c..7e451e587726 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -588,6 +588,7 @@ static int __add_reloc_root(struct btrfs_root *root, struct reloc_control *rc)
 		btrfs_err(fs_info,
 			    "Duplicate root found for start=%llu while inserting into relocation tree",
 			    node->bytenr);
+		kfree(node);
 		return -EEXIST;
 	}
 
-- 
2.34.1

Re: [PATCH v2] btrfs: free mapping node on duplicate reloc root insert

[Qu Wenruo]

在 2026/7/12 12:47, Guanghui Yang 写道:
> __add_reloc_root() allocates a mapping_node before inserting it into
> rc->reloc_root_tree.  If rb_simple_insert() finds an existing entry, it
> returns the existing rb_node and leaves the newly allocated node unlinked.
> 
> The error path then returns -EEXIST without freeing the new node.  Since
> the node was never inserted into reloc_root_tree, the later cleanup in
> put_reloc_control() cannot find it either.
> 
> Free the newly allocated node before returning -EEXIST.
> 
> The callers currently assert that -EEXIST should not happen, so this is a
> defensive cleanup for an unexpected duplicate insert path.  If the path is
> ever reached, the local allocation should still be released.
> 
> Fixes: 57a304cfd43b ("btrfs: do not panic in __add_reloc_root")
> Signed-off-by: Guanghui Yang <3497809730@qq.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

> ---
> Changes in v2:
> - Add Fixes tag as requested by Qu Wenruo.
> 
>   fs/btrfs/relocation.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index fb85bc8b345c..7e451e587726 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -588,6 +588,7 @@ static int __add_reloc_root(struct btrfs_root *root, struct reloc_control *rc)
>   		btrfs_err(fs_info,
>   			    "Duplicate root found for start=%llu while inserting into relocation tree",
>   			    node->bytenr);
> +		kfree(node);
>   		return -EEXIST;
>   	}
>