[PATCH] mdrestore: fix extent length overflow in v2 restore path

[PATCH] mdrestore: fix extent length overflow in v2 restore path

[Manognya Singuru]

Aisle Research reported that a crafted metadump v2 extent
length can overflow the signed int used for length,
leading to incorrect size calculations and a
potential heap buffer over-read in restore_meta_extent().

Change the length type to uint64_t and ensure all size
arithmetic in 64-bit before converting to size_t for I/O
calls.

Signed-off-by: Manognya Singuru <msinguru@redhat.com>
---
 mdrestore/xfs_mdrestore.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index f10c4bef..9c7f77e0 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -395,11 +395,9 @@ restore_meta_extent(
 	char		*device,
 	void		*buf,
 	uint64_t	offset,
-	int		len)
+	uint64_t		len)
 {
-	int		io_size;
-
-	io_size = min(len, MDR_IO_BUF_SIZE);
+	size_t io_size = min(len, MDR_IO_BUF_SIZE);
 
 	do {
 		if (fread(buf, io_size, 1, md_fp) != 1)
@@ -428,7 +426,7 @@ restore_v2(
 	int64_t			mb_read = 0;
 	int64_t			bytes_read;
 	uint64_t		offset;
-	int			len;
+	uint64_t			len;
 
 	block_buffer = malloc(MDR_IO_BUF_SIZE);
 	if (block_buffer == NULL)
@@ -442,7 +440,7 @@ restore_v2(
 			XME_ADDR_DATA_DEVICE)
 		fatal("Invalid superblock disk address/length\n");
 
-	len = BBTOB(be32_to_cpu(xme.xme_len));
+	len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));
 
 	if (fread(block_buffer, len, 1, md_fp) != 1)
 		fatal("error reading from metadump file\n");
@@ -503,7 +501,7 @@ restore_v2(
 			break;
 		}
 
-		len = BBTOB(be32_to_cpu(xme.xme_len));
+		len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));
 
 		restore_meta_extent(md_fp, fd, device, block_buffer, offset,
 				len);
-- 
2.54.0

Re: [PATCH] mdrestore: fix extent length overflow in v2 restore path

[Darrick J. Wong]

On Thu, Jul 09, 2026 at 04:09:15PM +0530, Manognya Singuru wrote:
> Aisle Research reported that a crafted metadump v2 extent
> length can overflow the signed int used for length,
> leading to incorrect size calculations and a
> potential heap buffer over-read in restore_meta_extent().
> 
> Change the length type to uint64_t and ensure all size
> arithmetic in 64-bit before converting to size_t for I/O
> calls.
> 
> Signed-off-by: Manognya Singuru <msinguru@redhat.com>
> ---
>  mdrestore/xfs_mdrestore.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index f10c4bef..9c7f77e0 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -395,11 +395,9 @@ restore_meta_extent(
>  	char		*device,
>  	void		*buf,
>  	uint64_t	offset,
> -	int		len)
> +	uint64_t		len)

Please make the indentation consistent with the existing code here and
elsewhere in the patch.

>  {
> -	int		io_size;
> -
> -	io_size = min(len, MDR_IO_BUF_SIZE);
> +	size_t io_size = min(len, MDR_IO_BUF_SIZE);
>  
>  	do {
>  		if (fread(buf, io_size, 1, md_fp) != 1)
> @@ -428,7 +426,7 @@ restore_v2(
>  	int64_t			mb_read = 0;
>  	int64_t			bytes_read;
>  	uint64_t		offset;
> -	int			len;
> +	uint64_t			len;
>  
>  	block_buffer = malloc(MDR_IO_BUF_SIZE);
>  	if (block_buffer == NULL)
> @@ -442,7 +440,7 @@ restore_v2(
>  			XME_ADDR_DATA_DEVICE)
>  		fatal("Invalid superblock disk address/length\n");
>  
> -	len = BBTOB(be32_to_cpu(xme.xme_len));
> +	len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));

/me wonders if BBTOB ought to cast its parameter to uint64_t but this
change here avoids shift truncation if xme_len is large, so

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  
>  	if (fread(block_buffer, len, 1, md_fp) != 1)
>  		fatal("error reading from metadump file\n");
> @@ -503,7 +501,7 @@ restore_v2(
>  			break;
>  		}
>  
> -		len = BBTOB(be32_to_cpu(xme.xme_len));
> +		len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));
>  
>  		restore_meta_extent(md_fp, fd, device, block_buffer, offset,
>  				len);
> -- 
> 2.54.0
> 
> 

Re: [PATCH] mdrestore: fix extent length overflow in v2 restore path

[Manognya Singuru]

On Thu, Jul 9, 2026 at 8:11 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Thu, Jul 09, 2026 at 04:09:15PM +0530, Manognya Singuru wrote:
> > Aisle Research reported that a crafted metadump v2 extent
> > length can overflow the signed int used for length,
> > leading to incorrect size calculations and a
> > potential heap buffer over-read in restore_meta_extent().
> >
> > Change the length type to uint64_t and ensure all size
> > arithmetic in 64-bit before converting to size_t for I/O
> > calls.
> >
> > Signed-off-by: Manognya Singuru <msinguru@redhat.com>
> > ---
> >  mdrestore/xfs_mdrestore.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> > index f10c4bef..9c7f77e0 100644
> > --- a/mdrestore/xfs_mdrestore.c
> > +++ b/mdrestore/xfs_mdrestore.c
> > @@ -395,11 +395,9 @@ restore_meta_extent(
> >       char            *device,
> >       void            *buf,
> >       uint64_t        offset,
> > -     int             len)
> > +     uint64_t                len)
>
> Please make the indentation consistent with the existing code here and
> elsewhere in the patch.
>

Sure, I have made the indentation consistent and sent PATCH v2.

> >  {
> > -     int             io_size;
> > -
> > -     io_size = min(len, MDR_IO_BUF_SIZE);
> > +     size_t io_size = min(len, MDR_IO_BUF_SIZE);
> >
> >       do {
> >               if (fread(buf, io_size, 1, md_fp) != 1)
> > @@ -428,7 +426,7 @@ restore_v2(
> >       int64_t                 mb_read = 0;
> >       int64_t                 bytes_read;
> >       uint64_t                offset;
> > -     int                     len;
> > +     uint64_t                        len;
> >
> >       block_buffer = malloc(MDR_IO_BUF_SIZE);
> >       if (block_buffer == NULL)
> > @@ -442,7 +440,7 @@ restore_v2(
> >                       XME_ADDR_DATA_DEVICE)
> >               fatal("Invalid superblock disk address/length\n");
> >
> > -     len = BBTOB(be32_to_cpu(xme.xme_len));
> > +     len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));
>
> /me wonders if BBTOB ought to cast its parameter to uint64_t but this
> change here avoids shift truncation if xme_len is large, so
>

Yes, my thinking was that casting to uint64_t would force BBTOB() to
perform the shift in 64-bit arithmetic and avoid shift truncation.


> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
>
> --D
>
> >
> >       if (fread(block_buffer, len, 1, md_fp) != 1)
> >               fatal("error reading from metadump file\n");
> > @@ -503,7 +501,7 @@ restore_v2(
> >                       break;
> >               }
> >
> > -             len = BBTOB(be32_to_cpu(xme.xme_len));
> > +             len = BBTOB((uint64_t)be32_to_cpu(xme.xme_len));
> >
> >               restore_meta_extent(md_fp, fd, device, block_buffer, offset,
> >                               len);
> > --
> > 2.54.0
> >
> >
>