Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info
@ 2026-05-10  7:49 Zhang Cen
  2026-05-10  8:18 ` Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Zhang Cen @ 2026-05-10  7:49 UTC (permalink / raw)
  To: Chris Mason, David Sterba
  Cc: linux-btrfs, zerocling0077, 2045gemini, Zhang Cen

btrfs_ioctl_get_subvol_info() derives the copied subvolume name
length from btrfs_item_size() and copies it into subvol_info->name,
which is only BTRFS_VOL_NAME_MAX + 1 bytes long. A malformed
ROOT_BACKREF can therefore make the length underflow or exceed the
fixed ioctl buffer.

Validate that the item is large enough for struct btrfs_root_ref,
that the on-disk name_len fits inside the remaining payload, and that
the copied name does not exceed BTRFS_VOL_NAME_MAX. If any of those
checks fail, return -EUCLEAN instead of copying from corrupted
metadata. After validation, copy only name_len bytes and terminate the
result explicitly.

Sanitizer validation reported:
BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
Write of size 505 at addr ffff88810936d608
Call trace:
  dump_stack_lvl() (?:?)
  print_address_description() (mm/kasan/report.c:373)
  read_extent_buffer() (?:?)
  print_report() (?:?)
  __virt_addr_valid() (?:?)
  srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
  kasan_addr_to_slab() (mm/kasan/common.c:45)
  kasan_report() (?:?)
  kasan_check_range() (?:?)
  __asan_memcpy() (mm/kasan/shadow.c:103)
  btrfs_ioctl_get_subvol_info() (fs/btrfs/ioctl.c:2034)
  btrfs_get_32() (fs/btrfs/ioctl.c:?)
  btrfs_set_16() (fs/btrfs/ioctl.c:?)
  btrfs_test_get_subvol_info_name_oob() (fs/btrfs/ioctl.c:?)
  btrfs_run_sanity_tests() (fs/btrfs/ioctl.c:?)
  init_btrfs_fs() (fs/btrfs/super.c:2690)
  do_one_initcall() (init/main.c:1382)
  __kasan_kmalloc() (?:?)
  rcu_is_watching() (?:?)
  do_initcalls() (init/main.c:1457)
  kernel_init_freeable() (init/main.c:1674)
  kernel_init() (init/main.c:1584)
  ret_from_fork() (?:?)
  __switch_to() (?:?)
  ret_from_fork_asm() (?:?)
  kasan_save_stack() (mm/kasan/common.c:52)
  kasan_save_track() (mm/kasan/common.c:74)

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>

---
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a39460bf68a7..0647e672464a 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1956,7 +1956,8 @@ 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;
+	u32 item_size;
+	u16 name_len;
 	int slot;
 	int ret = 0;
 
@@ -2034,14 +2035,26 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 			subvol_info->parent_id = key.offset;
 
 			rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
+			item_size = btrfs_item_size(leaf, slot);
+			if (item_size < sizeof(*rref)) {
+				ret = -EUCLEAN;
+				goto out;
+			}
+
+			name_len = btrfs_root_ref_name_len(leaf, rref);
+			if (name_len > item_size - sizeof(*rref) ||
+			    name_len > BTRFS_VOL_NAME_MAX) {
+				ret = -EUCLEAN;
+				goto out;
+			}
+
 			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);
+			subvol_info->name[name_len] = '\0';
 		} else {
 			ret = -ENOENT;
 			goto out;

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

* Re: [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info
  2026-05-10  7:49 [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info Zhang Cen
@ 2026-05-10  8:18 ` Qu Wenruo
       [not found] ` <qu-root-backref-20260510-161800@local>
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2026-05-10  8:18 UTC (permalink / raw)
  To: Zhang Cen, Chris Mason, David Sterba
  Cc: linux-btrfs, zerocling0077, 2045gemini



在 2026/5/10 17:19, Zhang Cen 写道:
> btrfs_ioctl_get_subvol_info() derives the copied subvolume name
> length from btrfs_item_size() and copies it into subvol_info->name,
> which is only BTRFS_VOL_NAME_MAX + 1 bytes long. A malformed
> ROOT_BACKREF can therefore make the length underflow or exceed the
> fixed ioctl buffer.
> 
> Validate that the item is large enough for struct btrfs_root_ref,
> that the on-disk name_len fits inside the remaining payload, and that
> the copied name does not exceed BTRFS_VOL_NAME_MAX. If any of those
> checks fail, return -EUCLEAN instead of copying from corrupted
> metadata. After validation, copy only name_len bytes and terminate the
> result explicitly.

The validation is too late.

btrfs subvolume creation only accept BTRFS_SUBVOL_NAME_MAX + 1 (for the 
terminating \0).

This means a subvolume ref/backref should not have a name longer than 
BTRFS_SUBVOL_NAME_MAX in the first place.

Such independent checks should all be done inside tree-checker.

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

* Re: [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info
       [not found] ` <qu-root-backref-20260510-161800@local>
