Date: Tue, 13 Apr 2010 00:02:55 +0300 From: Andriy Gapon <avg@freebsd.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r206098 - head/sys/fs/msdosfs Message-ID: <4BC38A7F.4040003@freebsd.org> In-Reply-To: <201004021522.o32FMNgu095467@svn.freebsd.org> References: <201004021522.o32FMNgu095467@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@fabiankeil.de>, > Paul B. Mahol <onemda@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4BC38A7F.4040003>