Page MenuHomeFreeBSD

fstyp: detect Raspberry Pi Pico boot filesystem as FAT
ClosedPublic

Authored by emaste on Mar 28 2022, 9:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 9:05 PM
Unknown Object (File)
Sat, Nov 9, 12:59 PM
Unknown Object (File)
Wed, Nov 6, 1:47 AM
Unknown Object (File)
Wed, Nov 6, 1:43 AM
Unknown Object (File)
Tue, Nov 5, 11:21 PM
Unknown Object (File)
Oct 16 2024, 11:42 PM
Unknown Object (File)
Oct 15 2024, 7:43 AM
Unknown Object (File)
Oct 15 2024, 7:43 AM
Subscribers

Details

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste created this revision.
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 requested changes to this revision.Mar 29 2022, 4:42 AM

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.

This revision now requires changes to proceed.Mar 29 2022, 4:42 AM

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.

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.)

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 edited the summary of this revision. (Show Details)
  • correct offset
  • add comment
  • update commit message
This revision is now accepted and ready to land.Mar 29 2022, 9:21 PM