* [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
@ 2025-10-15 7:24 Dewei Meng
2025-10-15 7:59 ` Daniel Vacek
2025-10-15 8:24 ` Qu Wenruo
0 siblings, 2 replies; 8+ messages in thread
From: Dewei Meng @ 2025-10-15 7:24 UTC (permalink / raw)
To: clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef, Dewei Meng
If fs_info->super_copy or fs_info->super_for_commit is NULL in
btrfs_get_tree_subvol(), the btrfs_check_leaked_roots() will get the
btrfs_root list entry using the fs_info->allocated_roots->next
which is NULL.
syzkaller reported the following information:
------------[ cut here ]------------
BUG: unable to handle page fault for address: fffffffffffffbb0
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
Oops: Oops: 0000 [#1] SMP KASAN PTI
CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4 PREEMPT(lazy)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-fallback.h:457 [inline]
RIP: 0010:atomic_read include/linux/atomic/atomic-instrumented.h:33 [inline]
RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-io.c:1230
[...]
Call Trace:
<TASK>
btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
vfs_get_tree+0x98/0x320 fs/super.c:1759
do_new_mount+0x357/0x660 fs/namespace.c:3899
path_mount+0x716/0x19c0 fs/namespace.c:4226
do_mount fs/namespace.c:4239 [inline]
__do_sys_mount fs/namespace.c:4450 [inline]
__se_sys_mount fs/namespace.c:4427 [inline]
__x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7f032eaffa8d
[...]
This should check if the fs_info->allocated_roots->next is NULL before
accessing it.
Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
---
fs/btrfs/disk-io.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0aa7e5d1b05f..76db7f98187a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct btrfs_fs_info *fs_info)
#ifdef CONFIG_BTRFS_DEBUG
struct btrfs_root *root;
+ if (!fs_info->allocated_roots.next)
+ return;
+
while (!list_empty(&fs_info->allocated_roots)) {
char buf[BTRFS_ROOT_NAME_BUF_LEN];
--
2.43.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 7:24 [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots() Dewei Meng
@ 2025-10-15 7:59 ` Daniel Vacek
2025-10-15 8:24 ` Qu Wenruo
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Vacek @ 2025-10-15 7:59 UTC (permalink / raw)
To: Dewei Meng; +Cc: clm, dsterba, linux-btrfs, linux-kernel, josef
On Wed, 15 Oct 2025 at 09:39, Dewei Meng <mengdewei@cqsoftware.com.cn> wrote:
>
> If fs_info->super_copy or fs_info->super_for_commit is NULL in
> btrfs_get_tree_subvol(), the btrfs_check_leaked_roots() will get the
> btrfs_root list entry using the fs_info->allocated_roots->next
> which is NULL.
>
> syzkaller reported the following information:
> ------------[ cut here ]------------
> BUG: unable to handle page fault for address: fffffffffffffbb0
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
> Oops: Oops: 0000 [#1] SMP KASAN PTI
> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4 PREEMPT(lazy)
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
> RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-fallback.h:457 [inline]
> RIP: 0010:atomic_read include/linux/atomic/atomic-instrumented.h:33 [inline]
> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-io.c:1230
> [...]
> Call Trace:
> <TASK>
> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
> vfs_get_tree+0x98/0x320 fs/super.c:1759
> do_new_mount+0x357/0x660 fs/namespace.c:3899
> path_mount+0x716/0x19c0 fs/namespace.c:4226
> do_mount fs/namespace.c:4239 [inline]
> __do_sys_mount fs/namespace.c:4450 [inline]
> __se_sys_mount fs/namespace.c:4427 [inline]
> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7f032eaffa8d
> [...]
>
> This should check if the fs_info->allocated_roots->next is NULL before
> accessing it.
>
> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
> ---
> fs/btrfs/disk-io.c | 3 +++
> 1 file changed, 3 insertions(+)
Looks good to me.
Reviewed-by: Daniel Vacek <neelx@suse.com>
Thank you
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 0aa7e5d1b05f..76db7f98187a 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct btrfs_fs_info *fs_info)
> #ifdef CONFIG_BTRFS_DEBUG
> struct btrfs_root *root;
>
> + if (!fs_info->allocated_roots.next)
> + return;
> +
> while (!list_empty(&fs_info->allocated_roots)) {
> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>
> --
> 2.43.5
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 7:24 [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots() Dewei Meng
2025-10-15 7:59 ` Daniel Vacek
@ 2025-10-15 8:24 ` Qu Wenruo
2025-10-15 8:26 ` Qu Wenruo
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Qu Wenruo @ 2025-10-15 8:24 UTC (permalink / raw)
To: Dewei Meng, clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef
在 2025/10/15 17:54, Dewei Meng 写道:
> If fs_info->super_copy or fs_info->super_for_commit is NULL in
> btrfs_get_tree_subvol(),
Please reorganize this sentence. It would be way more easier to read by
just saying something like "If memory allocation failed for
fs_info->super_copy or fs_info->super_for_commit in
btrfs_get_tree_subvol()".
> the btrfs_check_leaked_roots() will get the
> btrfs_root list entry using the fs_info->allocated_roots->next
> which is NULL.
>
> syzkaller reported the following information:
> ------------[ cut here ]------------
> BUG: unable to handle page fault for address: fffffffffffffbb0
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
> Oops: Oops: 0000 [#1] SMP KASAN PTI
> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4 PREEMPT(lazy)
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
> RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-fallback.h:457 [inline]
> RIP: 0010:atomic_read include/linux/atomic/atomic-instrumented.h:33 [inline]
> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-io.c:1230
> [...]
> Call Trace:
> <TASK>
> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
> vfs_get_tree+0x98/0x320 fs/super.c:1759
> do_new_mount+0x357/0x660 fs/namespace.c:3899
> path_mount+0x716/0x19c0 fs/namespace.c:4226
> do_mount fs/namespace.c:4239 [inline]
> __do_sys_mount fs/namespace.c:4450 [inline]
> __se_sys_mount fs/namespace.c:4427 [inline]
> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7f032eaffa8d
> [...]
>
> This should check if the fs_info->allocated_roots->next is NULL before
> accessing it.
>
> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
> ---
> fs/btrfs/disk-io.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 0aa7e5d1b05f..76db7f98187a 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct btrfs_fs_info *fs_info)
> #ifdef CONFIG_BTRFS_DEBUG
> struct btrfs_root *root;
>
> + if (!fs_info->allocated_roots.next)
> + return;
> +
The check looks too adhoc to me.
It would be much easier to just call kvfree() in the error handling of
super_copy/super_for_commit allocation, we do not and should not call
btrfs_free_fs_info() before calling btrfs_init_fs_info().
Thanks,
Qu
> while (!list_empty(&fs_info->allocated_roots)) {
> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 8:24 ` Qu Wenruo
@ 2025-10-15 8:26 ` Qu Wenruo
2025-10-15 8:41 ` Daniel Vacek
2025-10-15 10:02 ` Dewei Meng
2 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2025-10-15 8:26 UTC (permalink / raw)
To: Dewei Meng, clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef
在 2025/10/15 18:54, Qu Wenruo 写道:
>
>
> 在 2025/10/15 17:54, Dewei Meng 写道:
>> If fs_info->super_copy or fs_info->super_for_commit is NULL in
>> btrfs_get_tree_subvol(),
>
> Please reorganize this sentence. It would be way more easier to read by
> just saying something like "If memory allocation failed for fs_info-
> >super_copy or fs_info->super_for_commit in btrfs_get_tree_subvol()".
>
>> the btrfs_check_leaked_roots() will get the
>> btrfs_root list entry using the fs_info->allocated_roots->next
>> which is NULL.
>>
>> syzkaller reported the following information:
>> ------------[ cut here ]------------
>> BUG: unable to handle page fault for address: fffffffffffffbb0
>> #PF: supervisor read access in kernel mode
>> #PF: error_code(0x0000) - not-present page
>> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4
>> PREEMPT(lazy)
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
>> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
>> RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-
>> fallback.h:457 [inline]
>> RIP: 0010:atomic_read include/linux/atomic/atomic-instrumented.h:33
>> [inline]
>> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
>> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-io.c:1230
>> [...]
>> Call Trace:
>> <TASK>
>> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
>> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
>> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
>> vfs_get_tree+0x98/0x320 fs/super.c:1759
>> do_new_mount+0x357/0x660 fs/namespace.c:3899
>> path_mount+0x716/0x19c0 fs/namespace.c:4226
>> do_mount fs/namespace.c:4239 [inline]
>> __do_sys_mount fs/namespace.c:4450 [inline]
>> __se_sys_mount fs/namespace.c:4427 [inline]
>> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> RIP: 0033:0x7f032eaffa8d
>> [...]
>>
>> This should check if the fs_info->allocated_roots->next is NULL before
>> accessing it.
>>
>> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
>> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
>> ---
>> fs/btrfs/disk-io.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 0aa7e5d1b05f..76db7f98187a 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct
>> btrfs_fs_info *fs_info)
>> #ifdef CONFIG_BTRFS_DEBUG
>> struct btrfs_root *root;
>> + if (!fs_info->allocated_roots.next)
>> + return;
>> +
>
> The check looks too adhoc to me.
>
> It would be much easier to just call kvfree()
Of course with kfree() on the super_copy/super_for_commit before
kvfree() on the fs_info.
> in the error handling of
> super_copy/super_for_commit allocation, we do not and should not call
> btrfs_free_fs_info() before calling btrfs_init_fs_info().
>
> Thanks,
> Qu
>> while (!list_empty(&fs_info->allocated_roots)) {
>> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 8:24 ` Qu Wenruo
2025-10-15 8:26 ` Qu Wenruo
@ 2025-10-15 8:41 ` Daniel Vacek
2025-10-15 10:02 ` Dewei Meng
2 siblings, 0 replies; 8+ messages in thread
From: Daniel Vacek @ 2025-10-15 8:41 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Dewei Meng, clm, dsterba, linux-btrfs, linux-kernel, josef
On Wed, 15 Oct 2025 at 10:24, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> 在 2025/10/15 17:54, Dewei Meng 写道:
> > If fs_info->super_copy or fs_info->super_for_commit is NULL in
> > btrfs_get_tree_subvol(),
>
> Please reorganize this sentence. It would be way more easier to read by
> just saying something like "If memory allocation failed for
> fs_info->super_copy or fs_info->super_for_commit in
> btrfs_get_tree_subvol()".
>
> > the btrfs_check_leaked_roots() will get the
> > btrfs_root list entry using the fs_info->allocated_roots->next
> > which is NULL.
> >
> > syzkaller reported the following information:
> > ------------[ cut here ]------------
> > BUG: unable to handle page fault for address: fffffffffffffbb0
> > #PF: supervisor read access in kernel mode
> > #PF: error_code(0x0000) - not-present page
> > PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
> > Oops: Oops: 0000 [#1] SMP KASAN PTI
> > CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4 PREEMPT(lazy)
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
> > RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
> > RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-fallback.h:457 [inline]
> > RIP: 0010:atomic_read include/linux/atomic/atomic-instrumented.h:33 [inline]
> > RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
> > RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-io.c:1230
> > [...]
> > Call Trace:
> > <TASK>
> > btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
> > btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
> > btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
> > vfs_get_tree+0x98/0x320 fs/super.c:1759
> > do_new_mount+0x357/0x660 fs/namespace.c:3899
> > path_mount+0x716/0x19c0 fs/namespace.c:4226
> > do_mount fs/namespace.c:4239 [inline]
> > __do_sys_mount fs/namespace.c:4450 [inline]
> > __se_sys_mount fs/namespace.c:4427 [inline]
> > __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
> > do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> > do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
> > entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x7f032eaffa8d
> > [...]
> >
> > This should check if the fs_info->allocated_roots->next is NULL before
> > accessing it.
> >
> > Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
> > Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
> > ---
> > fs/btrfs/disk-io.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 0aa7e5d1b05f..76db7f98187a 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct btrfs_fs_info *fs_info)
> > #ifdef CONFIG_BTRFS_DEBUG
> > struct btrfs_root *root;
> >
> > + if (!fs_info->allocated_roots.next)
> > + return;
> > +
>
> The check looks too adhoc to me.
>
> It would be much easier to just call kvfree() in the error handling of
> super_copy/super_for_commit allocation, we do not and should not call
> btrfs_free_fs_info() before calling btrfs_init_fs_info().
Right. I like this solution better too.
--nX
> Thanks,
> Qu
> > while (!list_empty(&fs_info->allocated_roots)) {
> > char buf[BTRFS_ROOT_NAME_BUF_LEN];
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 8:24 ` Qu Wenruo
2025-10-15 8:26 ` Qu Wenruo
2025-10-15 8:41 ` Daniel Vacek
@ 2025-10-15 10:02 ` Dewei Meng
2025-10-15 10:07 ` Qu Wenruo
2 siblings, 1 reply; 8+ messages in thread
From: Dewei Meng @ 2025-10-15 10:02 UTC (permalink / raw)
To: Qu Wenruo, clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef
在 2025/10/15 16:24, Qu Wenruo 写道:
>
>
> 在 2025/10/15 17:54, Dewei Meng 写道:
>> If fs_info->super_copy or fs_info->super_for_commit is NULL in
>> btrfs_get_tree_subvol(),
>
> Please reorganize this sentence. It would be way more easier to read
> by just saying something like "If memory allocation failed for
> fs_info->super_copy or fs_info->super_for_commit in
> btrfs_get_tree_subvol()".
I agree, I will fix these words to make them easier to read.
>
>> the btrfs_check_leaked_roots() will get the
>> btrfs_root list entry using the fs_info->allocated_roots->next
>> which is NULL.
>>
>> syzkaller reported the following information:
>> ------------[ cut here ]------------
>> BUG: unable to handle page fault for address: fffffffffffffbb0
>> #PF: supervisor read access in kernel mode
>> #PF: error_code(0x0000) - not-present page
>> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4
>> PREEMPT(lazy)
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
>> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
>> RIP: 0010:raw_atomic_read
>> include/linux/atomic/atomic-arch-fallback.h:457 [inline]
>> RIP: 0010:atomic_read
>> include/linux/atomic/atomic-instrumented.h:33 [inline]
>> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
>> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0
>> fs/btrfs/disk-io.c:1230
>> [...]
>> Call Trace:
>> <TASK>
>> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
>> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
>> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
>> vfs_get_tree+0x98/0x320 fs/super.c:1759
>> do_new_mount+0x357/0x660 fs/namespace.c:3899
>> path_mount+0x716/0x19c0 fs/namespace.c:4226
>> do_mount fs/namespace.c:4239 [inline]
>> __do_sys_mount fs/namespace.c:4450 [inline]
>> __se_sys_mount fs/namespace.c:4427 [inline]
>> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> RIP: 0033:0x7f032eaffa8d
>> [...]
>>
>> This should check if the fs_info->allocated_roots->next is NULL before
>> accessing it.
>>
>> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
>> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
>> ---
>> fs/btrfs/disk-io.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 0aa7e5d1b05f..76db7f98187a 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct
>> btrfs_fs_info *fs_info)
>> #ifdef CONFIG_BTRFS_DEBUG
>> struct btrfs_root *root;
>> + if (!fs_info->allocated_roots.next)
>> + return;
>> +
>
> The check looks too adhoc to me.
>
> It would be much easier to just call kvfree() in the error handling of
> super_copy/super_for_commit allocation, we do not and should not call
> btrfs_free_fs_info() before calling btrfs_init_fs_info().
It is a good solution to fix this bug, or can we put the
'btrfs_init_fs_info(fs_info)' before super_copy/super_for_commit allocation?
Thanks,
Dewei Meng
>
> Thanks,
> Qu
>> while (!list_empty(&fs_info->allocated_roots)) {
>> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 10:02 ` Dewei Meng
@ 2025-10-15 10:07 ` Qu Wenruo
2025-10-16 2:11 ` Dewei Meng
0 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2025-10-15 10:07 UTC (permalink / raw)
To: Dewei Meng, clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef
在 2025/10/15 20:32, Dewei Meng 写道:
>
> 在 2025/10/15 16:24, Qu Wenruo 写道:
>>
>>
>> 在 2025/10/15 17:54, Dewei Meng 写道:
>>> If fs_info->super_copy or fs_info->super_for_commit is NULL in
>>> btrfs_get_tree_subvol(),
>>
>> Please reorganize this sentence. It would be way more easier to read
>> by just saying something like "If memory allocation failed for
>> fs_info->super_copy or fs_info->super_for_commit in
>> btrfs_get_tree_subvol()".
> I agree, I will fix these words to make them easier to read.
>>
>>> the btrfs_check_leaked_roots() will get the
>>> btrfs_root list entry using the fs_info->allocated_roots->next
>>> which is NULL.
>>>
>>> syzkaller reported the following information:
>>> ------------[ cut here ]------------
>>> BUG: unable to handle page fault for address: fffffffffffffbb0
>>> #PF: supervisor read access in kernel mode
>>> #PF: error_code(0x0000) - not-present page
>>> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
>>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>>> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4
>>> PREEMPT(lazy)
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
>>> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23 [inline]
>>> RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-
>>> fallback.h:457 [inline]
>>> RIP: 0010:atomic_read include/linux/atomic/atomic-
>>> instrumented.h:33 [inline]
>>> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
>>> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-
>>> io.c:1230
>>> [...]
>>> Call Trace:
>>> <TASK>
>>> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
>>> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
>>> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
>>> vfs_get_tree+0x98/0x320 fs/super.c:1759
>>> do_new_mount+0x357/0x660 fs/namespace.c:3899
>>> path_mount+0x716/0x19c0 fs/namespace.c:4226
>>> do_mount fs/namespace.c:4239 [inline]
>>> __do_sys_mount fs/namespace.c:4450 [inline]
>>> __se_sys_mount fs/namespace.c:4427 [inline]
>>> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
>>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>>> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
>>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>> RIP: 0033:0x7f032eaffa8d
>>> [...]
>>>
>>> This should check if the fs_info->allocated_roots->next is NULL before
>>> accessing it.
>>>
>>> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
>>> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
>>> ---
>>> fs/btrfs/disk-io.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index 0aa7e5d1b05f..76db7f98187a 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct
>>> btrfs_fs_info *fs_info)
>>> #ifdef CONFIG_BTRFS_DEBUG
>>> struct btrfs_root *root;
>>> + if (!fs_info->allocated_roots.next)
>>> + return;
>>> +
>>
>> The check looks too adhoc to me.
>>
>> It would be much easier to just call kvfree() in the error handling of
>> super_copy/super_for_commit allocation, we do not and should not call
>> btrfs_free_fs_info() before calling btrfs_init_fs_info().
>
> It is a good solution to fix this bug, or can we put the
> 'btrfs_init_fs_info(fs_info)' before super_copy/super_for_commit
> allocation?
That also sounds fine to me.
>
> Thanks,
>
> Dewei Meng
>
>>
>> Thanks,
>> Qu
>>> while (!list_empty(&fs_info->allocated_roots)) {
>>> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>>
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()
2025-10-15 10:07 ` Qu Wenruo
@ 2025-10-16 2:11 ` Dewei Meng
0 siblings, 0 replies; 8+ messages in thread
From: Dewei Meng @ 2025-10-16 2:11 UTC (permalink / raw)
To: Qu Wenruo, clm, dsterba; +Cc: linux-btrfs, linux-kernel, josef
在 2025/10/15 18:07, Qu Wenruo 写道:
>
>
> 在 2025/10/15 20:32, Dewei Meng 写道:
>>
>> 在 2025/10/15 16:24, Qu Wenruo 写道:
>>>
>>>
>>> 在 2025/10/15 17:54, Dewei Meng 写道:
>>>> If fs_info->super_copy or fs_info->super_for_commit is NULL in
>>>> btrfs_get_tree_subvol(),
>>>
>>> Please reorganize this sentence. It would be way more easier to read
>>> by just saying something like "If memory allocation failed for
>>> fs_info->super_copy or fs_info->super_for_commit in
>>> btrfs_get_tree_subvol()".
>> I agree, I will fix these words to make them easier to read.
>>>
>>>> the btrfs_check_leaked_roots() will get the
>>>> btrfs_root list entry using the fs_info->allocated_roots->next
>>>> which is NULL.
>>>>
>>>> syzkaller reported the following information:
>>>> ------------[ cut here ]------------
>>>> BUG: unable to handle page fault for address: fffffffffffffbb0
>>>> #PF: supervisor read access in kernel mode
>>>> #PF: error_code(0x0000) - not-present page
>>>> PGD 64c9067 P4D 64c9067 PUD 64cb067 PMD 0
>>>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>>>> CPU: 0 UID: 0 PID: 1402 Comm: syz.1.35 Not tainted 6.15.8 #4
>>>> PREEMPT(lazy)
>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), (...)
>>>> RIP: 0010:arch_atomic_read arch/x86/include/asm/atomic.h:23
>>>> [inline]
>>>> RIP: 0010:raw_atomic_read include/linux/atomic/atomic-arch-
>>>> fallback.h:457 [inline]
>>>> RIP: 0010:atomic_read include/linux/atomic/atomic-
>>>> instrumented.h:33 [inline]
>>>> RIP: 0010:refcount_read include/linux/refcount.h:170 [inline]
>>>> RIP: 0010:btrfs_check_leaked_roots+0x18f/0x2c0 fs/btrfs/disk-
>>>> io.c:1230
>>>> [...]
>>>> Call Trace:
>>>> <TASK>
>>>> btrfs_free_fs_info+0x310/0x410 fs/btrfs/disk-io.c:1280
>>>> btrfs_get_tree_subvol+0x592/0x6b0 fs/btrfs/super.c:2029
>>>> btrfs_get_tree+0x63/0x80 fs/btrfs/super.c:2097
>>>> vfs_get_tree+0x98/0x320 fs/super.c:1759
>>>> do_new_mount+0x357/0x660 fs/namespace.c:3899
>>>> path_mount+0x716/0x19c0 fs/namespace.c:4226
>>>> do_mount fs/namespace.c:4239 [inline]
>>>> __do_sys_mount fs/namespace.c:4450 [inline]
>>>> __se_sys_mount fs/namespace.c:4427 [inline]
>>>> __x64_sys_mount+0x28c/0x310 fs/namespace.c:4427
>>>> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>>>> do_syscall_64+0x92/0x180 arch/x86/entry/syscall_64.c:94
>>>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>> RIP: 0033:0x7f032eaffa8d
>>>> [...]
>>>>
>>>> This should check if the fs_info->allocated_roots->next is NULL before
>>>> accessing it.
>>>>
>>>> Fixes: 3bb17a25bcb0 ("btrfs: add get_tree callback for new mount API")
>>>> Signed-off-by: Dewei Meng <mengdewei@cqsoftware.com.cn>
>>>> ---
>>>> fs/btrfs/disk-io.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>> index 0aa7e5d1b05f..76db7f98187a 100644
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -1213,6 +1213,9 @@ void btrfs_check_leaked_roots(const struct
>>>> btrfs_fs_info *fs_info)
>>>> #ifdef CONFIG_BTRFS_DEBUG
>>>> struct btrfs_root *root;
>>>> + if (!fs_info->allocated_roots.next)
>>>> + return;
>>>> +
>>>
>>> The check looks too adhoc to me.
>>>
>>> It would be much easier to just call kvfree() in the error handling
>>> of super_copy/super_for_commit allocation, we do not and should not
>>> call btrfs_free_fs_info() before calling btrfs_init_fs_info().
>>
>> It is a good solution to fix this bug, or can we put the
>> 'btrfs_init_fs_info(fs_info)' before super_copy/super_for_commit
>> allocation?
>
> That also sounds fine to me.
I have tested the kfree method, and test case runs fine. I plan to take
your suggestion and re-do the patch.
Thanks.
>
>>
>> Thanks,
>>
>> Dewei Meng
>>
>>>
>>> Thanks,
>>> Qu
>>>> while (!list_empty(&fs_info->allocated_roots)) {
>>>> char buf[BTRFS_ROOT_NAME_BUF_LEN];
>>>
>>>
>>>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-16 2:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-15 7:24 [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots() Dewei Meng
2025-10-15 7:59 ` Daniel Vacek
2025-10-15 8:24 ` Qu Wenruo
2025-10-15 8:26 ` Qu Wenruo
2025-10-15 8:41 ` Daniel Vacek
2025-10-15 10:02 ` Dewei Meng
2025-10-15 10:07 ` Qu Wenruo
2025-10-16 2:11 ` Dewei Meng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®