lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8d26b493-6bc4-488c-b0a7-f2d129d94089@gmx.com>
Date: Wed, 28 Aug 2024 19:00:03 +0930
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Fedor Pchelkin <pchelkin@...ras.ru>, David Sterba <dsterba@...e.com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
 Boris Burkov <boris@....io>, linux-btrfs@...r.kernel.org,
 linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org,
 syzbot+81670362c283f3dd889c@...kaller.appspotmail.com, stable@...r.kernel.org
Subject: Re: [PATCH] btrfs: qgroup: add missing extent changeset release



在 2024/8/28 18:54, Qu Wenruo 写道:
>
>
> 在 2024/8/28 00:42, Fedor Pchelkin 写道:
>> The extent changeset may have some additional memory dynamically
>> allocated
>> for ulist in result of clear_record_extent_bits() execution.
>>
>> Release it after the local changeset is no longer needed in
>> BTRFS_QGROUP_MODE_DISABLED case.
>>
>> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>>
>> Reported-by: syzbot+81670362c283f3dd889c@...kaller.appspotmail.com
>> Closes:
>> https://lore.kernel.org/lkml/000000000000aa8c0c060ade165e@google.com
>> Fixes: af0e2aab3b70 ("btrfs: qgroup: flush reservations during quota
>> disable")
>> Cc: stable@...r.kernel.org # 6.10+
>> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
>
> Reviewed-by: Qu Wenruo <wqu@...e.com>
>
> In this particular case, the changeset is really only locally utilized,
> thus should always be released.

My bad, after checking your latest reply to David, I think we can go one
step further.

Just do not pass changeset to clear_record_extent_bits().

A changeset is utilized for two reasons:

- To let the caller know how many bytes are changed
   Just like what we did for the qgroup enabled case.

- Allow the caller to revert its change
   This happens for qgroup_unreserve_range() when we hit an error and
   needs to free what we just reserved.

In this particular case, since qgroup is already disabled, we just want
to clear the extent io tree bits, not really bother how many bytes are
released nor keep the info for reverting.

So just pass NULL and everything should be fine.

Thanks,
Qu
>
> Thanks,
> Qu
>> ---
>>   fs/btrfs/qgroup.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 5d57a285d59b..4f1fa5d427e1 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -4345,9 +4345,10 @@ static int __btrfs_qgroup_release_data(struct
>> btrfs_inode *inode,
>>
>>       if (btrfs_qgroup_mode(inode->root->fs_info) ==
>> BTRFS_QGROUP_MODE_DISABLED) {
>>           extent_changeset_init(&changeset);
>> -        return clear_record_extent_bits(&inode->io_tree, start,
>> -                        start + len - 1,
>> -                        EXTENT_QGROUP_RESERVED, &changeset);
>> +        ret = clear_record_extent_bits(&inode->io_tree, start,
>> +                           start + len - 1,
>> +                           EXTENT_QGROUP_RESERVED, &changeset);
>> +        goto out;
>>       }
>>
>>       /* In release case, we shouldn't have @reserved */
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