[PATCH v2] metadump: catch used extent array overflow

[PATCH v2] metadump: catch used extent array overflow

[cem]

From: Carlos Maiolino <cem@kernel.org>

An user reported a SIGSEGV when attempting to create a metadump image of
a filesystem.
The reason is because we fail to catch a possible overflow in the
used extents array in process_exinode() which may happen if the extent
count is corrupted.
This leads process_bmbt_reclist() to attempt to index into the array
using the bogus extent count with:

convert_extent(&rp[numrecs - 1], &o, &s, &c, &f);

Fix this by extending the used counter to uint64_t and
checking for the overflow possibility.

Reported-by: "hubert ." <hubjin657@outlook.com>
Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	v2:
	 - use uint64_t instead of xfs_extnum_t
	 - use check_mul_overflow() for overflow check

 db/metadump.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index 24eb99da1723..39639a0d51b0 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -2395,21 +2395,24 @@ process_btinode(
 
 static int
 process_exinode(
-	struct xfs_dinode 	*dip,
+	struct xfs_dinode	*dip,
 	int			whichfork)
 {
 	xfs_extnum_t		max_nex = xfs_iext_max_nextents(
 			xfs_dinode_has_large_extent_counts(dip), whichfork);
 	xfs_extnum_t		nex = xfs_dfork_nextents(dip, whichfork);
-	int			used = nex * sizeof(struct xfs_bmbt_rec);
+	uint64_t		used;
 
-	if (nex > max_nex || used > XFS_DFORK_SIZE(dip, mp, whichfork)) {
-		if (metadump.show_warnings)
-			print_warning("bad number of extents %llu in inode %lld",
-				(unsigned long long)nex,
-				(long long)metadump.cur_ino);
-		return 1;
-	}
+	if (check_mul_overflow(nex, sizeof(struct xfs_bmbt_rec), &used))
+		goto out_warn;
+
+	/* Invalid number of extents */
+	if (nex > max_nex)
+		goto out_warn;
+
+	/* Extent array should fit into the inode fork */
+	if (used > XFS_DFORK_SIZE(dip, mp, whichfork))
+		goto out_warn;
 
 	/* Zero unused data fork past used extents */
 	if (metadump.zero_stale_data &&
@@ -2421,6 +2424,12 @@ process_exinode(
 	return process_bmbt_reclist(dip, whichfork,
 			(struct xfs_bmbt_rec *)XFS_DFORK_PTR(dip, whichfork),
 			nex);
+
+out_warn:
+	if (metadump.show_warnings)
+		print_warning("bad number of extents %llu in inode %lld",
+			(unsigned long long)nex, (long long)metadump.cur_ino);
+	return 1;
 }
 
 static int
-- 
2.51.0

Re: [PATCH v2] metadump: catch used extent array overflow

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>