It is possible to create and use an msdos filesystem with 2048-byte sectors; but it is not possible to "fsck" it. "fsck_msdosfs" on such a fileystem returns: "could not read boot block (Invalid argument)" Fix: The following POC patch uses DIOCGSECTORSIZE instead of a fixed value of DOSBOOTBLOCKSIZE. ============= #include "ext.h" #include "fsutil.h" @@ -47,11 +48,23 @@ u_char backup[DOSBOOTBLOCKSIZE]; int ret = FSOK; int i; + u_char *tblock; + unsigned int sectorsize; - if (read(dosfs, block, sizeof block) != sizeof block) { + if ((ioctl(dosfs, DIOCGSECTORSIZE, §orsize) == -1) || sectorsize < sizeof block) { + printf("** could not get valid device sectorsize, assuming %d\n", sizeof block); + sectorsize = sizeof block; + } + tblock = (u_char *)malloc(sectorsize); + if (read(dosfs, tblock, sectorsize) != sectorsize) { perror("could not read boot block"); return FSFATAL; } + memcpy(block, tblock, sizeof block); + free(tblock); + if (sectorsize != sizeof block) { + printf("** non-standard sectorsize of %d\n", sectorsize); + } if (block[510] != 0x55 || block[511] != 0xaa) { pfatal("Invalid signature in boot block: %02x%02x", =============--ino5c9CEu9dg5VKq8PBLbjahW1LhgN3rM9abFLqoaQs3GATg Content-Type: text/plain; name="file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="file.diff" --- src/sbin/fsck_msdosfs/boot.c.orig 2010-07-03 06:49:57.000000000 -0400 +++ src/sbin/fsck_msdosfs/boot.c 2010-12-24 05:49:09.000000000 -0500 @@ -35,6 +35,7 @@ #include <string.h> #include <stdio.h> #include <unistd.h> +#include <sys/disk.h> How-To-Repeat: dd if=/dev/zero bs=1m of=msdos.img count=20 MD=`mdconfig -af msdos.img` gnop create -S 2048 -v $MD newfs_msdos -n1 $MD.nop fsck_msdosfs /dev/$MD.nop ** /dev/md4.nop could not read boot block (Invalid argument) ... mount -t msdosfs /dev/$MD.nop /mnt
On Fri, 24 Dec 2010, Keith White wrote: >> Description: > > It is possible to create and use an msdos filesystem with 2048-byte > sectors; but it is not possible to "fsck" it. "fsck_msdosfs" on > such a fileystem returns: > "could not read boot block (Invalid argument)" > >> Fix: > > The following POC patch uses DIOCGSECTORSIZE instead of a fixed value of DOSBOOTBLOCKSIZE. A much larger patch is needed, since fsck_msdosfs does many other reads size DOSBOOTBLOCKSIZE or twice that, and to actually fix things it still tries writing size DOSBOOTBLOCKSIZE for the boot block as well as the writes corresponding to the other reads. Also, newfs_msdos used to always put the boot block signature and fsinfo in wrong places (relative to the end of the physical sector) when the physical sector size is not 512, so fsck_msdosfs needs to look in these wrong places as well as the right places, at least for its signature checks so that it doesn't abort, and consider moving stuff from the wrong places to the right places (but since msdosfs just doesn't see stuff in wrong places and doesn't check the primary signature, it is OK for fsck_msdosfs to ignore the misplaced metadata and create fresh metadata; this is especially true for fsinfo since it should be advisory-only). The unportable DIOCGSECTORSIZE ioctl is not needed and shouldn't be used, since the boot block contains the sector size that the file system was created with, and size returned by the ioctl (if any) it doesn't work unless the media is the same as that on which the file system was created and everything agrees about it. One case where the size returned by the ioctl is no good is for fsck on images of file systems in regular files -- then the ioctl always fail so it doesn't return any size. > ============= > --- src/sbin/fsck_msdosfs/boot.c.orig 2010-07-03 06:49:57.000000000 -0400 > +++ src/sbin/fsck_msdosfs/boot.c 2010-12-24 05:49:09.000000000 -0500 > @@ -35,6 +35,7 @@ > #include <string.h> > #include <stdio.h> > #include <unistd.h> > +#include <sys/disk.h> > > #include "ext.h" > #include "fsutil.h" > @@ -47,11 +48,23 @@ > u_char backup[DOSBOOTBLOCKSIZE]; read()s of size sizeof(backup) and possibly of sizeof(fsinfo) (1K) will still fail. The failures are fatal, so I think your patch only works on the unusual non-FAT32 case when these reads aren't attempted, and when there are also no errors in the boot blocks or fsinfo so no writes are attempted. I'm not completely happy with my patch. It preserves some style bugs. The reblocking is laborious. Perhaps it should be simplified using utility xread() and xwrite() functions, and make missing signatures non-fatal. It is very bogus that msdosfs now ignores the primary signature, while fsck_msdosfs refuses to even look at the file system when there is no primary signature or the primary signature is where newfs_msdos used to put it but not where it should be (except with this patch it accepts it in either place); so fsck_msdosfs can't even fix common signature problems; these should be handled like corrupted primary superblocks for ffs (ask but blindly continue if allowed). % Index: boot.c % =================================================================== % RCS file: /home/ncvs/src/sbin/fsck_msdosfs/boot.c,v % retrieving revision 1.6 % diff -u -2 -r1.6 boot.c % --- boot.c 31 Jan 2008 13:16:29 -0000 1.6 % +++ boot.c 3 Jul 2010 16:53:33 -0000 % @@ -48,4 +48,6 @@ % #include "fsutil.h" % % +#define IOSIZE 65536 % + % int % readboot(dosfs, boot) % @@ -53,18 +55,48 @@ % struct bootblock *boot; % { % + u_char ioblock[IOSIZE]; % + u_char iofsinfo[IOSIZE]; % + u_char iobackup[IOSIZE]; % u_char block[DOSBOOTBLOCKSIZE]; % u_char fsinfo[2 * DOSBOOTBLOCKSIZE]; % u_char backup[DOSBOOTBLOCKSIZE]; % + u_char *infop; % + size_t iosize; % + u_int secsize; % int ret = FSOK; % % - if (read(dosfs, block, sizeof block) < sizeof block) { % + /* Search for an i/o size that works. */ % + for (iosize = IOSIZE; iosize >= DOSBOOTBLOCKSIZE; iosize >>= 1) { % + if (lseek(dosfs, (off_t)0, SEEK_SET) == 0 && % + read(dosfs, ioblock, iosize) == (ssize_t)iosize) % + break; % + } % + if (iosize < DOSBOOTBLOCKSIZE) { % perror("could not read boot block"); % return FSFATAL; % } % + memcpy(block, ioblock, sizeof block); % % - if (block[510] != 0x55 || block[511] != 0xaa) { % - pfatal("Invalid signature in boot block: %02x%02x", block[511], block[510]); % + /* % + * Preliminary decode to determine where the signature might be. % + * It is supposed to be at the end of a 512-block, but we used to % + * put it at the end of a sector. Accept the latter so as to fix % + * it someday. % + */ % + secsize = block[11] + (block[12] << 8); % + if (secsize < sizeof block || secsize > IOSIZE) { % + perror("Preposterous or unsupported sector size"); % return FSFATAL; % } % + if (block[510] != 0x55 || block[511] != 0xaa) { % + if (ioblock[secsize - 2] != 0x55 || % + ioblock[secsize - 1] != 0xaa) { % + pfatal("Invalid signature in boot block: %02x%02x", % + block[511], block[510]); % + return FSFATAL; % + } % + pwarn( % + "Invalid primary signature in boot block -- using secondary\n"); % + } % % memset(boot, 0, sizeof *boot); % @@ -107,11 +139,12 @@ % boot->Backup = block[50] + (block[51] << 8); % % + iosize = (secsize >= sizeof fsinfo) ? secsize : sizeof fsinfo; % if (lseek(dosfs, boot->FSInfo * boot->BytesPerSec, SEEK_SET) % != boot->FSInfo * boot->BytesPerSec % - || read(dosfs, fsinfo, sizeof fsinfo) % - != sizeof fsinfo) { % + || read(dosfs, iofsinfo, iosize) != (ssize_t)iosize) { % perror("could not read fsinfo block"); % return FSFATAL; % } % + memcpy(fsinfo, iofsinfo, sizeof fsinfo); % if (memcmp(fsinfo, "RRaA", 4) % || memcmp(fsinfo + 0x1e4, "rrAa", 4) % @@ -124,4 +157,22 @@ % || fsinfo[0x3fe] != 0x55 % || fsinfo[0x3ff] != 0xaa) { % + infop = &iofsinfo[secsize - DOSBOOTBLOCKSIZE]; % + if (memcmp(fsinfo, "RRaA", 4) == 0 && % + memcmp(infop + 0x1e4, "rrAa", 4) == 0 && % + infop[0x1fc] == 0 && % + infop[0x1fd] == 0 && % + infop[0x1fe] == 0x55 && % + infop[0x1ff] == 0xaa) { % + pwarn( % + "Invalid signature in fsinfo block -- using secondary\n"); % + /* % + * Silently fix up the actual fsinfo data % + * (just 2 32-bit words) since this data % + * is advisory and the indentation is % + * already too painful to ask about this. % + */ % + memcpy(fsinfo + 0x1e8, infop + 0x1e8, 8); % + goto over; % + } % pwarn("Invalid signature in fsinfo block\n"); % if (ask(0, "Fix")) { % @@ -134,12 +185,14 @@ % fsinfo[0x3fe] = 0x55; % fsinfo[0x3ff] = 0xaa; % + memcpy(iofsinfo, fsinfo, sizeof fsinfo); % if (lseek(dosfs, boot->FSInfo * boot->BytesPerSec, SEEK_SET) % != boot->FSInfo * boot->BytesPerSec % - || write(dosfs, fsinfo, sizeof fsinfo) % - != sizeof fsinfo) { % + || write(dosfs, iofsinfo, iosize) % + != (ssize_t)iosize) { % perror("Unable to write FSInfo"); % return FSFATAL; % } % ret = FSBOOTMOD; % +over: ; % } else % boot->FSInfo = 0; % @@ -156,8 +209,9 @@ % if (lseek(dosfs, boot->Backup * boot->BytesPerSec, SEEK_SET) % != boot->Backup * boot->BytesPerSec % - || read(dosfs, backup, sizeof backup) != sizeof backup) { % + || read(dosfs, iobackup, secsize) != (ssize_t)secsize) { % perror("could not read backup bootblock"); % return FSFATAL; % } % + memcpy(backup, iobackup, sizeof backup); % backup[65] = block[65]; /* XXX */ % if (memcmp(block + 11, backup + 11, 79)) { % @@ -235,12 +299,18 @@ % struct bootblock *boot; % { % + u_char iofsinfo[IOSIZE]; % u_char fsinfo[2 * DOSBOOTBLOCKSIZE]; % + size_t iosize; % + u_int secsize; % % + secsize = boot->BytesPerSec; % + iosize = (secsize >= sizeof fsinfo) ? secsize : sizeof fsinfo; % if (lseek(dosfs, boot->FSInfo * boot->BytesPerSec, SEEK_SET) % != boot->FSInfo * boot->BytesPerSec % - || read(dosfs, fsinfo, sizeof fsinfo) != sizeof fsinfo) { % + || read(dosfs, iofsinfo, iosize) != (ssize_t)iosize) { % perror("could not read fsinfo block"); % return FSFATAL; % } % + memcpy(fsinfo, iofsinfo, sizeof fsinfo); % fsinfo[0x1e8] = (u_char)boot->FSFree; % fsinfo[0x1e9] = (u_char)(boot->FSFree >> 8); % @@ -251,8 +321,8 @@ % fsinfo[0x1ee] = (u_char)(boot->FSNext >> 16); % fsinfo[0x1ef] = (u_char)(boot->FSNext >> 24); % + memcpy(iofsinfo, fsinfo, sizeof fsinfo); % if (lseek(dosfs, boot->FSInfo * boot->BytesPerSec, SEEK_SET) % != boot->FSInfo * boot->BytesPerSec % - || write(dosfs, fsinfo, sizeof fsinfo) % - != sizeof fsinfo) { % + || write(dosfs, iofsinfo, iosize) != (ssize_t)iosize) { % perror("Unable to write FSInfo"); % return FSFATAL; Bruce
Created attachment 145599 [details] Updated Bruce's patch to apply on FreeBSD 10 (and 11) Update bde's patch for 10-stable. Only compile-tested so far.
(In reply to Pedro F. Giffuni from comment #2) FWIW, I tested the patch with a 32K cluster size, which according to Microsoft is the maximum default cluster size for fat32: https://support.microsoft.com/en-us/kb/192322?wa=wsignin1.0 Partition size Cluster size ------------------------------------- 512 MB to 8,191 MB 4 KB 8,192 MB to 16,383 MB 8 KB 16,384 MB to 32,767 MB 16 KB Larger than 32,768 MB 32 KB (Of course FAT16 accepts bigger values). I am able to run fsck on the filesystem before and after the patch, however, the filesystem is OK so fsck is actually doing nothing on both cases.
For bugs matching the following conditions: - Status == In Progress - Assignee == "bugs@FreeBSD.org" - Last Modified Year <= 2017 Do - Set Status to "Open"
(In reply to Pedro F. Giffuni from comment #3) I think this was already fixed by Doug in r273865 (2014-10-30)? Note that the current code still assumes that the sector is less or equal to 4KiB, which may become an issue when we have a native sector size that is greater than bpbBytesPerSec, but to fix that we need a more through overhaul.