svn commit: r206098 - head/sys/fs/msdosfs

[Andriy Gapon]

on 02/04/2010 18:22 Andriy Gapon said the following:
> Author: avg
> Date: Fri Apr  2 15:22:23 2010
> New Revision: 206098
> URL: http://svn.freebsd.org/changeset/base/206098
> 
> Log:
>   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.

Now that this change is MFC-ed I want to state for the record that I am not sure
that this is the best or even correct fix.
Couple of things are mixed here:
1. Constraint violation of original FAT spec with implied 512-byte sector size.
2. Violation of FreeBSD buffer cache limit on block size.

The fix might be overly defensive.
Perhaps msdos code needs to be taught how to properly handle bytes/cluster ratio
greater than MAXBSIZE.  Those two are not related and msdos code should know
better than to make too large reads, it should split them in smaller reads of
allowed size.


>   Reported by:	Fabian Keil <freebsd-listen at fabiankeil.de>,
>   		Paul B. Mahol <onemda at gmail.com>
>   Discussed with:	bde, kib
>   MFC after:	1 week
>   X-ToDo:		fix newfs_msdos
> 
> Modified:
>   head/sys/fs/msdosfs/msdosfs_vfsops.c
> 
> Modified: head/sys/fs/msdosfs/msdosfs_vfsops.c
> ==============================================================================
> --- head/sys/fs/msdosfs/msdosfs_vfsops.c	Fri Apr  2 15:12:31 2010	(r206097)
> +++ head/sys/fs/msdosfs/msdosfs_vfsops.c	Fri Apr  2 15:22:23 2010	(r206098)
> @@ -580,6 +580,7 @@ mountmsdosfs(struct vnode *devvp, struct
>  	  || (pmp->pm_BytesPerSec & (pmp->pm_BytesPerSec - 1))
>  	  || (pmp->pm_HugeSectors == 0)
>  	  || (pmp->pm_FATsecs == 0)
> +	  || (SecPerClust * pmp->pm_BlkPerSec > MAXBSIZE / DEV_BSIZE)
>  	) {
>  		error = EINVAL;
>  		goto error_exit;


-- 
Andriy Gapon