[PATCH] metadump: catch used extent array overflow

[PATCH] 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 xfs_extnum_t (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>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
---
 db/metadump.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index 24eb99da1723..430120ec8888 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -2395,21 +2395,25 @@ 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);
+	xfs_extnum_t		used = nex * sizeof(struct xfs_bmbt_rec);
 
-	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;
-	}
+	/* Catch used extent array overflows */
+	if (used < nex)
+	       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 +2425,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] metadump: catch used extent array overflow

[Christoph Hellwig]

On Tue, Nov 11, 2025 at 03:10:57PM +0100, cem@kernel.org wrote:
> -	int			used = nex * sizeof(struct xfs_bmbt_rec);
> +	xfs_extnum_t		used = nex * sizeof(struct xfs_bmbt_rec);

used really isn't a xfs_extnum_t, so you probably just want to use an
undecored uint64_t.

Otherwise this looks good.

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

[Carlos Maiolino]

On Tue, Nov 11, 2025 at 06:19:50AM -0800, Christoph Hellwig wrote:
> On Tue, Nov 11, 2025 at 03:10:57PM +0100, cem@kernel.org wrote:
> > -	int			used = nex * sizeof(struct xfs_bmbt_rec);
> > +	xfs_extnum_t		used = nex * sizeof(struct xfs_bmbt_rec);
> 
> used really isn't a xfs_extnum_t, so you probably just want to use an
> undecored uint64_t.

Fair enough. Thanks!

> 
> Otherwise this looks good.

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

[Darrick J. Wong]

On Tue, Nov 11, 2025 at 04:10:57PM +0100, Carlos Maiolino wrote:
> On Tue, Nov 11, 2025 at 06:19:50AM -0800, Christoph Hellwig wrote:
> > On Tue, Nov 11, 2025 at 03:10:57PM +0100, cem@kernel.org wrote:
> > > -	int			used = nex * sizeof(struct xfs_bmbt_rec);
> > > +	xfs_extnum_t		used = nex * sizeof(struct xfs_bmbt_rec);
> > 
> > used really isn't a xfs_extnum_t, so you probably just want to use an
> > undecored uint64_t.
> 
> Fair enough. Thanks!

Does check_mul_overflow work for this purpose?

--D

> > 
> > Otherwise this looks good.
> 

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

[Christoph Hellwig]

On Wed, Nov 12, 2025 at 08:36:08AM -0800, Darrick J. Wong wrote:
> > > used really isn't a xfs_extnum_t, so you probably just want to use an
> > > undecored uint64_t.
> > 
> > Fair enough. Thanks!
> 
> Does check_mul_overflow work for this purpose?


Oh, I didn't realize we have that in xfsprogs now.  Using it here
sounds like a good idea.

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

[Carlos Maiolino]

On Wed, Nov 12, 2025 at 08:39:38AM -0800, Christoph Hellwig wrote:
> On Wed, Nov 12, 2025 at 08:36:08AM -0800, Darrick J. Wong wrote:
> > > > used really isn't a xfs_extnum_t, so you probably just want to use an
> > > > undecored uint64_t.
> > >
> > > Fair enough. Thanks!
> >
> > Does check_mul_overflow work for this purpose?
> 
> 
> Oh, I didn't realize we have that in xfsprogs now.  Using it here
> sounds like a good idea.

+1, thanks Darrick, I'll update it.