From owner-freebsd-current Thu Feb 15 14:57:32 2001 Delivered-To: freebsd-current@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 5837E37B491; Thu, 15 Feb 2001 14:57:27 -0800 (PST) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.1/8.11.1) with SMTP id f1FMvDh48130; Thu, 15 Feb 2001 17:57:14 -0500 (EST) (envelope-from robert@fledge.watson.org) Date: Thu, 15 Feb 2001 17:57:13 -0500 (EST) From: Robert Watson X-Sender: robert@fledge.watson.org To: Martin Blapp Cc: adrian@freebsd.org, current@freebsd.org Subject: Re: Fix for mountpath lenght In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG As has been pointed out, this is simply incorrect due to an attempt to use kernel string operators on a string in the kernel address space. Generally speaking, it's a bad idea to explicitly perform string activities on userland strings, instead, to rely on the bounds checking in copyinstr() and related calls. The namei has all appropriate bounds checks it needs for normal nul-termianted string reading from userland. If you need to place an artificially low bound on the string length for a path name that is to be read in, and reject based on the pathname length, then namei() is probably not the right call to pull in the string in the first place. For many reasons, including that if shared memory is in use, then there's a race condition under SMP that can let malicious processes update the path between the two checks as they are non-atomic. What is it you're trying to accomplish here, exactly? Is it prevent paths >MNAMELEN to be used as targets of mounting operations? Or is it to truncate strings reported via statfs to some arbitrary bound? If it's the former, I think we need to be eliminating that need and moving to the latter. If it's the latter, then we need to perform the truncation wherefer f_mntfromname is set. That appears to be, btw, in vfs_mount() for each filesystem, and in vfs_rootmountalloc(). A quick glance at UFS seems to indicate that the MNAMELEN limit is already enforced there, and that this is the right solution. On Thu, 15 Feb 2001, Martin Blapp wrote: > > In mount.h, we have a #define MNAMELEN 80 > > and in struct statfs {} we have: > > char f_mntonname[MNAMELEN]; /* directory on which mounted */ > > but the kernel does no check to see if the mountpath is longer > than MNAMELEN, it just accepts it ? It's impossible to umount(8) > it, because umount(8) does not like to unmount some device which > does not belong to the mountpoint. > > --- vfs_syscalls.c Sun Nov 26 03:30:05 2000 > +++ vfs_syscalls.c.new Thu Feb 15 18:22:13 2001 > @@ -140,6 +140,8 @@ > /* > * Get vnode to be covered > */ > + if (strlen(SCARG(uap, path)) > MNAMELEN) > + return (ENAMETOOLONG); > NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_USERSPACE, > SCARG(uap, path), p); > if ((error = namei(&nd)) != 0) > > Martin Blapp, mb@imp.ch > ------------------------------------------------ > Improware AG, UNIX solution and service provider > Zurlindenstrasse 29, 4133 Pratteln, Switzerland > Phone: +41 79 370 26 05, Fax: +41 61 826 93 01 > ------------------------------------------------ > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-current" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message