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

[Qu Wenruo]

在 2026/10/7 22:25, Yalagada Pavan Kumar 写道:
> On Wed, Oct 07, 2026 at 08:29:14PM +1030, Qu Wenruo wrote:
> > 在 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.
>
> Thanks, Qu. I understand.
>
> I'll update the error handling to remove quota_root->dirty_list under trans_lock
> before dropping the quota root reference, based on the latest for-next. I'll send v2.

I'm exploring the idea to put such list_del() into btrfs_put_root(), 
which should handle it more gracefully and cover all other situations 
where a new root is created and dirtied.

Thanks,
Qu

> Thanks,
> Pavan
>
> > > > ---
> > > >    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);