[PATCH] btrfs: replace BUG_ON() with error return in get_new_location()

[PATCH] btrfs: replace BUG_ON() with error return in get_new_location()

[Teng Liu]

In get_new_location(), BUG_ON() crashes the kernel if the looked up
file extent item has any of offset, compression, encryption, or other
encoding set. While entries created by the relocation code itself are
not expected to have these fields set, the values come from on-disk
data and a malformed file system can reach this code with non-zero
values, panicking the kernel during a balance operation.

Replace the BUG_ON() with a return of -EUCLEAN, the established error
code in fs/btrfs/relocation.c for filesystem corruption. The caller in
replace_file_extents() already handles errors from get_new_location()
by breaking out of the loop without aborting the transaction so no
caller changes are needed.

Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
---
 fs/btrfs/relocation.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1c42c5180bdd..ce751c35945f 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -835,10 +835,11 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
 	fi = btrfs_item_ptr(leaf, path->slots[0],
 			    struct btrfs_file_extent_item);
 
-	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
-	       btrfs_file_extent_compression(leaf, fi) ||
-	       btrfs_file_extent_encryption(leaf, fi) ||
-	       btrfs_file_extent_other_encoding(leaf, fi));
+	if (unlikely(btrfs_file_extent_offset(leaf, fi) ||
+		     btrfs_file_extent_compression(leaf, fi) ||
+		     btrfs_file_extent_encryption(leaf, fi) ||
+		     btrfs_file_extent_other_encoding(leaf, fi)))
+		return -EUCLEAN;
 
 	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
 		return -EINVAL;
-- 
2.54.0

Re: [PATCH] btrfs: replace BUG_ON() with error return in get_new_location()

[Qu Wenruo]

在 2026/4/25 15:40, Teng Liu 写道:
> In get_new_location(), BUG_ON() crashes the kernel if the looked up
> file extent item has any of offset, compression, encryption, or other
> encoding set. While entries created by the relocation code itself are
> not expected to have these fields set, the values come from on-disk
> data and a malformed file system can reach this code with non-zero
> values, panicking the kernel during a balance operation.
> 
> Replace the BUG_ON() with a return of -EUCLEAN, the established error
> code in fs/btrfs/relocation.c for filesystem corruption. The caller in
> replace_file_extents() already handles errors from get_new_location()
> by breaking out of the loop without aborting the transaction so no
> caller changes are needed.
> 
> Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
> ---
>   fs/btrfs/relocation.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 1c42c5180bdd..ce751c35945f 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -835,10 +835,11 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
>   	fi = btrfs_item_ptr(leaf, path->slots[0],
>   			    struct btrfs_file_extent_item);
>   
> -	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
> -	       btrfs_file_extent_compression(leaf, fi) ||
> -	       btrfs_file_extent_encryption(leaf, fi) ||
> -	       btrfs_file_extent_other_encoding(leaf, fi));
> +	if (unlikely(btrfs_file_extent_offset(leaf, fi) ||
> +		     btrfs_file_extent_compression(leaf, fi) ||
> +		     btrfs_file_extent_encryption(leaf, fi) ||
> +		     btrfs_file_extent_other_encoding(leaf, fi)))
> +		return -EUCLEAN;

Every EUCLEAN should be paired with an error message.

And in this particular case, it's better to provide the tree dump, to 
really be sure that such data is corrupted other than caused by some bugs.

>   
>   	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
>   		return -EINVAL;

[PATCH v2] btrfs: replace BUG_ON() with error return in get_new_location()

[Teng Liu]

In get_new_location(), BUG_ON() crashes the kernel if the looked up
file extent item has any of offset, compression, encryption, or other
encoding set. The data reloc inode this lookup targets is populated
solely by relocation's own prealloc and writeback paths:

  - insert_prealloc_file_extent() memsets the stack item to 0 and only
    sets type/disk_bytenr/disk_num_bytes/num_bytes, so the four checked
    fields stay 0.
  - insert_ordered_extent_file_extent() copies oe->compress_type into
    file_extent compression, but the data reloc inode is created with
    BTRFS_INODE_NOCOMPRESS, so compress_type is always 0; encryption
    and other_encoding are reserved-and-zero per the comment in that
    function.

So observing non-zero compression, encryption or other_encoding here
should on-disk corruption. A malformed image can reach this code
through a balance operation and panic the kernel.

Replace the BUG_ON() with a return of -EUCLEAN, the established error
code in fs/btrfs/relocation.c for filesystem corruption, and pair it
with btrfs_print_leaf() and btrfs_err() as suggested by Qu allowing dmesg
to log the necessary information. The caller in replace_file_extents() 
already handles errors from get_new_location() by breaking out of the loop 
without aborting the transaction, so no caller changes are needed.

Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
---
Changes in v2:
 - Pair the -EUCLEAN return with btrfs_print_leaf() and btrfs_err() so
   the offending leaf is dumped to dmesg, per Qu's review:
   https://lore.kernel.org/linux-btrfs/6c54901d-5e07-4c46-9553-997b28c93b86@suse.com/
 - Expand the changelog to argue why non-zero compression/encryption/
   other_encoding in the data reloc inode imply on-disk corruption
   rather than a kernel bug.

 fs/btrfs/relocation.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1c42c5180bdd..bba28866df1c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -814,6 +814,7 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
 			    u64 bytenr, u64 num_bytes)
 {
 	struct btrfs_root *root = BTRFS_I(reloc_inode)->root;
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	BTRFS_PATH_AUTO_FREE(path);
 	struct btrfs_file_extent_item *fi;
 	struct extent_buffer *leaf;
@@ -835,10 +836,16 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
 	fi = btrfs_item_ptr(leaf, path->slots[0],
 			    struct btrfs_file_extent_item);
 
-	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
-	       btrfs_file_extent_compression(leaf, fi) ||
-	       btrfs_file_extent_encryption(leaf, fi) ||
-	       btrfs_file_extent_other_encoding(leaf, fi));
+	if (unlikely(btrfs_file_extent_offset(leaf, fi) ||
+		     btrfs_file_extent_compression(leaf, fi) ||
+		     btrfs_file_extent_encryption(leaf, fi) ||
+		     btrfs_file_extent_other_encoding(leaf, fi))) {
+		btrfs_print_leaf(leaf);
+		btrfs_err(fs_info,
+"unexpected non-zero fields in file extent item for data reloc inode %llu offset %llu (offset/compression/encryption/other_encoding must all be 0)",
+			  btrfs_ino(BTRFS_I(reloc_inode)), bytenr);
+		return -EUCLEAN;
+	}
 
 	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
 		return -EINVAL;
-- 
2.54.0

Re: [PATCH v2] btrfs: replace BUG_ON() with error return in get_new_location()

[Qu Wenruo]

