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: <CAL3q7H40wEFPz19ce+Fv5CkJVviTESearHBzLMFQc2UHZqJqNQ@mail.gmail.com>
Date: Thu, 9 Jan 2025 12:42:09 +0000
From: Filipe Manana <fdmanana@...nel.org>
To: 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-kernel@...r.kernel.org, 
	Filipe Manana <fdmanana@...e.com>, Johannes Thumshirn <johannes.thumshirn@....com>
Subject: Re: [PATCH v2 03/14] btrfs: fix search when deleting a RAID stripe-extent

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.

Thanks.


> +                               path->slots[0]--;
> +               }
>
>                 leaf = path->nodes[0];
>                 slot = path->slots[0];
>
> --
> 2.43.0
>
>