* [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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread[parent not found: <qu-root-backref-20260510-161800@local>]
* 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* [PATCH v3] btrfs: validate root ref names in tree-checker
@ 2026-05-10 15:28 Zhang Cen
0 siblings, 0 replies; 8+ messages in thread
From: Zhang Cen @ 2026-05-10 15:28 UTC (permalink / raw)
To: 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.
Changes since v2:
- Regenerate the mail-ready patch without the nested mbox wrapper.
- No code changes beyond the v2 fix.
Sanitizer validation reported:
KASAN slab-out-of-bounds in read_extent_buffer()
Write of size 505
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>
---
fs/btrfs/ioctl.c | 17 +++++++++++++----
fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a39460bf68a7..b43d33eaea07 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,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 a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 1f15d0793a9c..41417b2df32d 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_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] 8+ messages in thread
end of thread, other threads:[~2026-05-10 22:13 UTC | newest]
Thread overview: 8+ 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
-- strict thread matches above, loose matches on Subject: below --
2026-05-10 15:28 Zhang Cen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox