overflow in libsa dosfs, feature for efiboot (patches provided)

[Rob Newberry <robthedude%mac.com@localhost>]

Hi, everyone.

I've been playing with efiboot on Raspberry Pi.  I've added the ability to load a kernel, DTB file and RAM disk directly from an SD card that has a single FAT partition and no disklabel.  (This makes it a little easier to do some development on my Mac and not need to worry about FFS on the SD card.)

While debugging the efiboot changes, I found a lousy condition in libsa's dosfs.  My SD card is 64 GB, and as new files were added, they were being added with fairly high cluster values.  libsa's dosfs would be OK with the numbers themselves, but when it did particular bhtshifting operations, it  would overflow and then I would end up reading in bogus data.

Below are patches -- one to "dosfs.c" to fix the overflow problem, one to "efiblock.c" to add the "deal with no disklabel" feature.  I don't know the proper way to propose or advocate for these, but I'm sharing them here in the hopes that they'll eventually make it in -- though surely after some feedback from others.  Thanks for any advice!

Rob




#################### patch to dosfs for large SD cards ####################


diff --git a/sys/lib/libsa/dosfs.c b/sys/lib/libsa/dosfs.c
index c39a8c806b6..eaaeee512a8 100644
--- a/sys/lib/libsa/dosfs.c
+++ b/sys/lib/libsa/dosfs.c
@@ -118,7 +118,14 @@ static const struct direntry dot[2] = {
 #define blksec(fs, b)  ((b) << ((fs)->bshift - SSHIFT))
 
 /* Convert cluster number to offset within filesystem */
-#define blkoff(fs, b) (secbyt((fs)->lsndta) + blkbyt(fs, (b) - LOCLUS))
+static inline uint64_t blkoff( DOS_FS *fs, uint64_t b )
+{
+       uint64_t        r;
+       r = secbyt((uint64_t)fs->lsndta);
+       r += blkbyt(fs, b - LOCLUS );
+       return r;
+}
 
 /* Convert cluster number to logical sector number */
 #define blklsn(fs, b)  ((fs)->lsndta + blksec(fs, (b) - LOCLUS))
@@ -146,7 +153,7 @@ static off_t fsize(DOS_FS *, struct direntry *);
 static int fatcnt(DOS_FS *, u_int);
 static int fatget(DOS_FS *, u_int *);
 static int fatend(u_int, u_int);
-static int ioread(DOS_FS *, u_int, void *, u_int);
+static int ioread(DOS_FS *, uint64_t, void *, u_int);
 static int iobuf(DOS_FS *, u_int);
 static int ioget(struct open_file *, u_int, void *, u_int);
 
@@ -733,7 +740,7 @@ fatend(u_int sz, u_int c)
  * Offset-based I/O primitive
  */
 static int
-ioread(DOS_FS *fs, u_int offset, void *buf, u_int nbyte)
+ioread(DOS_FS *fs, uint64_t offset, void *buf, u_int nbyte)
 {
        char   *s;
        u_int   off, n;




############## patch to efiblock to allow loading kernel from FAT-only, non-disklabeled disk ##################



diff --git a/sys/stand/efiboot/efiblock.c b/sys/stand/efiboot/efiblock.c
index 73a4acfef32..8987d272736 100644
--- a/sys/stand/efiboot/efiblock.c
+++ b/sys/stand/efiboot/efiblock.c
@@ -179,6 +179,7 @@ efi_block_find_partitions_mbr(struct efi_block_dev *bdev)
        void *buf, *buf_start;
        UINT32 sz;
        int n;
+       int found_disklabel = 0;
 
        sz = __MAX(sizeof(mbr), bdev->bio->Media->BlockSize);
        sz = roundup(sz, bdev->bio->Media->BlockSize);
@@ -203,10 +204,60 @@ efi_block_find_partitions_mbr(struct efi_block_dev *bdev)
                        continue;
                if (mbr_part->mbrp_type == MBR_PTYPE_NETBSD) {
                        efi_block_find_partitions_disklabel(bdev, &mbr, le32toh(mbr_part->mbrp_start), le32toh(mbr_part->mbrp_size));
+                       found_disklabel = 1;
                        break;
                }
        }
 
+       if ( !found_disklabel )
+       {
+               struct efi_block_part *bpart;
+
+               for (n = 0; n < MBR_PART_COUNT; n++)
+               {
+                       mbr_part = &mbr.mbr_parts[n];
+                       if (le32toh(mbr_part->mbrp_size) == 0)
+                               continue;
+                       if (mbr_part->mbrp_type == MBR_PTYPE_FAT32)
+                       {
+                               // add the partition
+                               struct disklabel d;
+                               struct partition *p;
+
+                               // fake a disklabel
+                               memset( &d, 0, sizeof( d ) );
+                               d.d_magic = le32toh( DISKMAGIC );
+                               d.d_magic2 = le32toh( DISKMAGIC );
+                               d.d_npartitions = le16toh( 1 );
+
+
+                               d.d_secsize = le32toh(bdev->bio->Media->BlockSize);
+
+                               p = &d.d_partitions[0];
+
+                               // fill in p
+                               p->p_size = le32toh(mbr_part->mbrp_size);	// number of sectors in the partition
+                               p->p_offset = le32toh(mbr_part->mbrp_start);	// starting sector
+                               p->p_fsize = 1024;				// XXX basic fragment size?
+                               p->p_fstype = FS_MSDOS;				// filesystem type
+                               p->p_frag = 8;					// XXX file system fragments per block?
+                               p->p_cpg = 0;					// XXX ?
+
+                               bpart = alloc(sizeof(*bpart));
+                               bpart->index = 0;
+                               bpart->bdev = bdev;
+                               bpart->type = EFI_BLOCK_PART_DISKLABEL;