Re: xfs_metadump segmentation fault on large fs - xfsprogs 6.1

[Dave Chinner <david@fromorbit.com>]

On Fri, Sep 26, 2025 at 03:45:17PM +0200, Carlos Maiolino wrote:
> On Fri, Sep 26, 2025 at 07:39:18PM +1000, Dave Chinner wrote:
> > So there must be a bounds checking bug in process_exinode():
> > 
> > static int
> > process_exinode(
> >         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);
> > 
> >         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;
> >         }
> > 
> > Can you spot it?
> > 
> > Hint: ((2^28 + 1) * 2^4) - 1 as an int is?
> 
> Perhaps the patch below will suffice?
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 34f2d61700fe..1dd38ab84ade 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -2395,7 +2395,7 @@ 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(
> @@ -2403,7 +2403,13 @@ process_exinode(
>  	xfs_extnum_t		nex = xfs_dfork_nextents(dip, whichfork);
>  	int			used = nex * sizeof(struct xfs_bmbt_rec);
>  
> -	if (nex > max_nex || used > XFS_DFORK_SIZE(dip, mp, whichfork)) {
> +	/*
> +	 * We need to check for overflow of used counter.
> +	 * If the inode extent count is corrupted, we risk having a
> +	 * big enough number of extents to overflow it.
> +	 */
> +	if (used < nex || 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,
> 

That fixes this specific problem, but now it will reject valid
inodes with valid but large extent counts.

What type does XFS_SB_FEAT_INCOMPAT_NREXT64 require for extent
count calculations?  i.e. what's the size of xfs_extnum_t?

-Dave.
-- 
Dave Chinner
david@fromorbit.com