From owner-freebsd-fs@FreeBSD.ORG Wed Sep 25 03:07:11 2013 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id D8F8C396; Wed, 25 Sep 2013 03:07:11 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 8751220D8; Wed, 25 Sep 2013 03:07:11 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 95612781EA1; Wed, 25 Sep 2013 12:43:17 +1000 (EST) Date: Wed, 25 Sep 2013 12:43:16 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Kirk McKusick Subject: Re: kern/vfs_mount.c vfs_donmount() checks of MFSNAMELEN In-Reply-To: <201309240942.r8O9gKna000550@chez.mckusick.com> Message-ID: <20130925120938.C995@besplex.bde.org> References: <201309240942.r8O9gKna000550@chez.mckusick.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=YYGEuWhf c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=oauJmmNaXYEA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=kMyxM1bEWncA:10 a=FUyLlYIt_FMkiQppwXAA:9 a=CjuIK1q_8ugA:10 Cc: freebsd-fs X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Sep 2013 03:07:11 -0000 On Tue, 24 Sep 2013, Kirk McKusick wrote: >> Date: Tue, 24 Sep 2013 10:48:45 +1000 (EST) >> From: Bruce Evans >> ... >> Apart from the existence of this check being a bug, it is still off by >> 1 for fstypename and off by 2 for fspathlen. It used to be off by 2 >> for both. >> ... >> The existence of the MNAMELEN check is a bug. It breaks mounting of file >> systems when fspathlen is too long to fit in the copy of the pathname >> that will be returned by statfs() if statfs() is ever called (except with >> the off-by-2 bug it breaks even for pathnames that fit). mount() worked >> correctly in old versions of FreeBSD -- pathnames were only limited by >> MAXPATHLEN, except where they are copied to struct statfs and possibly >> ... > I agree with Bruce's arguments and his proposed solution. Notably the > change should be: > > Index: sys/kern/vfs_mount.c > =================================================================== > --- sys/kern/vfs_mount.c (revision 255831) > +++ sys/kern/vfs_mount.c (working copy) > @@ -656,7 +656,7 @@ > * variables will fit in our mp buffers, including the > * terminating NUL. > */ > - if (fstypelen >= MFSNAMELEN - 1 || fspathlen >= MNAMELEN - 1) { > + if (fstypelen > MFSNAMELEN || fspathlen > MNAMELEN) { > error = ENAMETOOLONG; > goto bail; > } Also (regarding the excessive limiting), ENAMETOOLONG is a documented error for fspath in mount.2, but the documentation only mentions the usual {PATH_MAX} and {NAME_MAX} limits, where as usual {PATH_MAX} and {NAME_MAX} are not mentioned but their values less 1 are hard-coded as 1023 and 255 respectively, so the above error is undocumented even for fspath. For fstype, the error is just undocumented since the documentation is only about path names but fstype is not a path name. Bruce