mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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®