@ 2026-05-10 14:37   ` Zhang Cen
  0 siblings, 0 replies; 7+ messages in thread
From: Zhang Cen @ 2026-05-10 14:37 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: Chris Mason, David Sterba, linux-btrfs, zerocling0077, 2045gemini,
	Zhang Cen

On Sun, May 10, 2026 at 04:18:00PM +0800, Qu Wenruo wrote:
> The validation is too late.
> 
> btrfs subvolume creation only accept BTRFS_SUBVOL_NAME_MAX + 1 (for the
> terminating \0).
> 
> This means a subvolume ref/backref should not have a name longer than
> BTRFS_SUBVOL_NAME_MAX in the first place.
> 
> Such independent checks should all be done inside tree-checker.

Agreed, thanks for the review.

I'll move the on-disk ROOT_REF/ROOT_BACKREF validation into tree-checker
for v2 and cover both key types there. The v2 change will reject items
smaller than struct btrfs_root_ref, reject records where name_len does not
match the item payload, and reject names longer than BTRFS_SUBVOL_NAME_MAX.

I'll keep the BTRFS_IOC_GET_SUBVOL_INFO-side BTRFS_VOL_NAME_MAX check only
as the local guard for its fixed-size UAPI output buffer before copying the
already validated name.

Thanks,
Zhang

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

* [PATCH v2] btrfs: validate root ref names in tree-checker
  2026-05-10  7:49 [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info Zhang Cen
  2026-05-10  8:18 ` Qu Wenruo
       [not found] ` <qu-root-backref-20260510-161800@local>
@ 2026-05-10 14:42 ` Zhang Cen
  2026-05-10 14:46   ` Cen Zhang
  2026-05-10 15:03 ` [PATCH v3] " Zhang Cen
  3 siblings, 1 reply; 7+ messages in thread
From: Zhang Cen @ 2026-05-10 14:42 UTC (permalink / raw)
  To: Chris Mason, David Sterba
  Cc: linux-btrfs, Qu Wenruo, zerocling0077, 2045gemini, 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 items smaller than struct btrfs_root_ref, reject records
whose name_len does not exactly describe the remaining item payload, and
reject names longer than BTRFS_SUBVOL_NAME_MAX.

For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
Terminate the copied name explicitly.

Sanitizer validation reported:
BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
Write of size 505 at addr ffff88810936d608
Call trace:
  dump_stack_lvl() (?:?)
  print_address_description() (mm/kasan/report.c:373)
  read_extent_buffer() (?:?)
  print_report() (?:?)
  __virt_addr_valid() (?:?)
  srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
  kasan_addr_to_slab() (mm/kasan/common.c:45)
  kasan_report() (?:?)
  kasan_check_range() (?:?)
  __asan_memcpy() (mm/kasan/shadow.c:103)
  btrfs_ioctl_get_subvol_info() (fs/btrfs/ioctl.c:1948)
  btrfs_get_32() (?:?)
  btrfs_set_16() (?:?)
  btrfs_test_get_subvol_info_name_oob() (?:?)
  btrfs_run_sanity_tests() (?:?)
  init_btrfs_fs() (fs/btrfs/super.c:2690)
  do_one_initcall() (init/main.c:1382)
  __kasan_kmalloc() (?:?)
  rcu_is_watching() (?:?)
  do_initcalls() (init/main.c:1457)
  kernel_init_freeable() (init/main.c:1674)
  kernel_init() (init/main.c:1584)
  ret_from_fork() (?:?)
  __switch_to() (?:?)
  ret_from_fork_asm() (?:?)
  kasan_save_stack() (mm/kasan/common.c:52)
  kasan_save_track() (mm/kasan/common.c:74)

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>

---
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
In-Reply-To: <20260510074943.2644335-1-rollkingzzc@gmail.com>
References: <20260510074943.2644335-1-rollkingzzc@gmail.com> <qu-root-backref-20260510-161800@local>
From: Zhang Cen <rollkingzzc@gmail.com>
Date: Sun, 10 May 2026 17:05:00 +0800
Subject: [PATCH v2] btrfs: validate root ref names in tree-checker
To: Chris Mason <clm@fb.com>,
    David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org,
    Qu Wenruo <wqu@suse.com>,
    zerocling0077@gmail.com,
    2045gemini@gmail.com,
    Zhang Cen <rollkingzzc@gmail.com>

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 items smaller than struct btrfs_root_ref, reject records
whose name_len does not exactly describe the remaining item payload, and
reject names longer than BTRFS_SUBVOL_NAME_MAX.

For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
Terminate the copied name explicitly.

Sanitizer validation reported:
BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
Write of size 505 at addr ffff88810936d608
Call trace:
  read_extent_buffer()
  btrfs_ioctl_get_subvol_info()

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
---
Changes since v1:
- Move ROOT_REF/ROOT_BACKREF structural validation into tree-checker.
- Validate both forward and backward root refs instead of only the ioctl path.
- Keep the ioctl-side BTRFS_VOL_NAME_MAX check as a destination-buffer guard.

 fs/btrfs/ioctl.c       | 17 +++++++++++++----
 fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)
diff --git aa/fs/btrfs/ioctl.c bb/fs/btrfs/ioctl.c
index a39460bf68a7..b43d33eaea07 100644
--- aa/fs/btrfs/ioctl.c
+++ bb/fs/btrfs/ioctl.c
@@ -1956,7 +1956,8 @@ 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;
+	u32 item_size;
+	u16 name_len;
 	int slot;
 	int ret = 0;
 
@@ -2034,14 +2035,21 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 			subvol_info->parent_id = key.offset;
 
 			rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
+			item_size = btrfs_item_size(leaf, slot);
+			name_len = btrfs_root_ref_name_len(leaf, rref);
+			if (name_len > item_size - sizeof(*rref) ||
+			    name_len > BTRFS_VOL_NAME_MAX) {
+				ret = -EUCLEAN;
+				goto out;
+			}
+
 			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);
+			subvol_info->name[name_len] = '\0';
 		} else {
 			ret = -ENOENT;
 			goto out;
diff --git aa/fs/btrfs/tree-checker.c bb/fs/btrfs/tree-checker.c
index 1f15d0793a9c..41417b2df32d 100644
--- aa/fs/btrfs/tree-checker.c
+++ bb/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_SUBVOL_NAME_MAX)) {
+		generic_err(leaf, slot,
+			    "root ref name too long for key type %u, have %u max %u",
+			    key->type, name_len, BTRFS_SUBVOL_NAME_MAX);
+		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);
-- 
2.43.0

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

* Re: [PATCH v2] btrfs: validate root ref names in tree-checker
  2026-05-10 14:42 ` [PATCH v2] btrfs: validate root ref names in tree-checker Zhang Cen
@ 2026-05-10 14:46   ` Cen Zhang
  0 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang @ 2026-05-10 14:46 UTC (permalink / raw)
  To: Chris Mason, David Sterba
  Cc: linux-btrfs, Qu Wenruo, zerocling0077, 2045gemini

I'm so sorry, I sent the wrong message.

