Hi,
sys/msdosfs is vulnerable to a division by zero when certain FAT
file systems are mounted. This happens due to an overflow of an
unsigned 8 bit value (sectors per cluster).
How to repeat (BEWARE, KERNEL WILL CRASH!!):
# dd if=/dev/zero of=nuke.iso bs=1K count=100
# vnconfig vnd0c nuke.iso
# newfs_msdos -S 1024 -c 128 vnd0c
### BEWARE, DON'T DO THIS!
# mount -t msdos /dev/vnd0c /mnt
So what happens?
In msdosfs_vfsops, the sector size (-S) is divided by DEV_BSIZE (512)
to retrieve "physical blocks in a sector". In this example it's 2.
Next critical part is the amount of sectors per cluster (-c). This
value is stored as an unsigned 8 bit in file system. In this example
it's 128.
Internally, the code seems to be quite dependent on the idea that
sectors must be of block size, i.e. 512.
So it "corrects" the amount of sectors in cluster (and many other
values by the way) through a multiplication of physical blocks in
a logical sector: 128 * 2 = 256
That is too large for unsigned 8 bit, the value flips to 0. All
previous checks didn't spot it and eventually, that value is used in
a division: Good bye. ;)
My fix:
The largest allowed sector size is 4096 bytes. A division by 512
can therefore be at worst 8. It's enough to increase SecPerClust to
an unsigned 16 bit value.
Also, don't accept file systems with sector sizes beyond 4096 bytes.
By the way: The introduced cast is important because the header files
state that "bpbSecPerClust" is signed...
Tobias
PS: Yes, this FAT file system as generated by newfs_msdos is invalid;
a cluster shall never be larger than 655356 bytes. Yet, a kernel
shouldn't crash on it.
Index: msdosfs_vfsops.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v
retrieving revision 1.66
diff -u -p -r1.66 msdosfs_vfsops.c
--- msdosfs_vfsops.c 18 Jun 2014 17:24:46 -0000 1.66
+++ msdosfs_vfsops.c 20 Jun 2014 19:07:39 -0000
@@ -284,7 +284,7 @@ msdosfs_mountfs(struct vnode *devvp, str
struct byte_bpb50 *b50;
struct byte_bpb710 *b710;
extern struct vnode *rootvp;
- u_int8_t SecPerClust;
+ uint16_t SecPerClust;
int ronly, error, bmapsiz;
uint32_t fat_max_clusters;
@@ -331,7 +331,7 @@ msdosfs_mountfs(struct vnode *devvp, str
* bootsector. Copy in the dos 5 variant of the bpb then fix up
* the fields that are different between dos 5 and dos 3.3.
*/
- SecPerClust = b50->bpbSecPerClust;
+ SecPerClust = (u_int8_t)b50->bpbSecPerClust;
pmp->pm_BytesPerSec = getushort(b50->bpbBytesPerSec);
pmp->pm_ResSectors = getushort(b50->bpbResSectors);
pmp->pm_FATs = b50->bpbFATs;
@@ -378,12 +378,13 @@ msdosfs_mountfs(struct vnode *devvp, str
/*
* More sanity checks:
* MSDOSFS sectors per cluster: >0 && power of 2
- * MSDOSFS sector size: >= DEV_BSIZE && power of 2
+ * MSDOSFS sector size: >= DEV_BSIZE && <= 4096 && power of 2
* HUGE sector count: >0
* FAT sectors: >0
*/
if ((SecPerClust == 0) || (SecPerClust & (SecPerClust - 1)) ||
(pmp->pm_BytesPerSec < DEV_BSIZE) ||
+ (pmp->pm_BytesPerSec > 4096) ||
(pmp->pm_BytesPerSec & (pmp->pm_BytesPerSec - 1)) ||
(pmp->pm_HugeSectors == 0) || (pmp->pm_FATsecs == 0)) {
error = EINVAL;
