[PATCH 0/1] mdrestore: fix restore_v2() superblock length check

[PATCH 0/1] mdrestore: fix restore_v2() superblock length check

[Pavel Reichl]

Hi all,

On s390x (big-endian), running xfstests -g metadump currently fails
6 out of 9 tests. The failure is triggered by the superblock
extent-length validation in restore_v2(). The code rejects
xme_len == 1, but a length of 1 is the correct and expected value,
since the superblock fits within a single 512-byte sector.

On big-endian systems, this length decodes to 1 and the check aborts
the restore. On little-endian systems, the same on-disk bytes are
interpreted as 16777216, so the faulty logic never triggers there.

The patch removes the incorrect rejection of a valid extent length of
1 and applies proper logic so that the superblock extent length is
validated consistently across all architectures.

The outline of the fix was discussed with Chandan (thanks).

Pavel Reichl (1):
  mdrestore: fix restore_v2() superblock length check

 mdrestore/xfs_mdrestore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.52.0

[PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Pavel Reichl]

On big-endian architectures (e.g. s390x), restoring a filesystem from a
v2 metadump fails with "Invalid superblock disk address/length". This is
caused by restore_v2() treating a superblock extent length of 1 as an
error, even though a length of 1 is expected because the superblock fits
within a 512-byte sector.

On little-endian systems, the same raw extent length bytes that represent
a value of 1 on big-endian are misinterpreted as 16777216 due to byte
ordering, so the faulty check never triggers there and the bug is hidden.

Fix the issue by using an endian-correct comparison of xme_len so that
the superblock extent length is validated properly and consistently on
all architectures.

Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 mdrestore/xfs_mdrestore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index f10c4bef..71c2bb9a 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -437,7 +437,7 @@ restore_v2(
 	if (fread(&xme, sizeof(xme), 1, md_fp) != 1)
 		fatal("error reading from metadump file\n");
 
-	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
+	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||
 	    (be64_to_cpu(xme.xme_addr) & XME_ADDR_DEVICE_MASK) !=
 			XME_ADDR_DATA_DEVICE)
 		fatal("Invalid superblock disk address/length\n");
-- 
2.52.0

Re: [PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Darrick J. Wong]

On Tue, Dec 09, 2025 at 09:27:00PM +0100, Pavel Reichl wrote:
> On big-endian architectures (e.g. s390x), restoring a filesystem from a
> v2 metadump fails with "Invalid superblock disk address/length". This is
> caused by restore_v2() treating a superblock extent length of 1 as an
> error, even though a length of 1 is expected because the superblock fits
> within a 512-byte sector.
> 
> On little-endian systems, the same raw extent length bytes that represent
> a value of 1 on big-endian are misinterpreted as 16777216 due to byte
> ordering, so the faulty check never triggers there and the bug is hidden.
> 
> Fix the issue by using an endian-correct comparison of xme_len so that
> the superblock extent length is validated properly and consistently on
> all architectures.
> 
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
>  mdrestore/xfs_mdrestore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index f10c4bef..71c2bb9a 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -437,7 +437,7 @@ restore_v2(
>  	if (fread(&xme, sizeof(xme), 1, md_fp) != 1)
>  		fatal("error reading from metadump file\n");
>  
> -	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
> +	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||

xme.xme_len is the ondisk value, so that should be be32_to_cpu().

Otherwise the patch looks ok.

--D

>  	    (be64_to_cpu(xme.xme_addr) & XME_ADDR_DEVICE_MASK) !=
>  			XME_ADDR_DATA_DEVICE)
>  		fatal("Invalid superblock disk address/length\n");
> -- 
> 2.52.0
> 
> 

Re: [PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Christoph Hellwig]

On Tue, Dec 09, 2025 at 12:50:17PM -0800, Darrick J. Wong wrote:
> > -	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
> > +	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||
> 
> xme.xme_len is the ondisk value, so that should be be32_to_cpu().
> 
> Otherwise the patch looks ok.

We really need to bring back regular sparse runs on the userspace
code.  Let's see if I can get it back working..

Re: [PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Christoph Hellwig]

On Tue, Dec 09, 2025 at 09:28:43PM -0800, Christoph Hellwig wrote:
> On Tue, Dec 09, 2025 at 12:50:17PM -0800, Darrick J. Wong wrote:
> > > -	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
> > > +	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||
> > 
> > xme.xme_len is the ondisk value, so that should be be32_to_cpu().
> > 
> > Otherwise the patch looks ok.
> 
> We really need to bring back regular sparse runs on the userspace
> code.  Let's see if I can get it back working..

I just gave it a try, and make CC=cgcc still works in theory.
But between the urcu headers making it throw up, issues in the
Linux UAPI headers and our own redefinition of the __be32/__be16
types it generates so much noise that it stops reporting before
any real issues including this one.  Sigh.  I'll see if there
is a way to clean some of this up and get useful results.

Re: [PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Darrick J. Wong]

On Tue, Dec 09, 2025 at 10:00:32PM -0800, Christoph Hellwig wrote:
> On Tue, Dec 09, 2025 at 09:28:43PM -0800, Christoph Hellwig wrote:
> > On Tue, Dec 09, 2025 at 12:50:17PM -0800, Darrick J. Wong wrote:
> > > > -	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
> > > > +	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||
> > > 
> > > xme.xme_len is the ondisk value, so that should be be32_to_cpu().
> > > 
> > > Otherwise the patch looks ok.
> > 
> > We really need to bring back regular sparse runs on the userspace
> > code.  Let's see if I can get it back working..
> 
> I just gave it a try, and make CC=cgcc still works in theory.
> But between the urcu headers making it throw up, issues in the
> Linux UAPI headers and our own redefinition of the __be32/__be16
> types it generates so much noise that it stops reporting before
> any real issues including this one.  Sigh.  I'll see if there
> is a way to clean some of this up and get useful results.

I usually just grep -v out the macro crap and squint real hard to find
the real issues.  Oftentimes Dan Carpenter would help out, but I hear
that his work on that is coming to an end.

--D

Re: [PATCH 1/1] mdrestore: fix restore_v2() superblock length check

[Eric Sandeen]

On 12/10/25 12:00 AM, Christoph Hellwig wrote:
> On Tue, Dec 09, 2025 at 09:28:43PM -0800, Christoph Hellwig wrote:
>> On Tue, Dec 09, 2025 at 12:50:17PM -0800, Darrick J. Wong wrote:
>>>> -	if (xme.xme_addr != 0 || xme.xme_len == 1 ||
>>>> +	if (xme.xme_addr != 0 || cpu_to_be32(xme.xme_len) != 1 ||
>>>
>>> xme.xme_len is the ondisk value, so that should be be32_to_cpu().
>>>
>>> Otherwise the patch looks ok.
>>
>> We really need to bring back regular sparse runs on the userspace
>> code.  Let's see if I can get it back working..
> 
> I just gave it a try, and make CC=cgcc still works in theory.
> But between the urcu headers making it throw up, issues in the
> Linux UAPI headers and our own redefinition of the __be32/__be16
> types it generates so much noise that it stops reporting before
> any real issues including this one.  Sigh.  I'll see if there
> is a way to clean some of this up and get useful results.
> 

"make C=1 / C=2" worked once but when I ran it after seeing this patch,
it didn't seem to catch any errors. It spewed a lot of other things
though, as you mention (urcu, ugh).

I didn't realize that those results could swamp out other reports. :(

-Eric