On Thursday 01 October 2009 15:35:12 Karel Zak wrote: > Hi Lawrence, > > thanks for your bug report and patch. > > On Wed, Sep 30, 2009 at 03:15:41PM +0200, Lawrence Rust wrote: > > function. A very important feature of these disks is the boot signature > > - bytes 0x55, 0xaa at the end of the 1st sector which indicate to the > > BIOS that > > Ah.. yes, I know. I fixed exactly the same problem in libvolume_id > few years ago :-) Unfortunately, this 0x55 0xaa check wasn't ported > to libblkid. > > > To fix this I wrote this small patch to check for the 0x55, 0xaa bytes at > > the end of the sector: > > You didn't test the patch, right? :-) Hmm, I thought that I did test but I see now that the patch apparently worked but was testing the wrong data! That'll teach me to make assumptions about code correctness. > > + /* Old floppies have a valid MBR signature */ > > + if (ms->ms_pmagic[0] != 0x55 || ms->ms_pmagic[1] != 0xAA) > > + return 1; > > there was a wrong comment in msdos_super_block, the ms_pmagic is at > 0xef offset. The MBR signature is at 0x1ef. Well spotted, but I believe that we are both mistaken. This link: http://support.microsoft.com/kb/140418 details the layout of a DOS boot sector, which is 512 bytes long. This ref is also in agreement with my "DOS Programmers Reference" by Que from 1992. The ms_pmagic and vs_pmagic fields should both be at offset 0x1fe (as noted in the original comments), unfortunately both vs_dummy2 and ms_dummy2 are the wrong length! I have included a compile time check on the sizeof these structs to avoid further problems. Hopefully the final diff: diff -uprN util-linux-ng-2.16.1/shlibs/blkid/src/probers/vfat.c util-linux-ng-2.16.1-new/shlibs/blkid/src/probers/vfat.c --- util-linux-ng-2.16.1/shlibs/blkid/src/probers/vfat.c 2009-08-21 15:59:30.000000000 +0200 +++ util-linux-ng-2.16.1-new/shlibs/blkid/src/probers/vfat.c 2009-10-01 21:05:26.000000000 +0200 @@ -18,6 +18,13 @@ #include "blkidP.h" +#undef STATIC_ASSERT +#ifdef __GNUC__ +# define STATIC_ASSERT(e) extern char _dummy[(e) ? 1 : -1] __attribute__((unused)) +#else +# define STATIC_ASSERT(e) extern char _dummy[(e) ? 1 : -1] +#endif + /* Yucky misaligned values */ struct vfat_super_block { /* 00*/ unsigned char vs_ignored[3]; @@ -45,7 +52,7 @@ struct vfat_super_block { /* 43*/ unsigned char vs_serno[4]; /* 47*/ unsigned char vs_label[11]; /* 52*/ unsigned char vs_magic[8]; -/* 5a*/ unsigned char vs_dummy2[164]; +/* 5a*/ unsigned char vs_dummy2[0x1fe -0x5a]; /*1fe*/ unsigned char vs_pmagic[2]; }; @@ -58,20 +65,23 @@ struct msdos_super_block { /* 0e*/ uint16_t ms_reserved; /* 10*/ uint8_t ms_fats; /* 11*/ unsigned char ms_dir_entries[2]; -/* 13*/ unsigned char ms_sectors[2]; +/* 13*/ unsigned char ms_sectors[2]; /* =0 iff V3 or later */ /* 15*/ unsigned char ms_media; -/* 16*/ uint16_t ms_fat_length; +/* 16*/ uint16_t ms_fat_length; /* Sectors per FAT */ /* 18*/ uint16_t ms_secs_track; /* 1a*/ uint16_t ms_heads; /* 1c*/ uint32_t ms_hidden; -/* 20*/ uint32_t ms_total_sect; -/* 24*/ unsigned char ms_unknown[3]; +/* V3 BPB */ +/* 20*/ uint32_t ms_total_sect; /* iff ms_sectors == 0 */ +/* V4 BPB */ +/* 24*/ unsigned char ms_unknown[3]; /* Phys drive no., resvd, V4 sig (0x29) */ /* 27*/ unsigned char ms_serno[4]; /* 2b*/ unsigned char ms_label[11]; /* 36*/ unsigned char ms_magic[8]; -/* 3d*/ unsigned char ms_dummy2[192]; +/* 3e*/ unsigned char ms_dummy2[0x1fe -0x3e]; /*1fe*/ unsigned char ms_pmagic[2]; }; +STATIC_ASSERT( sizeof( struct msdos_super_block) == 512); struct vfat_dir_entry { uint8_t name[11]; @@ -143,6 +153,10 @@ static int probe_fat_nomagic(blkid_probe if (!ms) return -1; + /* Old floppies have a valid MBR signature */ + if (ms->ms_pmagic[0] != 0x55 || ms->ms_pmagic[1] != 0xAA) + return 1; + /* heads check */ if (ms->ms_heads == 0) return 1; signed-off-by: Lawrence Rust Fingers crossed for the lat time :-) -- Lawrence Rust -- To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
