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

[Fedor Pchelkin]

On Sat, 26. Apr 08:03, Darrick J. Wong wrote:
> 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

No need to apply this one as a fix (e.g. for further inclusion into stable
releases) ?

I'll send out the refactoring patches for review when the cmp_int()-moving
one hits Linus' tree - it's currently in mm-nonmm-unstable branch of
akpm/mm.git and in linux-next repo. Though I'm not aware of how xfs trees
interact with linux-next.