Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure

[Qu Wenruo <wqu@suse.com>]

在 2026/10/7 19:47, Qu Wenruo 写道:
> 
> 
> 在 2026/10/7 19:36, pavankumaryalagada@gmail.com 写道:
>> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
>>
>> The quota root remains on the transaction's dirty root list when
>> kzalloc_obj() fails and btrfs_quota_enable() releases it without
>> aborting the transaction. Later add_root_to_dirty_list() then accesses
>> the freed dirty_list, causing a slab-use-after-free.
>>
>> Abort the transaction on allocation failure to clean up the dirty
>> root before releasing the quota root.
>>
>> Reported-by: syzbot+947286c775f432b073a8@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=947286c775f432b073a8
>> Fixes: 8d54518b5e52 ("btrfs: qgroup: pre-allocate btrfs_qgroup to 
>> reduce GFP_ATOMIC usage")
>> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>

My bad, Sashiko exposed a valid but very rare race that the temporary 
quota_root can be added to fs_info->dirty_cowonly_roots list.

This requires enough subvolumes to make quota_root to be higher than 
level 0 in the first place, which is not common but definitely possible.

Although all readers of fs_info->dirty_cowonly_roots won't be reached 
after the transaction is aborted, there is still a very small window 
that another thread is already holding a trans handler just after the 
transaction is aborted and the quota root is freed.

In that case the other thread may access the already freed quota_root 
through fs_info->dirty_cowonly_roots->next.

I'm afraid we need to call list_del(&quota_root->dirty_list) with proper 
trans_lock hold during error handling.
> 
>> ---
>>   fs/btrfs/qgroup.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index f68b696b4bf7..42be06675c90 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -1204,6 +1204,7 @@ int btrfs_quota_enable(struct btrfs_fs_info 
>> *fs_info,
>>       prealloc = kzalloc_obj(*prealloc, GFP_NOFS);
>>       if (!prealloc) {
>>           ret = -ENOMEM;
>> +        btrfs_abort_transaction(trans, ret);
>>           goto out_free_path;
>>       }
>>       qgroup = add_qgroup_rb(fs_info, prealloc, BTRFS_FS_TREE_OBJECTID);
>