[PATCH -next] btrfs: fix use-after-free in btrfs_get_global_root

From: Ye Bin
Date: Mon Aug 22 2022 - 21:48:10 EST


Syzkaller reported UAF as follows:
==================================================================
BUG: KASAN: use-after-free in btrfs_get_global_root+0x663/0xa10
Read of size 4 at addr ffff88811ddbb3c0 by task kworker/u16:1/11

CPU: 4 PID: 11 Comm: kworker/u16:1 Not tainted 6.0.0-rc1-next-20220822+ #2
Workqueue: btrfs-qgroup-rescan btrfs_work_helper
Call Trace:
<TASK>
dump_stack_lvl+0x6e/0x91
print_report.cold+0xb2/0x6bb
kasan_report+0xa8/0x130
kasan_check_range+0x13f/0x1d0
btrfs_get_global_root+0x663/0xa10
btrfs_get_fs_root_commit_root+0xa5/0x150
find_parent_nodes+0x92f/0x2990
btrfs_find_all_roots_safe+0x12d/0x220
btrfs_find_all_roots+0xbb/0xd0
btrfs_qgroup_rescan_worker+0x600/0xc30
btrfs_work_helper+0xff/0x750
process_one_work+0x52c/0x930
worker_thread+0x352/0x8c0
kthread+0x1b9/0x200
ret_from_fork+0x22/0x30
</TASK>

Allocated by task 1895:
kasan_save_stack+0x1e/0x40
__kasan_kmalloc+0xa9/0xe0
btrfs_alloc_root+0x40/0x820
btrfs_create_tree+0xf8/0x500
btrfs_quota_enable+0x30a/0x1120
btrfs_ioctl+0x50a3/0x59f0
__x64_sys_ioctl+0x130/0x170
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd

Freed by task 1895:
kasan_save_stack+0x1e/0x40
kasan_set_track+0x21/0x30
kasan_set_free_info+0x20/0x40
__kasan_slab_free+0x127/0x1c0
kfree+0xa8/0x2d0
btrfs_put_root+0x1ca/0x230
btrfs_quota_enable+0x87c/0x1120
btrfs_ioctl+0x50a3/0x59f0
__x64_sys_ioctl+0x130/0x170
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
==================================================================

Above issue may happens as follows:
p1 p2
btrfs_quota_enable
spin_lock(&fs_info->qgroup_lock);
fs_info->quota_root = quota_root;
spin_unlock(&fs_info->qgroup_lock);

ret = qgroup_rescan_init -> return error
if (ret)
btrfs_put_root(quota_root);
kfree(root);

if (ret) {
ulist_free(fs_info->qgroup_ulist);
fs_info->qgroup_ulist = NULL;
btrfs_sysfs_del_qgroups(fs_info);
} btrfs_qgroup_rescan_worker
btrfs_find_all_roots
btrfs_find_all_roots_safe
find_parent_nodes
btrfs_get_fs_root_commit_root
btrfs_grab_root(fs_info->quota_root)
-> quota_root already freed

Syzkaller also reported another issue:
==================================================================
BUG: KASAN: use-after-free in ulist_release+0x30/0xb3
Read of size 8 at addr ffff88811413d048 by task rep/2921

CPU: 3 PID: 2921 Comm: rep Not tainted 6.0.0-rc1-next-20220822+ #3
rep[2921] cmdline: ./rep
Call Trace:
<TASK>
dump_stack_lvl+0x6e/0x91
print_report.cold+0xb2/0x6bb
kasan_report+0xa8/0x130
ulist_release+0x30/0xb3
ulist_reinit+0x16/0x56
btrfs_qgroup_free_refroot+0x288/0x3f0
btrfs_qgroup_free_meta_all_pertrans+0xed/0x1e0
commit_fs_roots+0x28c/0x430
btrfs_commit_transaction+0x9a6/0x1b40
btrfs_qgroup_rescan+0x7e/0x130
btrfs_ioctl+0x48ed/0x59f0
__x64_sys_ioctl+0x130/0x170
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
</TASK>

Allocated by task 2900:
kasan_save_stack+0x1e/0x40
__kasan_kmalloc+0xa9/0xe0
ulist_alloc+0x5c/0xe0
btrfs_quota_enable+0x1b2/0x1160
btrfs_ioctl+0x50a3/0x59f0
__x64_sys_ioctl+0x130/0x170
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd

Freed by task 2900:
kasan_save_stack+0x1e/0x40
kasan_set_track+0x21/0x30
kasan_set_free_info+0x20/0x40
__kasan_slab_free+0x127/0x1c0
kfree+0xa8/0x2d0
ulist_free.cold+0x15/0x1a
btrfs_quota_enable+0x8bf/0x1160
btrfs_ioctl+0x50a3/0x59f0
__x64_sys_ioctl+0x130/0x170
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
==================================================================

To solve above issues just set 'fs_info->quota_root' after qgroup_rescan_init
return success.

Signed-off-by: Ye Bin <yebin10@xxxxxxxxxx>
---
fs/btrfs/qgroup.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index db723c0026bd..16f0b038295a 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1158,18 +1158,18 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info)
if (ret)
goto out_free_path;

- /*
- * Set quota enabled flag after committing the transaction, to avoid
- * deadlocks on fs_info->qgroup_ioctl_lock with concurrent snapshot
- * creation.
- */
- spin_lock(&fs_info->qgroup_lock);
- fs_info->quota_root = quota_root;
- set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
- spin_unlock(&fs_info->qgroup_lock);
-
ret = qgroup_rescan_init(fs_info, 0, 1);
if (!ret) {
+ /*
+ * Set quota enabled flag after committing the transaction, to
+ * avoid deadlocks on fs_info->qgroup_ioctl_lock with concurrent
+ * snapshot creation.
+ */
+ spin_lock(&fs_info->qgroup_lock);
+ fs_info->quota_root = quota_root;
+ set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
+ spin_unlock(&fs_info->qgroup_lock);
+
qgroup_rescan_zero_tracking(fs_info);
fs_info->qgroup_rescan_running = true;
btrfs_queue_work(fs_info->qgroup_rescan_workers,
--
2.31.1