Commit cfa2df68 authored by Darrick J. Wong's avatar Darrick J. Wong

xfs: fix an agbno overflow in __xfs_getfsmap_datadev

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: default avatarDave Chinner <david@fromorbit.com>
Fixes: 63ef7a35 ("xfs: fix interval filtering in multi-step fsmap queries")
Signed-off-by: default avatarDarrick J. Wong <djwong@kernel.org>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
parent 0bb80ecc
...@@ -565,6 +565,19 @@ xfs_getfsmap_rtdev_rtbitmap( ...@@ -565,6 +565,19 @@ xfs_getfsmap_rtdev_rtbitmap(
} }
#endif /* CONFIG_XFS_RT */ #endif /* CONFIG_XFS_RT */
static inline bool
rmap_not_shareable(struct xfs_mount *mp, const struct xfs_rmap_irec *r)
{
if (!xfs_has_reflink(mp))
return true;
if (XFS_RMAP_NON_INODE_OWNER(r->rm_owner))
return true;
if (r->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
XFS_RMAP_UNWRITTEN))
return true;
return false;
}
/* Execute a getfsmap query against the regular data device. */ /* Execute a getfsmap query against the regular data device. */
STATIC int STATIC int
__xfs_getfsmap_datadev( __xfs_getfsmap_datadev(
...@@ -598,7 +611,6 @@ __xfs_getfsmap_datadev( ...@@ -598,7 +611,6 @@ __xfs_getfsmap_datadev(
* low to the fsmap low key and max out the high key to the end * low to the fsmap low key and max out the high key to the end
* of the AG. * of the AG.
*/ */
info->low.rm_startblock = XFS_FSB_TO_AGBNO(mp, start_fsb);
info->low.rm_offset = XFS_BB_TO_FSBT(mp, keys[0].fmr_offset); info->low.rm_offset = XFS_BB_TO_FSBT(mp, keys[0].fmr_offset);
error = xfs_fsmap_owner_to_rmap(&info->low, &keys[0]); error = xfs_fsmap_owner_to_rmap(&info->low, &keys[0]);
if (error) if (error)
...@@ -608,12 +620,9 @@ __xfs_getfsmap_datadev( ...@@ -608,12 +620,9 @@ __xfs_getfsmap_datadev(
/* Adjust the low key if we are continuing from where we left off. */ /* Adjust the low key if we are continuing from where we left off. */
if (info->low.rm_blockcount == 0) { if (info->low.rm_blockcount == 0) {
/* empty */ /* No previous record from which to continue */
} else if (XFS_RMAP_NON_INODE_OWNER(info->low.rm_owner) || } else if (rmap_not_shareable(mp, &info->low)) {
(info->low.rm_flags & (XFS_RMAP_ATTR_FORK | /* Last record seen was an unshareable extent */
XFS_RMAP_BMBT_BLOCK |
XFS_RMAP_UNWRITTEN))) {
info->low.rm_startblock += info->low.rm_blockcount;
info->low.rm_owner = 0; info->low.rm_owner = 0;
info->low.rm_offset = 0; info->low.rm_offset = 0;
...@@ -621,8 +630,10 @@ __xfs_getfsmap_datadev( ...@@ -621,8 +630,10 @@ __xfs_getfsmap_datadev(
if (XFS_FSB_TO_DADDR(mp, start_fsb) >= eofs) if (XFS_FSB_TO_DADDR(mp, start_fsb) >= eofs)
return 0; return 0;
} else { } else {
/* Last record seen was a shareable file data extent */
info->low.rm_offset += info->low.rm_blockcount; info->low.rm_offset += info->low.rm_blockcount;
} }
info->low.rm_startblock = XFS_FSB_TO_AGBNO(mp, start_fsb);
info->high.rm_startblock = -1U; info->high.rm_startblock = -1U;
info->high.rm_owner = ULLONG_MAX; info->high.rm_owner = ULLONG_MAX;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment