From owner-svn-src-all@FreeBSD.ORG Mon Apr 12 21:03:00 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 150851065705; Mon, 12 Apr 2010 21:03:00 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id E02888FC25; Mon, 12 Apr 2010 21:02:58 +0000 (UTC) Received: from porto.topspin.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id AAA04401; Tue, 13 Apr 2010 00:02:56 +0300 (EEST) (envelope-from avg@freebsd.org) Received: from localhost.topspin.kiev.ua ([127.0.0.1]) by porto.topspin.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1O1QmW-000Iu9-Cx; Tue, 13 Apr 2010 00:02:56 +0300 Message-ID: <4BC38A7F.4040003@freebsd.org> Date: Tue, 13 Apr 2010 00:02:55 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.24 (X11/20100321) MIME-Version: 1.0 To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201004021522.o32FMNgu095467@svn.freebsd.org> In-Reply-To: <201004021522.o32FMNgu095467@svn.freebsd.org> X-Enigmail-Version: 0.96.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Subject: Re: svn commit: r206098 - head/sys/fs/msdosfs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Apr 2010 21:03:00 -0000 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 , > Paul B. Mahol > 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