And again...

Could have saved myself time by checking FreeBSD in detail first.
They fixed the issue by adding a simple check:

According to FAT specs, the size of a cluster shall not exceed 64 KB.
That is even a rare case, 32 KB shouldn't be crossed for compatibility
reasons.

Therefore, it's enough to check if we would have more than 128
"physical blocks" around.  If that is the case, the filesystem is
invalid and we won't overflow or crash.

FreeBSD's commit message:

mountmsdosfs: reject too high value of bytes per cluster

Bytes per cluster are calcuated as bytes per sector times sectors per
cluster.  Too high value can overflow an internal variable with type
that can hold only values in valid range.  Trying to use a wider type
results in an attempt to read more than MAXBSIZE at once, a panic.
Unfortunately, it is FreeBSD newfs_msdos that  produces filesystems
with invalid parameters for certain types of media.


Basically their version from revision 206098, adjusted to fit into our
style of the if-block.


Tobias

Index: msdosfs_vfsops.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v
retrieving revision 1.66
diff -u -p -u -p -r1.66 msdosfs_vfsops.c
--- msdosfs_vfsops.c    18 Jun 2014 17:24:46 -0000      1.66
+++ msdosfs_vfsops.c    23 Jun 2014 17:51:37 -0000
@@ -385,7 +385,8 @@ msdosfs_mountfs(struct vnode *devvp, str
        if ((SecPerClust == 0) || (SecPerClust & (SecPerClust - 1)) ||
            (pmp->pm_BytesPerSec < DEV_BSIZE) ||
            (pmp->pm_BytesPerSec & (pmp->pm_BytesPerSec - 1)) ||
-           (pmp->pm_HugeSectors == 0) || (pmp->pm_FATsecs == 0)) {
+           (pmp->pm_HugeSectors == 0) || (pmp->pm_FATsecs == 0) ||
+           (SecPerClust * pmp->pm_BlkPerSec > MAXBSIZE / DEV_BSIZE)) {
                error = EINVAL;
                goto error_exit;
        }               

Reply via email to