Skip to content

Commit cfa2df6

Browse files
author
Darrick J. Wong
committed
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: Dave Chinner <[email protected]> Fixes: 63ef7a3 ("xfs: fix interval filtering in multi-step fsmap queries") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]>
1 parent 0bb80ec commit cfa2df6

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

fs/xfs/xfs_fsmap.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,19 @@ xfs_getfsmap_rtdev_rtbitmap(
565565
}
566566
#endif /* CONFIG_XFS_RT */
567567

568+
static inline bool
569+
rmap_not_shareable(struct xfs_mount *mp, const struct xfs_rmap_irec *r)
570+
{
571+
if (!xfs_has_reflink(mp))
572+
return true;
573+
if (XFS_RMAP_NON_INODE_OWNER(r->rm_owner))
574+
return true;
575+
if (r->rm_flags & (XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK |
576+
XFS_RMAP_UNWRITTEN))
577+
return true;
578+
return false;
579+
}
580+
568581
/* Execute a getfsmap query against the regular data device. */
569582
STATIC int
570583
__xfs_getfsmap_datadev(
@@ -598,7 +611,6 @@ __xfs_getfsmap_datadev(
598611
* low to the fsmap low key and max out the high key to the end
599612
* of the AG.
600613
*/
601-
info->low.rm_startblock = XFS_FSB_TO_AGBNO(mp, start_fsb);
602614
info->low.rm_offset = XFS_BB_TO_FSBT(mp, keys[0].fmr_offset);
603615
error = xfs_fsmap_owner_to_rmap(&info->low, &keys[0]);
604616
if (error)
@@ -608,21 +620,20 @@ __xfs_getfsmap_datadev(
608620

609621
/* Adjust the low key if we are continuing from where we left off. */
610622
if (info->low.rm_blockcount == 0) {
611-
/* empty */
612-
} else if (XFS_RMAP_NON_INODE_OWNER(info->low.rm_owner) ||
613-
(info->low.rm_flags & (XFS_RMAP_ATTR_FORK |
614-
XFS_RMAP_BMBT_BLOCK |
615-
XFS_RMAP_UNWRITTEN))) {
616-
info->low.rm_startblock += info->low.rm_blockcount;
623+
/* No previous record from which to continue */
624+
} else if (rmap_not_shareable(mp, &info->low)) {
625+
/* Last record seen was an unshareable extent */
617626
info->low.rm_owner = 0;
618627
info->low.rm_offset = 0;
619628

620629
start_fsb += info->low.rm_blockcount;
621630
if (XFS_FSB_TO_DADDR(mp, start_fsb) >= eofs)
622631
return 0;
623632
} else {
633+
/* Last record seen was a shareable file data extent */
624634
info->low.rm_offset += info->low.rm_blockcount;
625635
}
636+
info->low.rm_startblock = XFS_FSB_TO_AGBNO(mp, start_fsb);
626637

627638
info->high.rm_startblock = -1U;
628639
info->high.rm_owner = ULLONG_MAX;

0 commit comments

Comments
 (0)