在 2026/4/27 05:46, Teng Liu 写道:
> In get_new_location(), BUG_ON() crashes the kernel if the looked up
> file extent item has any of offset, compression, encryption, or other
> encoding set. The data reloc inode this lookup targets is populated
> solely by relocation's own prealloc and writeback paths:
> 
>    - insert_prealloc_file_extent() memsets the stack item to 0 and only
>      sets type/disk_bytenr/disk_num_bytes/num_bytes, so the four checked
>      fields stay 0.
>    - insert_ordered_extent_file_extent() copies oe->compress_type into
>      file_extent compression, but the data reloc inode is created with
>      BTRFS_INODE_NOCOMPRESS, so compress_type is always 0; encryption
>      and other_encoding are reserved-and-zero per the comment in that
>      function.
> 
> So observing non-zero compression, encryption or other_encoding here
> should on-disk corruption. A malformed image can reach this code
> through a balance operation and panic the kernel.
> 
> Replace the BUG_ON() with a return of -EUCLEAN, the established error
> code in fs/btrfs/relocation.c for filesystem corruption, and pair it
> with btrfs_print_leaf() and btrfs_err() as suggested by Qu allowing dmesg
> to log the necessary information. The caller in replace_file_extents()
> already handles errors from get_new_location() by breaking out of the loop
> without aborting the transaction, so no caller changes are needed.
> 
> Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
> ---
> Changes in v2:
>   - Pair the -EUCLEAN return with btrfs_print_leaf() and btrfs_err() so
>     the offending leaf is dumped to dmesg, per Qu's review:
>     https://lore.kernel.org/linux-btrfs/6c54901d-5e07-4c46-9553-997b28c93b86@suse.com/
>   - Expand the changelog to argue why non-zero compression/encryption/
>     other_encoding in the data reloc inode imply on-disk corruption
>     rather than a kernel bug.
> 
>   fs/btrfs/relocation.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 1c42c5180bdd..bba28866df1c 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -814,6 +814,7 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
>   			    u64 bytenr, u64 num_bytes)
>   {
>   	struct btrfs_root *root = BTRFS_I(reloc_inode)->root;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
>   	BTRFS_PATH_AUTO_FREE(path);
>   	struct btrfs_file_extent_item *fi;
>   	struct extent_buffer *leaf;
> @@ -835,10 +836,16 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
>   	fi = btrfs_item_ptr(leaf, path->slots[0],
>   			    struct btrfs_file_extent_item);
>   
> -	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
> -	       btrfs_file_extent_compression(leaf, fi) ||
> -	       btrfs_file_extent_encryption(leaf, fi) ||
> -	       btrfs_file_extent_other_encoding(leaf, fi));
> +	if (unlikely(btrfs_file_extent_offset(leaf, fi) ||
> +		     btrfs_file_extent_compression(leaf, fi) ||
> +		     btrfs_file_extent_encryption(leaf, fi) ||
> +		     btrfs_file_extent_other_encoding(leaf, fi))) {
> +		btrfs_print_leaf(leaf);
> +		btrfs_err(fs_info,
> +"unexpected non-zero fields in file extent item for data reloc inode %llu offset %llu (offset/compression/encryption/other_encoding must all be 0)",
> +			  btrfs_ino(BTRFS_I(reloc_inode)), bytenr);
> +		return -EUCLEAN;

This looks like exactly what we did in tree-checker.

And we have enough info to do this in tree-checker, the data reloc tree 
has a fixed id and owner in extent buffer header.

What about moving this to tree-checker where we already have 
check_extent_data_item().

Furthermore, if you move this check into tree-checker, it's better to 
print the offset/compress/encryption/other_encoding values.
And remove the "must all be 0", just something like:

file_extent_err(leaf, slot,
		"invalid members for data reloc tree, offset=%llu compress=%llu 
encryption=%llu other_encoding=%llu",
		...);


> +	}
>   
>   	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
>   		return -EINVAL;

Re: [PATCH v2] btrfs: replace BUG_ON() with error return in get_new_location()

[David Sterba]

