linux-kernel - Re: [PATCH v2 03/14] btrfs: fix search when deleting a RAID stripe-extent

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: <de8da1ce-1299-4feb-b855-6e991b852026@wdc.com>
Date: Thu, 9 Jan 2025 14:13:16 +0000
From: Johannes Thumshirn <Johannes.Thumshirn@....com>
To: Filipe Manana <fdmanana@...nel.org>, Johannes Thumshirn <jth@...nel.org>
CC: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>, David Sterba
	<dsterba@...e.com>, "linux-btrfs@...r.kernel.org"
	<linux-btrfs@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Filipe Manana <fdmanana@...e.com>
Subject: Re: [PATCH v2 03/14] btrfs: fix search when deleting a RAID
 stripe-extent

On 09.01.25 13:42, Filipe Manana wrote:
> On Tue, Jan 7, 2025 at 12:48 PM Johannes Thumshirn <jth@...nel.org> wrote:
>>
>> From: Johannes Thumshirn <johannes.thumshirn@....com>
>>
>> Only pick the previous slot, when btrfs_search_slot() returned '1'.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@....com>
>> ---
>>   fs/btrfs/raid-stripe-tree.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c
>> index 5c6224ed3eda53a11a41bffdf6c789fbd6d3a503..0c4d218a99d4aaea5da6c39624e20e77758a89d3 100644
>> --- a/fs/btrfs/raid-stripe-tree.c
>> +++ b/fs/btrfs/raid-stripe-tree.c
>> @@ -89,8 +89,12 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start, u64 le
>>                  if (ret < 0)
>>                          break;
>>
>> -               if (path->slots[0] == btrfs_header_nritems(path->nodes[0]))
>> -                       path->slots[0]--;
>> +               if (ret == 1) {
>> +                       ret = 0;
>> +                       if (path->slots[0] ==
>> +                           btrfs_header_nritems(path->nodes[0]))
> 
> Btw this can fit in a single line, it stays at 83 characters which is
> acceptable nowadays, making things a bit more readable.
> I've commented on this many times before in other patches.
> 
> Can you explain what bug is this patch fixing?
> The changelog doesn't provide any information about that.
> 
> path->slots[0] should only match btrfs_header_nritems(path->nodes[0])
> when the key wasn't found, that is, when ret == 1.
> So I don't see what this patch is trying to fix or improve.
> 
> Also, the 'ret = 0' is pointless, as we do it shortly after this code.
> 

You're right, that patch is BS. I'll drop it.