Bug 252787 - fstyp heuristics can misidentify UFS file systems created by newfs, goofs up autofs's special_media map
Summary: fstyp heuristics can misidentify UFS file systems created by newfs, goofs up ...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.2-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-17 19:09 UTC by Richard M Kreuter
Modified: 2021-02-02 21:14 UTC (History)
1 user (show)

See Also:


Attachments
Demonstrate fstyp's failure to detect UFS on a suitably evolved volume (854 bytes, application/x-sh)
2021-01-17 19:09 UTC, Richard M Kreuter
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard M Kreuter 2021-01-17 19:09:17 UTC
Created attachment 221685 [details]
Demonstrate fstyp's failure to detect UFS on a suitably evolved volume

The code fstyp uses to detect FAT and EXFAT look for magic strings in a block that UFS doesn't use, and fstyp checks for FAT formats before UFS. Together, these have consequence that fstyp will misidentify the file system type on a volume that has at one time been formatted with either of those formats and is then reformatted with newfs without any intervening operation that overwrite the first block on the volume. Reproduction attached.

Next, while I haven't reproduced, it looks as if the same issue will apply to a volume that was once NTFS formatted (because the NTFS magic string is in the same location, and the check for NTFS precedes the one for UFS).

Finally, because the default autofs special_media map uses fstyp to guess the file system format on a volume, this combined behavior of newfs & fstyp suffices to render such a file system unmountable by that map (though the volume is mountable manually). 

I ran into this issue on a recently purchased USB storage device after changing the MBR slice type (but not the slice's size) and creating a UFS file system on that slice using newfs, only to find that the automounter couldn't mount the volume.

Anyway, it's easy enough for a user to fix a given volume by overwriting some bytes where FAT/EXFAT/NTFS stores its magic string, so perhaps this is just a documentation issue, perhaps for the fstyp man page. (It's not clear to me that any of newfs, fstyp, or the special_media map is doing anything wrong. But they happen not to combine to achieve the desired result in this specific scenario, so something seems to want improvement.)
Comment 1 Conrad Meyer freebsd_committer 2021-01-17 19:49:32 UTC
In 13 (r355996), fstyp can locate exFAT labels and probably avoid a false positive match with UFS most of the time.  (Specifically, we look at the first 11 sectors now and verify them against a checksum in sector 12.)

(Unfortunately, some (all?) of the 'goto done's in fstyp_exfat() should be changed to 'goto fail'.  Until that is done, 13's fstyp is no better than 12's as far as just looking for the magic "EXFAT" string.)

newfs probably avoids writing the first sector for historical reasons (don't want to clobber a boot sector) which probably means there is an inherent collision between reimaged classic FAT / UFS volumes.  Maybe fstyp could be smarter here (some tie-breaking scheme for multiple positive results) if it is straightforward to be "really sure" a volume is UFS and not FAT.
Comment 2 commit-hook freebsd_committer 2021-01-17 19:58:45 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ddf61156132b610915325769cbb93ea11be0d433

commit ddf61156132b610915325769cbb93ea11be0d433
Author:     Conrad Meyer <cem@FreeBSD.org>
AuthorDate: 2021-01-17 19:55:06 +0000
Commit:     Conrad Meyer <cem@FreeBSD.org>
CommitDate: 2021-01-17 19:55:06 +0000

    fstyp(8): fix exfat detection

    In the presence of high-level errors (spec violations, bad boot blocks
    checksum), report non-detection instead of detection.

    PR:     252787 (related, but does not fully address)

 usr.sbin/fstyp/exfat.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
Comment 3 Richard M Kreuter 2021-01-19 22:10:33 UTC
By inspection, it appears that the improvements in 13 do not address this issue: I tested with a checkout that includes commit ddf61156, using my attached reproduction to create a suitably goofy disk image, and it still reports exfat after newfs. 

The UFS superblock starts at 8192 bytes, i.e., 16 sectors, from the start of a volume, right? If that's correct, then the exfat code's examination of the first 12 sectors won't avoid this false positive.

Anyhow, if someone wishes to consider this a defect in fstyp, one potential minimally disruptive approach would be to reorder the checks in fstyp.c so that the more common file system formats are checked for last (right now things are checked for in alphabetical order). For the purpose of heuristically guessing the format of a removable medium, I'd say that EXFAT or FAT is most likely (since commodity media is often pre-formatted that way), and so "falling through" to check for those after ruling out UFS, ZFS, etc., will arrive at the correct answer while avoiding this particular false positive.

That said, I'd be satisfied considering the possibility of a false positive from fstyp just a documentation issue. It's easy enough to overwrite the first sector of a disk with dd, for instance, so long as one knows that that's the cause of the false positive.
Comment 4 Ed Maste freebsd_committer 2021-02-02 20:57:15 UTC
(In reply to Richard M Kreuter from comment #3)

I think this approach sounds reasonable - UFS newfs on top of a preformatted FAT/exFAT USB stick seems a likely case.
Comment 5 commit-hook freebsd_committer 2021-02-02 21:14:22 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a072e2133ed36f1edf899068e6b026b129c919d8

commit a072e2133ed36f1edf899068e6b026b129c919d8
Author:     Conrad Meyer <cem@FreeBSD.org>
AuthorDate: 2021-01-17 19:55:06 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-02-02 21:12:31 +0000

    fstyp(8): fix exfat detection

    In the presence of high-level errors (spec violations, bad boot blocks
    checksum), report non-detection instead of detection.

    PR:     252787 (related, but does not fully address)
    (cherry picked from commit ddf61156132b610915325769cbb93ea11be0d433)

 usr.sbin/fstyp/exfat.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)