On Mon, Aug 26, 2019 at 02:49:09PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> In xfs_bmbt_diff_two_keys, we perform a signed int64_t subtraction with
> two unsigned 64-bit quantities. If the second quantity is actually the
> "maximum" key (all ones) as used in _query_all, the subtraction
> effectively becomes addition of two positive numbers and the function
> returns incorrect results. Fix this with explicit comparisons of the
> unsigned values. Nobody needs this now, but the online repair patches
> will need this to work properly.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap_btree.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index fbb18ba5d905..3c1a805b3775 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -400,8 +400,20 @@ xfs_bmbt_diff_two_keys(
> union xfs_btree_key *k1,
> union xfs_btree_key *k2)
> {
> - return (int64_t)be64_to_cpu(k1->bmbt.br_startoff) -
> - be64_to_cpu(k2->bmbt.br_startoff);
> + uint64_t a = be64_to_cpu(k1->bmbt.br_startoff);
> + uint64_t b = be64_to_cpu(k2->bmbt.br_startoff);
> +
> + /*
> + * Note: This routine previously casted a and b to int64 and subtracted
> + * them to generate a result. This lead to problems if b was the
> + * "maximum" key value (all ones) being signed incorrectly, hence this
> + * somewhat less efficient version.
> + */
> + if (a > b)
> + return 1;
> + else if (b > a)
> + return -1;
No need for an else here, but otherwise OK.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
Dave Chinner
david@xxxxxxxxxxxxx