On Mon, Apr 27, 2026 at 10:49:26AM +0930, Qu Wenruo wrote:
> > @@ -835,10 +836,16 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
> >   	fi = btrfs_item_ptr(leaf, path->slots[0],
> >   			    struct btrfs_file_extent_item);
> >   
> > -	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
> > -	       btrfs_file_extent_compression(leaf, fi) ||
> > -	       btrfs_file_extent_encryption(leaf, fi) ||
> > -	       btrfs_file_extent_other_encoding(leaf, fi));
> > +	if (unlikely(btrfs_file_extent_offset(leaf, fi) ||
> > +		     btrfs_file_extent_compression(leaf, fi) ||
> > +		     btrfs_file_extent_encryption(leaf, fi) ||
> > +		     btrfs_file_extent_other_encoding(leaf, fi))) {
> > +		btrfs_print_leaf(leaf);
> > +		btrfs_err(fs_info,
> > +"unexpected non-zero fields in file extent item for data reloc inode %llu offset %llu (offset/compression/encryption/other_encoding must all be 0)",
> > +			  btrfs_ino(BTRFS_I(reloc_inode)), bytenr);
> > +		return -EUCLEAN;
> 
> This looks like exactly what we did in tree-checker.
> 
> And we have enough info to do this in tree-checker, the data reloc tree 
> has a fixed id and owner in extent buffer header.
> 
> What about moving this to tree-checker where we already have 
> check_extent_data_item().

I think it would be better in the tree-checker, as this verifies the
item as it's read and not after the relocatikon is started.

The BUG_ON can be turned to an assertion though it duplicates some of
the checks, but this is otherwise harmless and cheap.

[PATCH v3] btrfs: validate data reloc tree file extent item members in tree-checker

[Teng Liu]

get_new_location() uses BUG_ON() to crash the kernel if the file extent
item it looks up has any of offset, compression, encryption, or
other_encoding set. The data reloc inode is only written by relocation's
own paths -- insert_prealloc_file_extent() and
insert_ordered_extent_file_extent() -- which always leave those four
fields at 0 (the data reloc inode is created with BTRFS_INODE_NOCOMPRESS,
and encryption/other_encoding are reserved-and-zero). Observing a
non-zero value therefore means the leaf decoded from disk does not match
what the kernel wrote, i.e. on-disk corruption. A malformed image can
reach this code via balance and panic the kernel.

Move the validation into tree-checker's check_extent_data_item(), where
the constraint is enforced when the leaf is read off disk rather than
after relocation has already started. The data reloc tree has a fixed
root id (BTRFS_DATA_RELOC_TREE_OBJECTID) recorded in the extent buffer
header, so check_extent_data_item() has all the information it needs to
apply this check on its own. Report violations via file_extent_err() and
print the four offending values.

In get_new_location() replace the BUG_ON() with an ASSERT().
The caller in replace_file_extents() already handles non-zero returns from
get_new_location() by breaking out of the loop without aborting the
transaction, so no caller changes are needed.

Suggested-by: Qu Wenruo <wqu@suse.com>
Suggested-by: David Sterba <dsterba@suse.com>
Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
---
Changes in v3:
 - Move the corruption check from relocation.c into tree-checker's
   check_extent_data_item(), per Qu and David. The data reloc tree's
   fixed objectid is recorded in the extent buffer header, so the
   check has all the context it needs at read time.
 - Use file_extent_err() and print offset/compression/encryption/
   other_encoding values, per Qu.
 - Replace the BUG_ON in get_new_location() with ASSERT() rather than
   -EUCLEAN, per David.

Changes in v2:
 - Pair the -EUCLEAN return with btrfs_print_leaf() and btrfs_err() so
   the offending leaf is dumped to dmesg, per Qu's review of v1.
 - Expand the changelog to argue why non-zero
   compression/encryption/other_encoding in the data reloc inode imply
   on-disk corruption.

 fs/btrfs/relocation.c   |  8 ++++----
 fs/btrfs/tree-checker.c | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 1c42c5180bdd..527d4dbfe31c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -835,10 +835,10 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
 	fi = btrfs_item_ptr(leaf, path->slots[0],
 			    struct btrfs_file_extent_item);
 
-	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
-	       btrfs_file_extent_compression(leaf, fi) ||
-	       btrfs_file_extent_encryption(leaf, fi) ||
-	       btrfs_file_extent_other_encoding(leaf, fi));
+	ASSERT(!btrfs_file_extent_offset(leaf, fi) &&
+	       !btrfs_file_extent_compression(leaf, fi) &&
+	       !btrfs_file_extent_encryption(leaf, fi) &&
+	       !btrfs_file_extent_other_encoding(leaf, fi));
 
 	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
 		return -EINVAL;
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 1f15d0793a9c..e4864f7a471e 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -296,6 +296,27 @@ static int check_extent_data_item(struct extent_buffer *leaf,
 		return 0;
 	}
 
