From owner-freebsd-hackers@FreeBSD.ORG Sat May 9 13:34:15 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BAC8E106566B; Sat, 9 May 2009 13:34:15 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id 4FBA78FC25; Sat, 9 May 2009 13:34:15 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtps (TLSv1:AES256-SHA:256) (Exim 4.63 (FreeBSD)) (envelope-from ) id 1M2mgv-000AxY-Bz; Sat, 09 May 2009 16:34:13 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id n49DYAO4056502 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 9 May 2009 16:34:10 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3) with ESMTP id n49DYARD053527; Sat, 9 May 2009 16:34:10 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id n49DY9jJ053526; Sat, 9 May 2009 16:34:09 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 9 May 2009 16:34:09 +0300 From: Kostik Belousov To: Jilles Tjoelker Message-ID: <20090509133409.GL1948@deviant.kiev.zoral.com.ua> References: <4A03A202.2050101@freebsd.org> <20090508201203.GJ1948@deviant.kiev.zoral.com.ua> <20090508230531.GA8413@stack.nl> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EVQB43uFympFEer4" Content-Disposition: inline In-Reply-To: <20090508230531.GA8413@stack.nl> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.94.2, clamav-milter version 0.94.2 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1M2mgv-000AxY-Bz ad16b0ca0ad2a7fe86d6aae237c438a2 X-Terabit: YES Cc: "'freebsd-hackers@freebsd.org'" , Tim Kientzle Subject: Re: fdescfs brokenness X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 May 2009 13:34:16 -0000 --EVQB43uFympFEer4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, May 09, 2009 at 01:05:31AM +0200, Jilles Tjoelker wrote: > On Fri, May 08, 2009 at 11:12:03PM +0300, Kostik Belousov wrote: > > On Thu, May 07, 2009 at 08:07:46PM -0700, Tim Kientzle wrote: > > > Colin Percival recently pointed out some issues > > > with tar and fdescfs. Part of the problem > > > here is tar; I need to rethink some of the > > > traversal logic. >=20 > > > But fdescfs is really wonky: >=20 > > > * This is a nit, but: ls /dev/fd/18 should not > > > return EBADF; it should return ENOENT, just > > > like any other reference to a non-existent filename. > > > (Just because a filename reflects a file descriptor > > > does not mean it is a file descriptor.) > > This is a traditional behaviour for fdescfs. According to man page, > > open("dev/fd/N") shall be equivalent to fcntl(N, F_DUPFD, 0). > > Solaris behaviour is the same. >=20 > On open, yes, but stat behaves differently on a Solaris 10 machine here. > A valid but unallocated fd number will still stat as a character > device, like an allocated fd. >=20 > % ls -l /dev/fd/0 /dev/fd/999 > crw-rw-rw- 1 root root 320, 0 May 9 00:06 /dev/fd/0 > crw-rw-rw- 1 root root 320, 999 May 9 00:06 /dev/fd/999 Yes, this makes sense. >=20 > By the way, both FreeBSD and Solaris also behave strangely if you try to > access fd numbers 1<<32 or higher. The strangeness is purely comsetical, in my opinion, but still. >=20 > Linux seems to behave strangely as well: the fds show up as symlinks, > some of which do not contain valid file names but can still be opened. > However, a command like > { read x <&5; read y which shows the first three lines under FreeBSD and Solaris, > shows the first line three times under Linux, so apparently it does not > duplicate file descriptors (at least in some cases). > I think it should be possible to write a directory walker program using > only standard interfaces. For standard-compiant fses, yes. AFAIR POSIX does not make any claims for the whole namespace. I did liked the idea of turning fdescfs nodes to character devices for stat(2). Besides fixing the issue, it also prevents recursive descent into the vfs, preventing the LOR. Being there, added check for the overflow. diff --git a/sys/fs/fdescfs/fdesc_vnops.c b/sys/fs/fdescfs/fdesc_vnops.c index d1788ae..9846357 100644 --- a/sys/fs/fdescfs/fdesc_vnops.c +++ b/sys/fs/fdescfs/fdesc_vnops.c @@ -264,7 +264,7 @@ fdesc_lookup(ap) struct thread *td =3D cnp->cn_thread; struct file *fp; int nlen =3D cnp->cn_namelen; - u_int fd; + u_int fd, fd1; int error; struct vnode *fvp; =20 @@ -296,7 +296,12 @@ fdesc_lookup(ap) error =3D ENOENT; goto bad; } - fd =3D 10 * fd + *pname++ - '0'; + fd1 =3D 10 * fd + *pname++ - '0'; + if (fd1 < fd) { + error =3D ENOENT; + goto bad; + } + fd =3D fd1; } =20 if ((error =3D fget(td, fd, &fp)) !=3D 0) @@ -383,78 +388,34 @@ fdesc_getattr(ap) { struct vnode *vp =3D ap->a_vp; struct vattr *vap =3D ap->a_vap; - struct thread *td =3D curthread; - struct file *fp; - struct stat stb; - u_int fd; - int error =3D 0; + + vap->va_mode =3D S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH; + vap->va_fileid =3D VTOFDESC(vp)->fd_ix; + vap->va_uid =3D 0; + vap->va_gid =3D 0; + vap->va_blocksize =3D DEV_BSIZE; + vap->va_atime.tv_sec =3D boottime.tv_sec; + vap->va_atime.tv_nsec =3D 0; + vap->va_mtime =3D vap->va_atime; + vap->va_ctime =3D vap->va_mtime; + vap->va_gen =3D 0; + vap->va_flags =3D 0; + vap->va_bytes =3D 0; + vap->va_filerev =3D 0; =20 switch (VTOFDESC(vp)->fd_type) { case Froot: - vap->va_mode =3D S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH; vap->va_type =3D VDIR; vap->va_nlink =3D 2; vap->va_size =3D DEV_BSIZE; - vap->va_fileid =3D VTOFDESC(vp)->fd_ix; - vap->va_uid =3D 0; - vap->va_gid =3D 0; - vap->va_blocksize =3D DEV_BSIZE; - vap->va_atime.tv_sec =3D boottime.tv_sec; - vap->va_atime.tv_nsec =3D 0; - vap->va_mtime =3D vap->va_atime; - vap->va_ctime =3D vap->va_mtime; - vap->va_gen =3D 0; - vap->va_flags =3D 0; vap->va_rdev =3D NODEV; - vap->va_bytes =3D 0; - vap->va_filerev =3D 0; break; =20 case Fdesc: - fd =3D VTOFDESC(vp)->fd_fd; - - if ((error =3D fget(td, fd, &fp)) !=3D 0) - return (error); - - bzero(&stb, sizeof(stb)); - error =3D fo_stat(fp, &stb, td->td_ucred, td); - fdrop(fp, td); - if (error =3D=3D 0) { - vap->va_type =3D IFTOVT(stb.st_mode); - vap->va_mode =3D stb.st_mode; - if (vap->va_type =3D=3D VDIR) - vap->va_mode &=3D ~(S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH); - vap->va_nlink =3D 1; - vap->va_flags =3D 0; - vap->va_bytes =3D stb.st_blocks * stb.st_blksize; - vap->va_fileid =3D VTOFDESC(vp)->fd_ix; - vap->va_size =3D stb.st_size; - vap->va_blocksize =3D stb.st_blksize; - vap->va_rdev =3D stb.st_rdev; - - /* - * If no time data is provided, use the current time. - */ - if (stb.st_atimespec.tv_sec =3D=3D 0 && - stb.st_atimespec.tv_nsec =3D=3D 0) - nanotime(&stb.st_atimespec); - - if (stb.st_ctimespec.tv_sec =3D=3D 0 && - stb.st_ctimespec.tv_nsec =3D=3D 0) - nanotime(&stb.st_ctimespec); - - if (stb.st_mtimespec.tv_sec =3D=3D 0 && - stb.st_mtimespec.tv_nsec =3D=3D 0) - nanotime(&stb.st_mtimespec); - - vap->va_atime =3D stb.st_atimespec; - vap->va_mtime =3D stb.st_mtimespec; - vap->va_ctime =3D stb.st_ctimespec; - vap->va_uid =3D stb.st_uid; - vap->va_gid =3D stb.st_gid; - vap->va_gen =3D 0; - vap->va_filerev =3D 0; - } + vap->va_type =3D VCHR; + vap->va_nlink =3D 1; + vap->va_size =3D 0; + vap->va_rdev =3D makedev(0, vap->va_fileid); break; =20 default: @@ -462,9 +423,8 @@ fdesc_getattr(ap) break; } =20 - if (error =3D=3D 0) - vp->v_type =3D vap->va_type; - return (error); + vp->v_type =3D vap->va_type; + return (0); } =20 static int --EVQB43uFympFEer4 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkoFhlEACgkQC3+MBN1Mb4jE9gCg5ZbgXvHn7Zwgu46EUGRXGDWu y0gAoLS6B9Epko3WxW786qfz/uj56tw5 =EjQ6 -----END PGP SIGNATURE----- --EVQB43uFympFEer4--