Date: Sat, 12 Mar 2011 22:05:30 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Jilles Tjoelker <jilles@stack.nl> Cc: freebsd-fs@freebsd.org, freebsd-standards@freebsd.org Subject: Re: open(O_NOFOLLOW) error when encountered symlink Message-ID: <20110312200530.GA78089@deviant.kiev.zoral.com.ua> In-Reply-To: <20110312193131.GA97300@stack.nl> References: <20110312170123.GT78089@deviant.kiev.zoral.com.ua> <20110312193131.GA97300@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110312200530.GA78089>