linux-kernel - Re: [PATCH] bcachefs: fix size used to calculate `nr` of devices in member_to

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] [day] [month] [year] [list]

Message-ID: <4n6rpfi4vi4gsrk3oiodqhjtjzaygkgg3yamsmn76trrw4o3zv@xxndklof3j6r>
Date: Thu, 12 Jun 2025 11:27:32 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Sergio Perez Gonzalez <sperezglz@...il.com>
Cc: linux-bcachefs@...r.kernel.org, linux-kernel@...r.kernel.org, 
	shuah@...nel.org, syzbot+e577022d4fba380653be@...kaller.appspotmail.com
Subject: Re: [PATCH] bcachefs: fix size used to calculate `nr` of devices in
 member_to_text()

On Thu, Jun 12, 2025 at 02:19:26AM -0600, Sergio Perez Gonzalez wrote:
> In bch2_sb_members_v1_to_text() there is an incorrect size used to
> calculate the number of devices.
> 
> The correct size should be `BCH_MEMBER_V1_BYTES` as implied by:
> 
> static struct bch_member *members_v1_get_mut(struct bch_sb_field_members_v1 *mi, int i)
> {
> 	return (void *) mi->_members + (i * BCH_MEMBER_V1_BYTES);
> }
> 
> Also invert the pointers used in the calculation, given that they yield a
> negative number.
> 
> Reported-by: syzbot+e577022d4fba380653be@...kaller.appspotmail.com
> Signed-off-by: Sergio Perez Gonzalez <sperezglz@...il.com>
> ---
>  fs/bcachefs/sb-members.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/bcachefs/sb-members.c b/fs/bcachefs/sb-members.c
> index c673e76ca27f..cc51313ebcf6 100644
> --- a/fs/bcachefs/sb-members.c
> +++ b/fs/bcachefs/sb-members.c
> @@ -325,10 +325,10 @@ static void bch2_sb_members_v1_to_text(struct printbuf *out, struct bch_sb *sb,
>  {
>  	struct bch_sb_field_members_v1 *mi = field_to_type(f, members_v1);
>  	struct bch_sb_field_disk_groups *gi = bch2_sb_field_get(sb, disk_groups);
> -	int nr = (vstruct_end(&mi->field) - (void *) &gi->entries[0]) / sizeof(gi->entries[0]);
> +	int nr = ((void *) &gi->entries[0] - vstruct_end(&mi->field)) / BCH_MEMBER_V1_BYTES;
>  

Actually, this shouldn't be referencing gi at all - brainfart on my end,
that's a completely different superblock section. That's where the
negative number came from.

>  	if (nr != sb->nr_devices)
> -		prt_printf(out, "nr_devices mismatch: have %i entries, should be %u", nr, sb->nr_devices);
> +		prt_printf(out, "nr_devices mismatch: have %i entries, should be %u\n", nr, sb->nr_devices);

Yeah, it should have a newline.

I'll fold that in.

>  
>  	for (int i = 0; i < nr; i++)
>  		member_to_text(out, members_v1_get(mi, i), gi, sb, i);
> @@ -344,10 +344,10 @@ static void bch2_sb_members_v2_to_text(struct printbuf *out, struct bch_sb *sb,
>  {
>  	struct bch_sb_field_members_v2 *mi = field_to_type(f, members_v2);
>  	struct bch_sb_field_disk_groups *gi = bch2_sb_field_get(sb, disk_groups);
> -	int nr = (vstruct_end(&mi->field) - (void *) &gi->entries[0]) / le16_to_cpu(mi->member_bytes);
> +	int nr = ((void *) &gi->entries[0] - vstruct_end(&mi->field)) / le16_to_cpu(mi->member_bytes);
>  
>  	if (nr != sb->nr_devices)
> -		prt_printf(out, "nr_devices mismatch: have %i entries, should be %u", nr, sb->nr_devices);
> +		prt_printf(out, "nr_devices mismatch: have %i entries, should be %u\n", nr, sb->nr_devices);
>  
>  	/*
>  	 * We call to_text() on superblock sections that haven't passed
> -- 
> 2.43.0
>