Best regards,
Zhang Cen

Zhang Cen <rollkingzzc@gmail.com> 于2026年5月10日周日 22:42写道:
>
> 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 items smaller than struct btrfs_root_ref, reject records
> whose name_len does not exactly describe the remaining item payload, and
> reject names longer than BTRFS_SUBVOL_NAME_MAX.
>
> For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
> keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
> Terminate the copied name explicitly.
>
> Sanitizer validation reported:
> BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
> Write of size 505 at addr ffff88810936d608
> Call trace:
>   dump_stack_lvl() (?:?)
>   print_address_description() (mm/kasan/report.c:373)
>   read_extent_buffer() (?:?)
>   print_report() (?:?)
>   __virt_addr_valid() (?:?)
>   srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
>   kasan_addr_to_slab() (mm/kasan/common.c:45)
>   kasan_report() (?:?)
>   kasan_check_range() (?:?)
>   __asan_memcpy() (mm/kasan/shadow.c:103)
>   btrfs_ioctl_get_subvol_info() (fs/btrfs/ioctl.c:1948)
>   btrfs_get_32() (?:?)
>   btrfs_set_16() (?:?)
>   btrfs_test_get_subvol_info_name_oob() (?:?)
>   btrfs_run_sanity_tests() (?:?)
>   init_btrfs_fs() (fs/btrfs/super.c:2690)
>   do_one_initcall() (init/main.c:1382)
>   __kasan_kmalloc() (?:?)
>   rcu_is_watching() (?:?)
>   do_initcalls() (init/main.c:1457)
>   kernel_init_freeable() (init/main.c:1674)
>   kernel_init() (init/main.c:1584)
>   ret_from_fork() (?:?)
>   __switch_to() (?:?)
>   ret_from_fork_asm() (?:?)
>   kasan_save_stack() (mm/kasan/common.c:52)
>   kasan_save_track() (mm/kasan/common.c:74)
>
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
>
> ---
> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> In-Reply-To: <20260510074943.2644335-1-rollkingzzc@gmail.com>
> References: <20260510074943.2644335-1-rollkingzzc@gmail.com> <qu-root-backref-20260510-161800@local>
> From: Zhang Cen <rollkingzzc@gmail.com>
> Date: Sun, 10 May 2026 17:05:00 +0800
> Subject: [PATCH v2] btrfs: validate root ref names in tree-checker
> To: Chris Mason <clm@fb.com>,
>     David Sterba <dsterba@suse.com>
> Cc: linux-btrfs@vger.kernel.org,
>     Qu Wenruo <wqu@suse.com>,
>     zerocling0077@gmail.com,
>     2045gemini@gmail.com,
>     Zhang Cen <rollkingzzc@gmail.com>
>
> 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 items smaller than struct btrfs_root_ref, reject records
> whose name_len does not exactly describe the remaining item payload, and
> reject names longer than BTRFS_SUBVOL_NAME_MAX.
>
> For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
> keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
> Terminate the copied name explicitly.
>
> Sanitizer validation reported:
> BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
> Write of size 505 at addr ffff88810936d608
> Call trace:
>   read_extent_buffer()
>   btrfs_ioctl_get_subvol_info()
>
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
> ---
> Changes since v1:
> - Move ROOT_REF/ROOT_BACKREF structural validation into tree-checker.
> - Validate both forward and backward root refs instead of only the ioctl path.
> - Keep the ioctl-side BTRFS_VOL_NAME_MAX check as a destination-buffer guard.
>
>  fs/btrfs/ioctl.c       | 17 +++++++++++++----
>  fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 4 deletions(-)
> diff --git aa/fs/btrfs/ioctl.c bb/fs/btrfs/ioctl.c
> index a39460bf68a7..b43d33eaea07 100644
> --- aa/fs/btrfs/ioctl.c
> +++ bb/fs/btrfs/ioctl.c
> @@ -1956,7 +1956,8 @@ 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;
> +       u32 item_size;
> +       u16 name_len;
>         int slot;
>         int ret = 0;
>
> @@ -2034,14 +2035,21 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>                         subvol_info->parent_id = key.offset;
>
>                         rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
> +                       item_size = btrfs_item_size(leaf, slot);
> +                       name_len = btrfs_root_ref_name_len(leaf, rref);
> +                       if (name_len > item_size - sizeof(*rref) ||
> +                           name_len > BTRFS_VOL_NAME_MAX) {
> +                               ret = -EUCLEAN;
> +                               goto out;
> +                       }
> +
>                         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);
> +                       subvol_info->name[name_len] = '\0';
>                 } else {
>                         ret = -ENOENT;
>                         goto out;
> diff --git aa/fs/btrfs/tree-checker.c bb/fs/btrfs/tree-checker.c
> index 1f15d0793a9c..41417b2df32d 100644
> --- aa/fs/btrfs/tree-checker.c
> +++ bb/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_SUBVOL_NAME_MAX)) {
> +               generic_err(leaf, slot,
> +                           "root ref name too long for key type %u, have %u max %u",
> +                           key->type, name_len, BTRFS_SUBVOL_NAME_MAX);
> +               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);
> --
> 2.43.0

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

