From owner-freebsd-fs@FreeBSD.ORG Sat Mar 12 20:05:38 2011 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4E216106564A; Sat, 12 Mar 2011 20:05:38 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id BEE568FC08; Sat, 12 Mar 2011 20:05:36 +0000 (UTC) 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 p2CK5Uh8087596 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 12 Mar 2011 22:05:30 +0200 (EET) (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.4/8.14.4) with ESMTP id p2CK5UPg045543; Sat, 12 Mar 2011 22:05:30 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p2CK5Ues045542; Sat, 12 Mar 2011 22:05:30 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 12 Mar 2011 22:05:30 +0200 From: Kostik Belousov To: Jilles Tjoelker Message-ID: <20110312200530.GA78089@deviant.kiev.zoral.com.ua> References: <20110312170123.GT78089@deviant.kiev.zoral.com.ua> <20110312193131.GA97300@stack.nl> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="FPVPwrFAzg68gQ6L" Content-Disposition: inline In-Reply-To: <20110312193131.GA97300@stack.nl> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-fs@freebsd.org, freebsd-standards@freebsd.org Subject: Re: open(O_NOFOLLOW) error when encountered symlink X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Mar 2011 20:05:38 -0000 --FPVPwrFAzg68gQ6L Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Mar 12, 2011 at 08:31:32PM +0100, Jilles Tjoelker wrote: > On Sat, Mar 12, 2011 at 07:01:23PM +0200, Kostik Belousov wrote: > > Hello, > > I noted the following discussion and commits in the gnu tar repository: >=20 > > http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00080.html > >=20 > > http://git.savannah.gnu.org/cgit/tar.git/commit/?id=3D1584b72ff271e7f82= 6dd64d7a1c7cd2f66504acb > > http://git.savannah.gnu.org/cgit/tar.git/commit/?id=3D649b747913d2b289e= 904b5f1d222af886acd209c >=20 > > The issue is that in case of open(path, O_NOFOLLOW), when path is naming > > a symlink, FreeBSD returns EMLINK error. On the other hand, the POSIX > > requirement is absolutely clear that it shall be ELOOP. >=20 > > I found FreeBSD commit r35088 that specifically changed the error code > > from the required ELOOP to EMLINK. I doubt that somebody can remember > > a reason for the change done more then 12 years ago. >=20 > In fact that change was done hours after the new ELOOP error. Do you mean r35083, r35084 and r35085 ? >=20 > > Anybody have strong objections against the patch below ? >=20 > Although it loses information (ELOOP may also be caused by the directory > prefix), I think we should make the change. >=20 > Please move the error condition in open.2 below the other [ELOOP] error. >=20 > usr.bin/cmp relies on the EMLINK error for the -h option and needs some > adjustment. If ELOOP is returned and O_NOFOLLOW is in use, it needs to > check using lstat() if the file is a symlink. This is quite serious argument against the change, IMO. Also, after your comment, I found the similar code in contrib/xz, and somewhat doubtful resolve_symlink() in rcs sources. I am recalling my proposal. Just for record, below is the updated patch diff --git a/lib/libc/sys/open.2 b/lib/libc/sys/open.2 index deca8bc..1c9095d 100644 --- a/lib/libc/sys/open.2 +++ b/lib/libc/sys/open.2 @@ -304,6 +304,9 @@ is specified or .Dv O_APPEND is not specified. .It Bq Er ELOOP +.Dv O_NOFOLLOW +was specified and the target is a symbolic link. +.It Bq Er ELOOP Too many symbolic links were encountered in translating the pathname. .It Bq Er EISDIR The named file is a directory, and the arguments specify @@ -318,9 +321,6 @@ is specified and the named file would reside on a read-= only file system. The process has already reached its limit for open file descriptors. .It Bq Er ENFILE The system file table is full. -.It Bq Er EMLINK -.Dv O_NOFOLLOW -was specified and the target is a symbolic link. .It Bq Er ENXIO The named file is a character special or block special file, and the device associated with this special file diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c index 7b5cad1..c7985ef 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -194,7 +194,7 @@ restart: vp =3D ndp->ni_vp; } if (vp->v_type =3D=3D VLNK) { - error =3D EMLINK; + error =3D ELOOP; goto bad; } if (vp->v_type =3D=3D VSOCK) { diff --git a/usr.bin/cmp/cmp.c b/usr.bin/cmp/cmp.c index f3ac717..84514d0 100644 --- a/usr.bin/cmp/cmp.c +++ b/usr.bin/cmp/cmp.c @@ -107,11 +107,14 @@ main(int argc, char *argv[]) fd1 =3D 0; file1 =3D "stdin"; } - else if ((fd1 =3D open(file1, oflag, 0)) < 0 && errno !=3D EMLINK) { - if (!sflag) - err(ERR_EXIT, "%s", file1); - else - exit(ERR_EXIT); + else if ((fd1 =3D open(file1, oflag, 0)) < 0) { + if (errno !=3D ELOOP || stat(file1, &sb1) =3D=3D -1 || + !S_ISLNK(sb1.st_mode)) { + if (!sflag) + err(ERR_EXIT, "%s", file1); + else + exit(ERR_EXIT); + } } if (strcmp(file2 =3D argv[1], "-") =3D=3D 0) { if (special) @@ -121,11 +124,14 @@ main(int argc, char *argv[]) fd2 =3D 0; file2 =3D "stdin"; } - else if ((fd2 =3D open(file2, oflag, 0)) < 0 && errno !=3D EMLINK) { - if (!sflag) - err(ERR_EXIT, "%s", file2); - else - exit(ERR_EXIT); + else if ((fd2 =3D open(file2, oflag, 0)) < 0) { + if (errno !=3D ELOOP || stat(file2, &sb2) =3D=3D -1 || + !S_ISLNK(sb2.st_mode)) { + if (!sflag) + err(ERR_EXIT, "%s", file2); + else + exit(ERR_EXIT); + } } =20 skip1 =3D argc > 2 ? strtol(argv[2], NULL, 0) : 0; --FPVPwrFAzg68gQ6L Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAk170goACgkQC3+MBN1Mb4gdCQCgn2kTliOfDaprReESRbgbMJYS aLUAn0YcEQTDRloHKRzF4s6SjlCH38V+ =+Epr -----END PGP SIGNATURE----- --FPVPwrFAzg68gQ6L--