open(O_NOFOLLOW) error when encountered symlink

[Jilles Tjoelker]

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:

> http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00080.html
> 
> http://git.savannah.gnu.org/cgit/tar.git/commit/?id=1584b72ff271e7f826dd64d7a1c7cd2f66504acb
> http://git.savannah.gnu.org/cgit/tar.git/commit/?id=649b747913d2b289e904b5f1d222af886acd209c

> 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.

> 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.

In fact that change was done hours after the new ELOOP error.

> Anybody have strong objections against the patch below ?

Although it loses information (ELOOP may also be caused by the directory
prefix), I think we should make the change.

Please move the error condition in open.2 below the other [ELOOP] error.

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.

> diff --git a/lib/libc/sys/open.2 b/lib/libc/sys/open.2
> index deca8bc..20877b5 100644
> --- a/lib/libc/sys/open.2
> +++ b/lib/libc/sys/open.2
> @@ -318,7 +318,7 @@ 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
> +.It Bq Er ELOOP
>  .Dv O_NOFOLLOW
>  was specified and the target is a symbolic link.
>  .It Bq Er ENXIO
> 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 = ndp->ni_vp;
>  	}
>  	if (vp->v_type == VLNK) {
> -		error = EMLINK;
> +		error = ELOOP;
>  		goto bad;
>  	}
>  	if (vp->v_type == VSOCK) {

-- 
Jilles Tjoelker