fstyp: detect Raspberry Pi Pico boot filesystem as FAT

[emaste]

Summary

fstyp looks for a 0x55 0xAA signature at offset 510, but this is not
required by specifications and is not proivded by the Raspberry Pi Nano
bootloader.

We should really remove the signature check and implement a more
comprehensive BPB validation instead, but it will require more
investigation and testing.  For now add a special case for the
Raspberry Pi Nano bootloader.

PR:             262896
MFC after:      3 days
Sponsored by:   The FreeBSD Foundation

[emaste]

usr.sbin/fstyp/msdosfs.c

50–51

I decided to be conservative for now and look for this specific set of bytes to be confident we can MFC without introducing false positives, but we could update this later on.

[delphij]

I don't think this is right...

0xaa55 at the end of 512-byte sector was the signature for boot sector indicating that this is a valid boot sector; specification mentioned that and demands the values has to be 0xaa55, but nobody is enforcing it (Okay, we did check:

#ifndef MSDOSFS_NOCHECKSIG
        if (bsp->bs50.bsBootSectSig0 != BOOTSIG0 ||
            bsp->bs50.bsBootSectSig1 != BOOTSIG1) {
                error = EINVAL;
                goto error_exit;
        }
#endif

but it's not really required by specifications; I'm pretty sure that Windows would happily mount and silently fix the 0xaa55 signature, Linux would just ignore it).

A more reasonable check would be to perform some sanity check the BPB, basically something like the following, adapted from boot.c from fsck_msdosfs:

// Valid jmp; from specification.
if (sector0[0] != 0xe9 && (sector0[0] != 0xeb || sector0[2] != 0x90))
    return (false);

// Bytes per sector must be power of two, and has to be at least 512 and at most 4096 per specification
bpbBytesPerSec = sector0[11] + (sector0[12] << 8);
if (bpbBytesPerSec < 512 || bpbBytesPerSec > 4096 || !powerof2(bpbBytesPerSec))
    return (false);

// Bytes per cluster must be power of two and non-zero
bpbSecPerClust = sector0[13];
if (bpbSecPerClust == 0 || !powerof2(bpbSecPerClust))
    return (false);

// Reserved sectors must be greater than 1
bpbResSectors = sector0[14] + (sector0[15] << 8);
if (bpbResSectors < 1)
    return (false);

// Must have FAT
bpbFATs = sector0[16];
if (bpbFATs == 0)
    return (false);

and return (true) after that, as we are somewhat confident that this is a valid BPB. We could potentially check if it's a valid FAT32 BPB, if media type was valid, etc. as well.

[probono_puredarwin.org]

The patch works for me. Tested with actual Raspberry Pi Pico hardware. With this patch, /usr/local/sbin/automount will mount it. Thank you very much.

[emaste]

From the proposed test

// Valid jmp; from specification.
if (sector0[0] != 0xe9 && (sector0[0] != 0xeb || sector0[2] != 0x90))
    return (false);

I don't see this in boot.c now?

My goal here was to introduce a minimal change that I can have high confidence will not introduce a regression or a false positive. I agree that we need to use an approach like you suggest, but would like to do so on a slightly longer timescale (so that I can collect a variety of FAT images, etc.)

[delphij]

In D34699#786398, @emaste wrote:

From the proposed test

// Valid jmp; from specification.
if (sector0[0] != 0xe9 && (sector0[0] != 0xeb || sector0[2] != 0x90))
    return (false);

I don't see this in boot.c now?

It's not, but I'm pretty sure that Windows does look at it (they don't look at the 0xaa55 signature, despite they would fix it silently upon dismount). If you feel uncomfortable with it we can omit it for now.

My goal here was to introduce a minimal change that I can have high confidence will not introduce a regression or a false positive. I agree that we need to use an approach like you suggest, but would like to do so on a slightly longer timescale (so that I can collect a variety of FAT images, etc.)

Ok

usr.sbin/fstyp/msdosfs.c

50

I think you mean 511 here?

[emaste]

• correct offset
• add comment
• update commit message