Re: [PATCH] btrfs: sysfs: fix NULL pointer dereference at btrfs_sysfs_del_qgroups()

[David Sterba <dsterba@suse.cz>]

On Mon, Aug 03, 2020 at 02:20:11PM +0800, Qu Wenruo wrote:
> [BUG]
> With next-20200731 tag (079ad2fb4bf9eba8a0aaab014b49705cd7f07c66),

I don't think linux-next commit ids are useful, even the tags get
removed after a month so the exact commit causing the crash is what we
want.

> unmounting a btrfs with quota disabled will cause the following NULL
> pointer dereference:
> 
>   BTRFS info (device dm-5): has skinny extents
>   BUG: kernel NULL pointer dereference, address: 0000000000000018
>   #PF: supervisor read access in kernel mode
>   #PF: error_code(0x0000) - not-present page
>   CPU: 7 PID: 637 Comm: umount Not tainted 5.8.0-rc7-next-20200731-custom #76
>   RIP: 0010:kobject_del+0x6/0x20
>   Call Trace:
>    btrfs_sysfs_del_qgroups+0xac/0xf0 [btrfs]
>    btrfs_free_qgroup_config+0x63/0x70 [btrfs]
>    close_ctree+0x1f5/0x323 [btrfs]
>    btrfs_put_super+0x15/0x17 [btrfs]
>    generic_shutdown_super+0x72/0x110
>    kill_anon_super+0x18/0x30
>    btrfs_kill_super+0x17/0x30 [btrfs]
>    deactivate_locked_super+0x3b/0xa0
>    deactivate_super+0x40/0x50
>    cleanup_mnt+0x135/0x190
>    __cleanup_mnt+0x12/0x20
>    task_work_run+0x64/0xb0
>    exit_to_user_mode_prepare+0x18a/0x190
>    syscall_exit_to_user_mode+0x4f/0x270
>    do_syscall_64+0x45/0x50
>    entry_SYSCALL_64_after_hwframe+0x44/0xa9
>   ---[ end trace 37b7adca5c1d5c5d ]---
> 
> [CAUSE]
> Commit 079ad2fb4bf9 ("kobject: Avoid premature parent object freeing in
> kobject_cleanup()") changed kobject_del() that it no longer accepts NULL
> pointer.

That commit reference should be sufficient.

> Before that commit, kobject_del() and kobject_put() all accept NULL
> pointers and just ignore such NULL pointers.
> 
> But that mentioned commit needs to access the parent node, killing the
> old NULL pointer behavior.
> 
> Unfortunately btrfs is relying on that hidden feature thus we will
> trigger such NULL pointer dereference.
> 
> [FIX]
> Instead of just saving several lines, do proper fs_info->qgroups_kobj
> check before calling kobject_del() and kobject_put().
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to misc-next, thanks.