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

[Qu Wenruo]

在 2026/10/7 19:47, Qu Wenruo 写道:
> 在 2026/10/7 19:36, pavankumaryalagada@xxxxxxxxx 写道:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@xxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > 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@xxxxxxxxx>
>
> Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>

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);