linux-kernel - Re: [PATCH] xfs: fix diff_two_keys calculation for cnt btree

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: <20250426150359.GQ25675@frogsfrogsfrogs>
Date: Sat, 26 Apr 2025 08:03:59 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Carlos Maiolino <cem@...nel.org>,
	Chandan Babu R <chandanbabu@...nel.org>,
	Brian Foster <bfoster@...hat.com>, linux-xfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org,
	Alexey Nepomnyashih <sdl@...ct.ru>, stable@...r.kernel.org
Subject: Re: [PATCH] xfs: fix diff_two_keys calculation for cnt btree

On Sat, Apr 26, 2025 at 04:42:31PM +0300, Fedor Pchelkin wrote:
> Currently the difference is computed on 32-bit unsigned values although
> eventually it is stored in a variable of int64_t type. This gives awkward
> results, e.g. when the diff _should_ be negative, it is represented as
> some large positive int64_t value.
> 
> Perform the calculations directly in int64_t as all other diff_two_keys
> routines actually do.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace static
> analysis tool.
> 
> Fixes: 08438b1e386b ("xfs: plumb in needed functions for range querying of the freespace btrees")
> Cc: stable@...r.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
> ---
>  fs/xfs/libxfs/xfs_alloc_btree.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
> index a4ac37ba5d51..b3c54ae90e25 100644
> --- a/fs/xfs/libxfs/xfs_alloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_alloc_btree.c
> @@ -238,13 +238,13 @@ xfs_cntbt_diff_two_keys(
>  	ASSERT(!mask || (mask->alloc.ar_blockcount &&
>  			 mask->alloc.ar_startblock));
>  
> -	diff =  be32_to_cpu(k1->alloc.ar_blockcount) -
> -		be32_to_cpu(k2->alloc.ar_blockcount);
> +	diff = (int64_t)be32_to_cpu(k1->alloc.ar_blockcount) -
> +			be32_to_cpu(k2->alloc.ar_blockcount);

Perhaps it's time to hoist cmp_int to include/ and refactor all these
things to use it?

#define cmp_int(l, r)          ((l > r) - (l < r))

--D

>  	if (diff)
>  		return diff;
>  
> -	return  be32_to_cpu(k1->alloc.ar_startblock) -
> -		be32_to_cpu(k2->alloc.ar_startblock);
> +	return (int64_t)be32_to_cpu(k1->alloc.ar_startblock) -
> +			be32_to_cpu(k2->alloc.ar_startblock);
>  }
>  
>  static xfs_failaddr_t
> -- 
> 2.49.0
> 
>