On Tue, Aug 22, 2023 at 06:00:46PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Dave Chinner reported that xfs/273 fails if the AG size happens to be an
> exact power of two. I traced this to an agbno integer overflow when the
> current GETFSMAP call is a continuation of a previous GETFSMAP call, and
> the last record returned was non-shareable space at the end of an AG.
>
> __xfs_getfsmap_datadev sets up a data device query by converting the
> incoming fmr_physical into an xfs_fsblock_t and cracking it into an agno
> and agbno pair. In the (failing) case of where fmr_blockcount of the
> low key is nonzero and the record was for a non-shareable extent, it
> will add fmr_blockcount to start_fsb and info->low.rm_startblock.
>
> If the low key was actually the last record for that AG, then this
> addition causes info->low.rm_startblock to point beyond EOAG. When the
> rmapbt range query starts, it'll return an empty set, and fsmap moves on
> to the next AG.
>
> Or so I thought. Remember how we added to start_fsb?
>
> If agsize < 1<<agblklog, start_fsb points to the same AG as the original
> fmr_physical from the low key. We run the rmapbt query, which returns
> nothing, so getfsmap zeroes info->low and moves on to the next AG.
>
> If agsize == 1<<agblklog, start_fsb now points to the next AG. We run
> the rmapbt query on the next AG with the excessively large
> rm_startblock. If this next AG is actually the last AG, we'll set
> info->high to EOFS (which is now has a lower rm_startblock than
> info->low), and the ranged btree query code will return -EINVAL. If
> it's not the last AG, we ignore all records for the intermediate AGs.
>
> Oops.
>
> Fix this by decoding start_fsb into agno and agbno only after making
> adjustments to start_fsb. This means that info->low.rm_startblock will
> always be set to a valid agbno, and we always start the rmapbt iteration
> in the correct AG.
>
> While we're at it, fix the predicate for determining if an fsmap record
> represents non-shareable space to include file data on pre-reflink
> filesystems.
>
> Reported-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Fixes: 63ef7a35912dd ("xfs: fix interval filtering in multi-step fsmap queries")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Fixes the regression, code looks fine.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
Dave Chinner
david@xxxxxxxxxxxxx
