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

[Qu Wenruo <wqu@suse.com>]

在 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;