mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] btrfs: fix use-after-free on quota enable allocation failure
@ 2026-10-07  9:06 pavankumaryalagada
  2026-10-07  9:17 ` Qu Wenruo
  0 siblings, 1 reply; 6+ messages in thread
From: pavankumaryalagada @ 2026-10-07  9:06 UTC (permalink / raw)
  To: dsterba
  Cc: mason, wqu, fdmanana, shuah, linux-btrfs, linux-kernel,
	Yalagada Pavan Kumar, syzbot+947286c775f432b073a8

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-07  9:06 [PATCH] btrfs: fix use-after-free on quota enable allocation failure pavankumaryalagada
@ 2026-10-07  9:17 ` Qu Wenruo
  2026-10-07  9:22   ` Qu Wenruo
  2026-10-07  9:59   ` Qu Wenruo
  0 siblings, 2 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-07  9:17 UTC (permalink / raw)
  To: pavankumaryalagada, dsterba
  Cc: mason, fdmanana, shuah, linux-btrfs, linux-kernel,
	syzbot+947286c775f432b073a8



在 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>

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-07  9:17 ` Qu Wenruo
@ 2026-10-07  9:22   ` Qu Wenruo
  2026-10-07  9:59   ` Qu Wenruo
  1 sibling, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-07  9:22 UTC (permalink / raw)
  To: pavankumaryalagada, dsterba
  Cc: mason, fdmanana, shuah, linux-btrfs, linux-kernel,
	syzbot+947286c775f432b073a8



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

BTW, you are on an older branch.

The latest for-next branch has the commit "btrfs: qgroup: merge error 
labels in btrfs_quota_enable()", which changed the branch.

Next time check the MAINTAINERS file to grab the latest devel branch.

>>       }
>>       qgroup = add_qgroup_rb(fs_info, prealloc, BTRFS_FS_TREE_OBJECTID);
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-07  9:17 ` Qu Wenruo
  2026-10-07  9:22   ` Qu Wenruo
@ 2026-10-07  9:59   ` Qu Wenruo
  2026-10-07 11:55     ` Yalagada Pavan Kumar
  1 sibling, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2026-10-07  9:59 UTC (permalink / raw)
  To: pavankumaryalagada, dsterba
  Cc: mason, fdmanana, shuah, linux-btrfs, linux-kernel,
	syzbot+947286c775f432b073a8



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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-07  9:59   ` Qu Wenruo
@ 2026-10-07 11:55     ` Yalagada Pavan Kumar
  2026-10-07 20:58       ` Qu Wenruo
  0 siblings, 1 reply; 6+ messages in thread
From: Yalagada Pavan Kumar @ 2026-10-07 11:55 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: dsterba, mason, fdmanana, shuah, linux-btrfs, linux-kernel,
	syzbot+947286c775f432b073a8

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@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.

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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-07 11:55     ` Yalagada Pavan Kumar
@ 2026-10-07 20:58       ` Qu Wenruo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-07 20:58 UTC (permalink / raw)
  To: Yalagada Pavan Kumar, Qu Wenruo
  Cc: dsterba, mason, fdmanana, shuah, linux-btrfs, linux-kernel,
	syzbot+947286c775f432b073a8



在 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@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.
> 
> 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);
>>>
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-07 20:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07  9:06 [PATCH] btrfs: fix use-after-free on quota enable allocation failure pavankumaryalagada
2026-10-07  9:17 ` Qu Wenruo
2026-10-07  9:22   ` Qu Wenruo
2026-10-07  9:59   ` Qu Wenruo
2026-10-07 11:55     ` Yalagada Pavan Kumar
2026-10-07 20:58       ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®