Re: [PATCH v4] btrfs: validate root ref names in tree-checker

[Qu Wenruo <wqu@suse.com>]

在 2026/5/11 16:31, Zhang Cen 写道:
> ROOT_REF and ROOT_BACKREF items contain a struct btrfs_root_ref followed
> by the subvolume name. Several readers assume that this layout is already
> valid and then use the on-disk name length directly. A corrupted item can
> therefore make those readers address bytes outside the item, and
> BTRFS_IOC_GET_SUBVOL_INFO can copy too many bytes into its fixed-size UAPI
> name buffer.
> 
> Validate ROOT_REF and ROOT_BACKREF items in tree-checker before any reader
> uses them. Reject records that do not contain a non-empty name, whose
> name_len does not exactly describe the remaining item payload, or whose
> name exceeds BTRFS_NAME_LEN.
> 
> For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len
> instead of deriving the copy length from the item size. The ioctl result is
> zeroed when allocated. That leaves the existing trailing zero byte
> untouched.
> 
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>

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

Will run a full test before merge.

Thanks,
Qu

> ---
>   fs/btrfs/ioctl.c        | 12 ++++++------
>   fs/btrfs/tree-checker.c | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a39460bf6..5958aa8c7 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1956,7 +1956,6 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>   	struct btrfs_root_ref *rref;
>   	struct extent_buffer *leaf;
>   	unsigned long item_off;
> -	unsigned long item_len;
>   	int slot;
>   	int ret = 0;
>   
> @@ -2031,17 +2030,18 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>   		btrfs_item_key_to_cpu(leaf, &key, slot);
>   		if (key.objectid == subvol_info->treeid &&
>   		    key.type == BTRFS_ROOT_BACKREF_KEY) {
> +			u16 name_len;
> +
>   			subvol_info->parent_id = key.offset;
>   
>   			rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
> +			name_len = btrfs_root_ref_name_len(leaf, rref);
>   			subvol_info->dirid = btrfs_root_ref_dirid(leaf, rref);
>   
> -			item_off = btrfs_item_ptr_offset(leaf, slot)
> -					+ sizeof(struct btrfs_root_ref);
> -			item_len = btrfs_item_size(leaf, slot)
> -					- sizeof(struct btrfs_root_ref);
> +			item_off = btrfs_item_ptr_offset(leaf, slot) +
> +				   sizeof(*rref);
>   			read_extent_buffer(leaf, subvol_info->name,
> -					   item_off, item_len);
> +					   item_off, name_len);
>   		} else {
>   			ret = -ENOENT;
>   			goto out;
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 1f15d0793..b48520ff0 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1371,6 +1371,38 @@ static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key,
>   	return 0;
>   }
>   
> +static int check_root_ref(struct extent_buffer *leaf, struct btrfs_key *key,
> +			  int slot)
> +{
> +	struct btrfs_root_ref *rref;
> +	u32 item_size = btrfs_item_size(leaf, slot);
> +	u32 name_len;
> +
> +	if (unlikely(item_size <= sizeof(*rref))) {
> +		generic_err(leaf, slot,
> +			    "invalid root ref item size for key type %u, have %u expect > %zu",
> +			    key->type, item_size, sizeof(*rref));
> +		return -EUCLEAN;
> +	}
> +
> +	rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
> +	name_len = btrfs_root_ref_name_len(leaf, rref);
> +	if (unlikely(name_len > BTRFS_NAME_LEN)) {
> +		generic_err(leaf, slot,
> +			    "root ref name too long for key type %u, have %u max %u",
> +			    key->type, name_len, BTRFS_NAME_LEN);
> +		return -EUCLEAN;
> +	}
> +	if (unlikely(item_size != sizeof(*rref) + name_len)) {
> +		generic_err(leaf, slot,
> +			    "invalid root ref item size for key type %u, have %u expect %zu",
> +			    key->type, item_size, sizeof(*rref) + name_len);
> +		return -EUCLEAN;
> +	}
> +
> +	return 0;
> +}
> +
>   __printf(3,4)
>   __cold
>   static void extent_err(const struct extent_buffer *eb, int slot,
> @@ -2226,6 +2258,10 @@ static enum btrfs_tree_block_status check_leaf_item(struct extent_buffer *leaf,
>   	case BTRFS_ROOT_ITEM_KEY:
>   		ret = check_root_item(leaf, key, slot);
>   		break;
> +	case BTRFS_ROOT_REF_KEY:
> +	case BTRFS_ROOT_BACKREF_KEY:
> +		ret = check_root_ref(leaf, key, slot);
> +		break;
>   	case BTRFS_EXTENT_ITEM_KEY:
>   	case BTRFS_METADATA_ITEM_KEY:
>   		ret = check_extent_item(leaf, key, slot, prev_key);