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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2448 bytes --]



On 2020/8/11 上午1:02, David Sterba wrote:
> 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.
> Just to add a note, kobject guys are going to restore the NULL-awareness
behavior, so the patch here is just to be extra safe.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]