Re: [PATCH] btrfs: Fix NULL pointer access in btrfs_check_leaked_roots()

[Qu Wenruo]

在 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@xxxxxxxxxxxxxxxxx>
> > ---
> >   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];