Date: Wed, 25 Sep 2013 12:43:16 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Kirk McKusick <mckusick@mckusick.com> Cc: freebsd-fs <freebsd-fs@FreeBSD.org> Subject: Re: kern/vfs_mount.c vfs_donmount() checks of MFSNAMELEN Message-ID: <20130925120938.C995@besplex.bde.org> In-Reply-To: <201309240942.r8O9gKna000550@chez.mckusick.com> References: <201309240942.r8O9gKna000550@chez.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 24 Sep 2013, Kirk McKusick wrote:
>> Date: Tue, 24 Sep 2013 10:48:45 +1000 (EST)
>> From: Bruce Evans <brde@optusnet.com.au>
>> ...
>> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130925120938.C995>