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@xxxxxxxxxxxxxxx
> Signed-off-by: Fedor Pchelkin <pchelkin@xxxxxxxxx>
> ---
> 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
>
>