+	/*
+	 * For the data reloc tree, file extent items are written by
+	 * relocation's own paths, which always leave offset, compression,
+	 * encryption and other_encoding as 0. Any non-zero value here means
+	 * the leaf decoded from disk does not match what the kernel wrote,
+	 * i.e. on-disk corruption.
+	 */
+	if (unlikely(btrfs_header_owner(leaf) == BTRFS_DATA_RELOC_TREE_OBJECTID &&
+		     (btrfs_file_extent_offset(leaf, fi) ||
+		      btrfs_file_extent_compression(leaf, fi) ||
+		      btrfs_file_extent_encryption(leaf, fi) ||
+		      btrfs_file_extent_other_encoding(leaf, fi)))) {
+		file_extent_err(leaf, slot,
+"invalid members for data reloc tree, offset=%llu compress=%u encryption=%u other_encoding=%u",
+				btrfs_file_extent_offset(leaf, fi),
+				btrfs_file_extent_compression(leaf, fi),
+				btrfs_file_extent_encryption(leaf, fi),
+				btrfs_file_extent_other_encoding(leaf, fi));
+		return -EUCLEAN;
+	}
+
 	/* Regular or preallocated extent has fixed item size */
 	if (unlikely(item_size != sizeof(*fi))) {
 		file_extent_err(leaf, slot,
-- 
2.54.0

Re: [PATCH v3] btrfs: validate data reloc tree file extent item members in tree-checker

[Qu Wenruo]

在 2026/4/28 05:54, Teng Liu 写道:
> get_new_location() uses BUG_ON() to crash the kernel if the file extent
> item it looks up has any of offset, compression, encryption, or
> other_encoding set. The data reloc inode is only written by relocation's
> own paths -- insert_prealloc_file_extent() and
> insert_ordered_extent_file_extent() -- which always leave those four
> fields at 0 (the data reloc inode is created with BTRFS_INODE_NOCOMPRESS,
> and encryption/other_encoding are reserved-and-zero). Observing a
> non-zero value therefore means the leaf decoded from disk does not match
> what the kernel wrote, i.e. on-disk corruption. A malformed image can
> reach this code via balance and panic the kernel.
> 
> Move the validation into tree-checker's check_extent_data_item(), where
> the constraint is enforced when the leaf is read off disk rather than
> after relocation has already started. The data reloc tree has a fixed
> root id (BTRFS_DATA_RELOC_TREE_OBJECTID) recorded in the extent buffer
> header, so check_extent_data_item() has all the information it needs to
> apply this check on its own. Report violations via file_extent_err() and
> print the four offending values.
> 
> In get_new_location() replace the BUG_ON() with an ASSERT().
> The caller in replace_file_extents() already handles non-zero returns from
> get_new_location() by breaking out of the loop without aborting the
> transaction, so no caller changes are needed.
> 
> Suggested-by: Qu Wenruo <wqu@suse.com>
> Suggested-by: David Sterba <dsterba@suse.com>
> Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>

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

And merged.

Thanks,
Qu

> ---
> Changes in v3:
>   - Move the corruption check from relocation.c into tree-checker's
>     check_extent_data_item(), per Qu and David. The data reloc tree's
>     fixed objectid is recorded in the extent buffer header, so the
>     check has all the context it needs at read time.
>   - Use file_extent_err() and print offset/compression/encryption/
>     other_encoding values, per Qu.
>   - Replace the BUG_ON in get_new_location() with ASSERT() rather than
>     -EUCLEAN, per David.
> 
> Changes in v2:
>   - Pair the -EUCLEAN return with btrfs_print_leaf() and btrfs_err() so
>     the offending leaf is dumped to dmesg, per Qu's review of v1.
>   - Expand the changelog to argue why non-zero
>     compression/encryption/other_encoding in the data reloc inode imply
>     on-disk corruption.
> 
>   fs/btrfs/relocation.c   |  8 ++++----
>   fs/btrfs/tree-checker.c | 21 +++++++++++++++++++++
>   2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 1c42c5180bdd..527d4dbfe31c 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -835,10 +835,10 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
>   	fi = btrfs_item_ptr(leaf, path->slots[0],
>   			    struct btrfs_file_extent_item);
>   
> -	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
> -	       btrfs_file_extent_compression(leaf, fi) ||
> -	       btrfs_file_extent_encryption(leaf, fi) ||
> -	       btrfs_file_extent_other_encoding(leaf, fi));
> +	ASSERT(!btrfs_file_extent_offset(leaf, fi) &&
> +	       !btrfs_file_extent_compression(leaf, fi) &&
> +	       !btrfs_file_extent_encryption(leaf, fi) &&
> +	       !btrfs_file_extent_other_encoding(leaf, fi));
>   
>   	if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
>   		return -EINVAL;
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 1f15d0793a9c..e4864f7a471e 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -296,6 +296,27 @@ static int check_extent_data_item(struct extent_buffer *leaf,
>   		return 0;
>   	}
>   
> +	/*
> +	 * For the data reloc tree, file extent items are written by
> +	 * relocation's own paths, which always leave offset, compression,
> +	 * encryption and other_encoding as 0. Any non-zero value here means
> +	 * the leaf decoded from disk does not match what the kernel wrote,
> +	 * i.e. on-disk corruption.
> +	 */
> +	if (unlikely(btrfs_header_owner(leaf) == BTRFS_DATA_RELOC_TREE_OBJECTID &&
> +		     (btrfs_file_extent_offset(leaf, fi) ||
> +		      btrfs_file_extent_compression(leaf, fi) ||
> +		      btrfs_file_extent_encryption(leaf, fi) ||
> +		      btrfs_file_extent_other_encoding(leaf, fi)))) {
> +		file_extent_err(leaf, slot,
> +"invalid members for data reloc tree, offset=%llu compress=%u encryption=%u other_encoding=%u",
> +				btrfs_file_extent_offset(leaf, fi),
> +				btrfs_file_extent_compression(leaf, fi),
> +				btrfs_file_extent_encryption(leaf, fi),
> +				btrfs_file_extent_other_encoding(leaf, fi));
> +		return -EUCLEAN;
> +	}
> +
>   	/* Regular or preallocated extent has fixed item size */
>   	if (unlikely(item_size != sizeof(*fi))) {
>   		file_extent_err(leaf, slot,

Re: [PATCH v3] btrfs: validate data reloc tree file extent item members in tree-checker

[Qu Wenruo]

在 2026/4/28 07:45, Qu Wenruo 写道:
> 
> 
> 在 2026/4/28 05:54, Teng Liu 写道:
>> get_new_location() uses BUG_ON() to crash the kernel if the file extent
>> item it looks up has any of offset, compression, encryption, or
>> other_encoding set. The data reloc inode is only written by relocation's
>> own paths -- insert_prealloc_file_extent() and
>> insert_ordered_extent_file_extent() -- which always leave those four
>> fields at 0 (the data reloc inode is created with BTRFS_INODE_NOCOMPRESS,
>> and encryption/other_encoding are reserved-and-zero). Observing a
>> non-zero value therefore means the leaf decoded from disk does not match
>> what the kernel wrote, i.e. on-disk corruption. A malformed image can
>> reach this code via balance and panic the kernel.
>>
>> Move the validation into tree-checker's check_extent_data_item(), where
>> the constraint is enforced when the leaf is read off disk rather than
>> after relocation has already started. The data reloc tree has a fixed
>> root id (BTRFS_DATA_RELOC_TREE_OBJECTID) recorded in the extent buffer
>> header, so check_extent_data_item() has all the information it needs to
>> apply this check on its own. Report violations via file_extent_err() and
>> print the four offending values.
>>
>> In get_new_location() replace the BUG_ON() with an ASSERT().
>> The caller in replace_file_extents() already handles non-zero returns 
>> from
>> get_new_location() by breaking out of the loop without aborting the
>> transaction, so no caller changes are needed.
>>
>> Suggested-by: Qu Wenruo <wqu@suse.com>
>> Suggested-by: David Sterba <dsterba@suse.com>
>> Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
>> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> And merged.

Unfortunately this tree-checker got triggered during btrfs/061 runs at 
write-time tree-checker, with arm64 64K page size.

The offending file extent is as the following:

[  536.885066] 	item 69 key (258 EXTENT_DATA 4063232) itemoff 12400 
itemsize 53
[  536.885067] 		generation 28 type 1
[  536.885067] 		extent data disk bytenr 10512723968 nr 36864
[  536.885068] 		extent data offset 24576 nr 12288 ram 36864
[  536.885069] 		extent compression 0

Note the offset is not zero, and the type is 1 which means it's a 
regular file extent.

So the check is causing false alerts.

Now the patch is reverted, and I'll spend more time digging into the case.

Thanks,
Qu

> 
> Thanks,
> Qu
> 
>> ---
>> Changes in v3:
>>   - Move the corruption check from relocation.c into tree-checker's
>>     check_extent_data_item(), per Qu and David. The data reloc tree's
>>     fixed objectid is recorded in the extent buffer header, so the
>>     check has all the context it needs at read time.
>>   - Use file_extent_err() and print offset/compression/encryption/
>>     other_encoding values, per Qu.
>>   - Replace the BUG_ON in get_new_location() with ASSERT() rather than
>>     -EUCLEAN, per David.
>>
>> Changes in v2:
>>   - Pair the -EUCLEAN return with btrfs_print_leaf() and btrfs_err() so
>>     the offending leaf is dumped to dmesg, per Qu's review of v1.
>>   - Expand the changelog to argue why non-zero
>>     compression/encryption/other_encoding in the data reloc inode imply
>>     on-disk corruption.
>>
>>   fs/btrfs/relocation.c   |  8 ++++----
>>   fs/btrfs/tree-checker.c | 21 +++++++++++++++++++++
>>   2 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 1c42c5180bdd..527d4dbfe31c 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -835,10 +835,10 @@ static int get_new_location(struct inode 
>> *reloc_inode, u64 *new_bytenr,
>>       fi = btrfs_item_ptr(leaf, path->slots[0],
>>                   struct btrfs_file_extent_item);
>> -    BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
>> -           btrfs_file_extent_compression(leaf, fi) ||
>> -           btrfs_file_extent_encryption(leaf, fi) ||
>> -           btrfs_file_extent_other_encoding(leaf, fi));
>> +    ASSERT(!btrfs_file_extent_offset(leaf, fi) &&
>> +           !btrfs_file_extent_compression(leaf, fi) &&
>> +           !btrfs_file_extent_encryption(leaf, fi) &&
>> +           !btrfs_file_extent_other_encoding(leaf, fi));
>>       if (num_bytes != btrfs_file_extent_disk_num_bytes(leaf, fi))
>>           return -EINVAL;
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 1f15d0793a9c..e4864f7a471e 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -296,6 +296,27 @@ static int check_extent_data_item(struct 
>> extent_buffer *leaf,
>>           return 0;
>>       }
>> +    /*
>> +     * For the data reloc tree, file extent items are written by
>> +     * relocation's own paths, which always leave offset, compression,
>> +     * encryption and other_encoding as 0. Any non-zero value here means
>> +     * the leaf decoded from disk does not match what the kernel wrote,
>> +     * i.e. on-disk corruption.
>> +     */
>> +    if (unlikely(btrfs_header_owner(leaf) == 
>> BTRFS_DATA_RELOC_TREE_OBJECTID &&
>> +             (btrfs_file_extent_offset(leaf, fi) ||
>> +              btrfs_file_extent_compression(leaf, fi) ||
>> +              btrfs_file_extent_encryption(leaf, fi) ||
>> +              btrfs_file_extent_other_encoding(leaf, fi)))) {
>> +        file_extent_err(leaf, slot,
>> +"invalid members for data reloc tree, offset=%llu compress=%u 
>> encryption=%u other_encoding=%u",
>> +                btrfs_file_extent_offset(leaf, fi),
>> +                btrfs_file_extent_compression(leaf, fi),
>> +                btrfs_file_extent_encryption(leaf, fi),
>> +                btrfs_file_extent_other_encoding(leaf, fi));
>> +        return -EUCLEAN;
>> +    }
>> +
>>       /* Regular or preallocated extent has fixed item size */
>>       if (unlikely(item_size != sizeof(*fi))) {
>>           file_extent_err(leaf, slot,
> 
> 

Re: [PATCH v3] btrfs: validate data reloc tree file extent item members in tree-checker

[Johannes Thumshirn]

On 4/27/26 10:24 PM, Teng Liu wrote:
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 1c42c5180bdd..527d4dbfe31c 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -835,10 +835,10 @@ static int get_new_location(struct inode *reloc_inode, u64 *new_bytenr,
>   	fi = btrfs_item_ptr(leaf, path->slots[0],
>   			    struct btrfs_file_extent_item);
>   
> -	BUG_ON(btrfs_file_extent_offset(leaf, fi) ||
> -	       btrfs_file_extent_compression(leaf, fi) ||
> -	       btrfs_file_extent_encryption(leaf, fi) ||
> -	       btrfs_file_extent_other_encoding(leaf, fi));
> +	ASSERT(!btrfs_file_extent_offset(leaf, fi) &&
> +	       !btrfs_file_extent_compression(leaf, fi) &&
> +	       !btrfs_file_extent_encryption(leaf, fi) &&
> +	       !btrfs_file_extent_other_encoding(leaf, fi));

Can you split that into multiple ASSERT()s? So we quickly see which one 
actually triggered.

Re: [PATCH v3] btrfs: validate data reloc tree file extent item members in tree-checker

[David Sterba]

On Tue, Apr 28, 2026 at 10:14:40AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2026/4/28 07:45, Qu Wenruo 写道:
> > 
> > 
> > 在 2026/4/28 05:54, Teng Liu 写道:
> >> get_new_location() uses BUG_ON() to crash the kernel if the file extent
> >> item it looks up has any of offset, compression, encryption, or
> >> other_encoding set. The data reloc inode is only written by relocation's
> >> own paths -- insert_prealloc_file_extent() and
> >> insert_ordered_extent_file_extent() -- which always leave those four
> >> fields at 0 (the data reloc inode is created with BTRFS_INODE_NOCOMPRESS,
> >> and encryption/other_encoding are reserved-and-zero). Observing a
> >> non-zero value therefore means the leaf decoded from disk does not match
> >> what the kernel wrote, i.e. on-disk corruption. A malformed image can
> >> reach this code via balance and panic the kernel.
> >>
> >> Move the validation into tree-checker's check_extent_data_item(), where
> >> the constraint is enforced when the leaf is read off disk rather than
> >> after relocation has already started. The data reloc tree has a fixed
> >> root id (BTRFS_DATA_RELOC_TREE_OBJECTID) recorded in the extent buffer
> >> header, so check_extent_data_item() has all the information it needs to
> >> apply this check on its own. Report violations via file_extent_err() and
> >> print the four offending values.
> >>
> >> In get_new_location() replace the BUG_ON() with an ASSERT().
> >> The caller in replace_file_extents() already handles non-zero returns 
> >> from
> >> get_new_location() by breaking out of the loop without aborting the
> >> transaction, so no caller changes are needed.
> >>
> >> Suggested-by: Qu Wenruo <wqu@suse.com>
> >> Suggested-by: David Sterba <dsterba@suse.com>
> >> Reported-by: syzbot+3e20d8f3d41bac5dc9a2@syzkaller.appspotmail.com
> >> Closes: https://syzkaller.appspot.com/bug?extid=3e20d8f3d41bac5dc9a2
> >> Signed-off-by: Teng Liu <27rabbitlt@gmail.com>
> > 
> > Reviewed-by: Qu Wenruo <wqu@suse.com>
> > 
> > And merged.
> 
> Unfortunately this tree-checker got triggered during btrfs/061 runs at 
> write-time tree-checker, with arm64 64K page size.
> 
> The offending file extent is as the following:
> 
> [  536.885066] 	item 69 key (258 EXTENT_DATA 4063232) itemoff 12400 
> itemsize 53
> [  536.885067] 		generation 28 type 1
> [  536.885067] 		extent data disk bytenr 10512723968 nr 36864
> [  536.885068] 		extent data offset 24576 nr 12288 ram 36864
> [  536.885069] 		extent compression 0
> 
> Note the offset is not zero, and the type is 1 which means it's a 
> regular file extent.
> 
> So the check is causing false alerts.

I maybe have an idea.  The difference from the BUG_ON and the
tree-checker is the context where it's called. In relocation it's
somewere in the middle and there are actions fixing up the offset. OTOH
when this is done in tree-checker the constraints are different.

  get_new_location() - verifies offset, compression, ...

The offset corresponds to 'bytenr' and is returned via *new_bytenr to
replace_file_extents() and then updated in the leaf

  btrfs_set_file_extent_disk_bytenr(leaf, fi, new_bytenr);

This eventually ends up in in the pre-write check.