* [PATCH v3] btrfs: validate root ref names in tree-checker
  2026-05-10  7:49 [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info Zhang Cen
                   ` (2 preceding siblings ...)
  2026-05-10 14:42 ` [PATCH v2] btrfs: validate root ref names in tree-checker Zhang Cen
@ 2026-05-10 15:03 ` Zhang Cen
  2026-05-10 22:12   ` Qu Wenruo
  3 siblings, 1 reply; 7+ messages in thread
From: Zhang Cen @ 2026-05-10 15:03 UTC (permalink / raw)
  To: Zhang Cen, Chris Mason, David Sterba
  Cc: linux-btrfs, Qu Wenruo, zerocling0077, 2045gemini

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 items smaller than struct btrfs_root_ref, reject records
whose name_len does not exactly describe the remaining item payload, and
reject names longer than BTRFS_SUBVOL_NAME_MAX.

For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
Terminate the copied name explicitly.

Sanitizer validation reported:
BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
Write of size 505 at addr ffff88810936d608
Call trace:
  read_extent_buffer()
  btrfs_ioctl_get_subvol_info()

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
---
Changes since v2:
- Regenerate the mail-ready patch without the nested mbox wrapper.
- No code changes beyond the v2 fix.

 fs/btrfs/ioctl.c       | 17 +++++++++++++----
 fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)
diff --git aa/fs/btrfs/ioctl.c bb/fs/btrfs/ioctl.c
index a39460bf68a7..b43d33eaea07 100644
--- aa/fs/btrfs/ioctl.c
+++ bb/fs/btrfs/ioctl.c
@@ -1956,7 +1956,8 @@ 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;
+	u32 item_size;
+	u16 name_len;
 	int slot;
 	int ret = 0;
 
@@ -2034,14 +2035,21 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 			subvol_info->parent_id = key.offset;
 
 			rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
+			item_size = btrfs_item_size(leaf, slot);
+			name_len = btrfs_root_ref_name_len(leaf, rref);
+			if (name_len > item_size - sizeof(*rref) ||
+			    name_len > BTRFS_VOL_NAME_MAX) {
+				ret = -EUCLEAN;
+				goto out;
+			}
+
 			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);
+			subvol_info->name[name_len] = '\0';
 		} else {
 			ret = -ENOENT;
 			goto out;
diff --git aa/fs/btrfs/tree-checker.c bb/fs/btrfs/tree-checker.c
index 1f15d0793a9c..41417b2df32d 100644
--- aa/fs/btrfs/tree-checker.c
+++ bb/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_SUBVOL_NAME_MAX)) {
+		generic_err(leaf, slot,
+			    "root ref name too long for key type %u, have %u max %u",
+			    key->type, name_len, BTRFS_SUBVOL_NAME_MAX);
+		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);
-- 
2.43.0

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

