Differential D37675

ufs/ffs: detect endian mismatch between machine and filesystem
ClosedPublic
Actions

Authored by alfredo on Dec 12 2022, 4:22 AM.

Details

Reviewers

mckusick

Group Reviewers

PowerPC

Commits

rGb13110e9f3ca: ufs/ffs: detect endian mismatch between machine and filesystem

Summary

Mount on a LE machine a filesystem formatted for BE is not supported currently. This adds a check for the superblock magic number using swapped bytes to guess and warn the user that it may be a valid superblock but endian is incompatible.

MFC after: 2 weeks

The following kernel output is seen when endian mismatch is detected:

Trying to mount root from ufs:/dev/vtbd0s3 []...
UFS2 superblock failed: possible endian mismatch between machine and filesystem
Attempted recovery for standard superblock: failed
Attempted extraction of recovery data from standard superblock: failed
Attempt to find boot zone recovery data.
panic: ffs_use_bread: non-NULL *bufp 0xc00800001b2868b0
...
...

Test Plan

test powerpc64 kernel with UFS2 created for BE and LE
test powerpc64le kernel with UFS2 created for BE and LE

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 48739
Build 45625: arc lint + arc unit

Event Timeline

alfredo created this revision.Dec 12 2022, 4:22 AM

Herald added a subscriber: imp. · View Herald TranscriptDec 12 2022, 4:22 AM

alfredo requested review of this revision.Dec 12 2022, 4:22 AM

I have some questions :)
Should we define a custom (negative) error number for endian mismatch?
Should we check for the returned error number and make the caller print the message instead?

Thanks for tracking down this issue. Your patch is a good start, but needs some followup in other areas of the code. I hope to come up with a proposal soon.

Here is my proposed fix:

{F53027994}

Let me know if this works for you.

In D37675#856834, @mckusick wrote:

Here is my proposed fix:

{F53027994}

Let me know if this works for you.

Wow, that was fast, thanks!
How can I access {F53027994}?

You should just need to click on the "Download" link in the diffs box. If that does not work let me know and I will "commandeer" this report and replace your diff with mine.

@mckusick it looks like you need to somehow give others read permission on the diff, only you have permission.

I cannot figure out how to share the diffs download, so here they are inline.

diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c
index 705f8c9c961d..0b462add0d05 100644
--- a/sys/ufs/ffs/ffs_subr.c
+++ b/sys/ufs/ffs/ffs_subr.c
@@ -35,6 +35,7 @@
 __FBSDID("$FreeBSD$");

 #include <sys/param.h>
+#include <sys/endian.h>
 #include <sys/limits.h>

 #ifndef _KERNEL
@@ -144,6 +145,7 @@ static int validate_sblock(struct fs *, int);
  *     EIO: non-existent or truncated superblock.
  *     EIO: error reading summary information.
  *     ENOENT: no usable known superblock found.
+ *     EILSEQ: filesystem with wrong byte order found.
  *     ENOMEM: failed to allocate space for the superblock.
  *     EINVAL: The previous newfs operation on this volume did not complete.
  *         The administrator must complete newfs before using this volume.
@@ -382,6 +384,17 @@ validate_sblock(struct fs *fs, int flags)
        prtmsg = ((flags & UFS_NOMSG) == 0);
        warnerr = (flags & UFS_NOWARNFAIL) == UFS_NOWARNFAIL ? 0 : ENOENT;
        wmsg = warnerr ? "" : " (Ignored)";
+       /*
+        * Check for endian mismatch between machine and filesystem.
+        */
+       if (((fs->fs_magic != FS_UFS2_MAGIC) &&
+           (bswap32(fs->fs_magic) == FS_UFS2_MAGIC)) ||
+           ((fs->fs_magic != FS_UFS1_MAGIC) &&
+           (bswap32(fs->fs_magic) == FS_UFS1_MAGIC))) {
+               MPRINT("UFS superblock failed due to endian mismatch "
+                   "between machine and filesystem\n");
+               return(EILSEQ);
+       }
        /*
         * If just validating for recovery, then do just the minimal
         * checks needed for the superblock fields needed to find
@@ -627,8 +640,16 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags,
         * failure can be avoided.
         */
        flags = UFS_NOMSG | nocsum;
-       if (ffs_sbget(devfd, fsp, UFS_STDSB, flags, filltype, readfunc) == 0)
-               return (0);
+       error = ffs_sbget(devfd, fsp, UFS_STDSB, flags, filltype, readfunc);
+       /*
+        * If successful or endian error, no need to try further.
+        */
+       if (error == 0 || error == EILSEQ) {
+               if (msg && error == EILSEQ)
+                       printf("UFS superblock failed due to endian mismatch "
+                           "between machine and filesystem\n");
+               return (error);
+       }
        /*
         * First try: ignoring hash failures.
         */
@@ -677,6 +698,7 @@ ffs_sbsearch(void *devfd, struct fs **fsp, int reqflags,
                 * but some devices lie. So we just try a plausible range.
                 */
                error = ENOENT;
+               fsrbuf = NULL;
                for (secsize = dbtob(1); secsize <= SBLOCKSIZE; secsize *= 2)
                        if ((error = (*readfunc)(devfd, (SBLOCK_UFS2 - secsize),
                            &fsrbuf, secsize)) == 0)

Update with code from mckusick

Harbormaster completed remote builds in B48738: Diff 114215.Dec 17 2022, 3:02 AM

remove empty line

Harbormaster completed remote builds in B48739: Diff 114216.Dec 17 2022, 3:03 AM

@mckusick , thank you for the improved patch!
During test I figured out that kernel retries to mount the filesystem several times. I prevented this changing sys/kern/vfs_mountroot.c

With the patch we have the following behavior during boot:

Trying to mount root from ufs:vtbd0s3 []...
UFS superblock failed due to endian mismatch between machine and filesystem
Mounting from ufs:vtbd0s3 failed with error 86.

..
..
mountroot>

And when mounting in the userland we get:

root@:~ # mount /dev/vtbd1s3 /mnt/
UFS superblock failed due to endian mismatch between machine and filesystem
mount: /dev/vtbd1s3: Illegal byte sequence

Your addition to vfs_mountroot.c (parse_mount()) looks correct. Thanks for pushing this through.

This revision is now accepted and ready to land.Dec 19 2022, 7:03 AM

Closed by commit rGb13110e9f3ca: ufs/ffs: detect endian mismatch between machine and filesystem (authored by alfredo). · Explain WhyDec 20 2022, 3:26 AM

This revision was automatically updated to reflect the committed changes.

alfredo added a commit: rGb13110e9f3ca: ufs/ffs: detect endian mismatch between machine and filesystem.