* Re: [PATCH v3] btrfs: validate root ref names in tree-checker
  2026-05-10 15:03 ` [PATCH v3] " Zhang Cen
@ 2026-05-10 22:12   ` Qu Wenruo
  0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2026-05-10 22:12 UTC (permalink / raw)
  To: Zhang Cen, Chris Mason, David Sterba
  Cc: linux-btrfs, zerocling0077, 2045gemini



在 2026/5/11 00:33, 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 items smaller than struct btrfs_root_ref, reject records
> whose name_len does not exactly describe the remaining item payload, and
> reject names longer than BTRFS_SUBVOL_NAME_MAX.
> 
> For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len and
> keep a local BTRFS_VOL_NAME_MAX guard for the fixed ioctl output field.
> Terminate the copied name explicitly.
> 
> Sanitizer validation reported:
> BUG: KASAN: slab-out-of-bounds in read_extent_buffer()
> Write of size 505 at addr ffff88810936d608
> Call trace:
>    read_extent_buffer()
>    btrfs_ioctl_get_subvol_info()
> 
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
> ---
> Changes since v2:
> - Regenerate the mail-ready patch without the nested mbox wrapper.
> - No code changes beyond the v2 fix.
> 
>   fs/btrfs/ioctl.c       | 17 +++++++++++++----
>   fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 52 insertions(+), 4 deletions(-)
> diff --git aa/fs/btrfs/ioctl.c bb/fs/btrfs/ioctl.c
> index a39460bf68a7..b43d33eaea07 100644
> --- aa/fs/btrfs/ioctl.c
> +++ bb/fs/btrfs/ioctl.c
> @@ -1956,7 +1956,8 @@ 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;
> +	u32 item_size;

Move the declaration where it's really used, aka, the if () branch where 
it's first utilized.

> +	u16 name_len;
>   	int slot;
>   	int ret = 0;
>   
> @@ -2034,14 +2035,21 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>   			subvol_info->parent_id = key.offset;
>   
>   			rref = btrfs_item_ptr(leaf, slot, struct btrfs_root_ref);
> +			item_size = btrfs_item_size(leaf, slot);
> +			name_len = btrfs_root_ref_name_len(leaf, rref);
> +			if (name_len > item_size - sizeof(*rref) ||
> +			    name_len > BTRFS_VOL_NAME_MAX) {

Those conditions are already checked in tree-checker.
And you return -EUCLEAN without any error message.

An ASSERT() will be more than enough, and I even prefer not to have an 
ASSERT().

And that's the idea of tree-checker, we validate everything (that we can 
validate) in tree-checker, so that callers won't need to bother 
re-checking them at random location again and again.

> +				ret = -EUCLEAN;
> +				goto out;
> +			}
> +
>   			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);
> +			subvol_info->name[name_len] = '\0';

No need to manually add terminating \0.

subvol_info is allocated by kzalloc_obj(), all members in 
subvol_info->name[] should be zero, and the BTRFS_VOL_NAME_MAX check 
ensured there is at least one byte left in name[] that is not touched.

>   		} else {
>   			ret = -ENOENT;
>   			goto out;
> diff --git aa/fs/btrfs/tree-checker.c bb/fs/btrfs/tree-checker.c
> index 1f15d0793a9c..41417b2df32d 100644
> --- aa/fs/btrfs/tree-checker.c
> +++ bb/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))) {

There should be a name_len that is not zero.
So the check should be (item_size <= sizeof(*ref)).

Thanks,
Qu

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

end of thread, other threads:[~2026-05-10 22:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10  7:49 [PATCH] btrfs: validate ROOT_BACKREF name before copying subvolume info Zhang Cen
2026-05-10  8:18 ` Qu Wenruo
     [not found] ` <qu-root-backref-20260510-161800@local>
2026-05-10 14:37   ` Zhang Cen
2026-05-10 14:42 ` [PATCH v2] btrfs: validate root ref names in tree-checker Zhang Cen
2026-05-10 14:46   ` Cen Zhang
2026-05-10 15:03 ` [PATCH v3] " Zhang Cen
2026-05-10 22:12   ` Qu Wenruo

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