[PATCH] flags parameter for linkat

[PATCH] flags parameter for linkat

[Ulrich Drepper]

I'm currently at the POSIX meeting and one thing covered was the
incompatibility of Linux's link() with the POSIX definition.  The
difference is the treatment of symbolic links in the destination
name.  Linux does not follow symlinks, POSIX requires it does.

Even somebody thinks this is a good default behavior we cannot
change this because it would break the ABI.  But the fact remains
that some application might want this behavior.

We have one chance to help implementing this without breaking the
behavior.  For this we could use the new linkat interface which
would need a new flags parameter.  If the new parameter is
AT_SYMLINK_FOLLOW the new behavior could be invoked.

I do not want to introduce such a patch now.  But we could add the
parameter now, just don't use it.  The patch below would do this.
Can we get this late patch applied before the release more or less
fixes the syscall API?

Signed-Off-By: Ulrich Drepper <drepper@redhat.com>


--- fs/namei.c	2006-02-23 05:51:45.000000000 -0800
+++ fs/namei.c-new	2006-02-23 05:58:31.000000000 -0800
@@ -2232,13 +2232,17 @@
  * and other special files.  --ADM
  */
 asmlinkage long sys_linkat(int olddfd, const char __user *oldname,
-			   int newdfd, const char __user *newname)
+			   int newdfd, const char __user *newname,
+			   int flags)
 {
 	struct dentry *new_dentry;
 	struct nameidata nd, old_nd;
 	int error;
 	char * to;
 
+	if (flags != 0)
+		return -EINVAL;
+
 	to = getname(newname);
 	if (IS_ERR(to))
 		return PTR_ERR(to);
@@ -2271,7 +2275,7 @@
 
 asmlinkage long sys_link(const char __user *oldname, const char __user *newname)
 {
-	return sys_linkat(AT_FDCWD, oldname, AT_FDCWD, newname);
+	return sys_linkat(AT_FDCWD, oldname, AT_FDCWD, newname, 0);
 }
 
 /*
--- ./arch/s390/kernel/compat_wrapper.S-new	2006-02-23 06:00:12.000000000 -0800
+++ ./arch/s390/kernel/compat_wrapper.S	2006-02-23 05:51:32.000000000 -0800
@@ -1552,7 +1552,6 @@
 	llgtr	%r3,%r3			# const char *
 	lgfr	%r4,%r4			# int
 	llgtr	%r5,%r5			# const char *
-	lgfr	%r6,%r6			# int
 	jg	sys_linkat
 
 	.globl sys_symlinkat_wrapper

Re: [PATCH] flags parameter for linkat

[David Vrabel]

Ulrich Drepper wrote:
> 
> --- ./arch/s390/kernel/compat_wrapper.S-new	2006-02-23 06:00:12.000000000 -0800
> +++ ./arch/s390/kernel/compat_wrapper.S	2006-02-23 05:51:32.000000000 -0800
> @@ -1552,7 +1552,6 @@
>  	llgtr	%r3,%r3			# const char *
>  	lgfr	%r4,%r4			# int
>  	llgtr	%r5,%r5			# const char *
> -	lgfr	%r6,%r6			# int
>  	jg	sys_linkat

This hunk appears to be reversed.

David Vrabel

Re: [PATCH] flags parameter for linkat

[Ralf Baechle]

On Thu, Feb 23, 2006 at 09:10:22AM -0500, Ulrich Drepper wrote:

> I'm currently at the POSIX meeting and one thing covered was the
> incompatibility of Linux's link() with the POSIX definition.  The
> difference is the treatment of symbolic links in the destination
> name.  Linux does not follow symlinks, POSIX requires it does.
> 
> Even somebody thinks this is a good default behavior we cannot
> change this because it would break the ABI.  But the fact remains
> that some application might want this behavior.
> 
> We have one chance to help implementing this without breaking the
> behavior.  For this we could use the new linkat interface which
> would need a new flags parameter.  If the new parameter is
> AT_SYMLINK_FOLLOW the new behavior could be invoked.
> 
> I do not want to introduce such a patch now.  But we could add the
> parameter now, just don't use it.  The patch below would do this.
> Can we get this late patch applied before the release more or less
> fixes the syscall API?

The number of arguments changes, so below patch would be needed for
32-bit MIPS.

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
index d83e033..2f2dc54 100644
--- a/arch/mips/kernel/scall32-o32.S
+++ b/arch/mips/kernel/scall32-o32.S
@@ -626,7 +626,7 @@ einval:	li	v0, -EINVAL
 	sys	sys_fstatat64		4
 	sys	sys_unlinkat		3
 	sys	sys_renameat		4	/* 4295 */
-	sys	sys_linkat		4
+	sys	sys_linkat		5
 	sys	sys_symlinkat		3
 	sys	sys_readlinkat		4
 	sys	sys_fchmodat		3

Re: [PATCH] flags parameter for linkat

[Christoph Hellwig]

On Thu, Feb 23, 2006 at 09:10:22AM -0500, Ulrich Drepper wrote:
> I'm currently at the POSIX meeting and one thing covered was the
> incompatibility of Linux's link() with the POSIX definition.  The
> difference is the treatment of symbolic links in the destination
> name.  Linux does not follow symlinks, POSIX requires it does.
> 
> Even somebody thinks this is a good default behavior we cannot
> change this because it would break the ABI.  But the fact remains
> that some application might want this behavior.
> 
> We have one chance to help implementing this without breaking the
> behavior.  For this we could use the new linkat interface which
> would need a new flags parameter.  If the new parameter is
> AT_SYMLINK_FOLLOW the new behavior could be invoked.
> 
> I do not want to introduce such a patch now.  But we could add the
> parameter now, just don't use it.  The patch below would do this.
> Can we get this late patch applied before the release more or less
> fixes the syscall API?

Please stop adding these crappy flags argument everywhere, they're also
creaping like a cancer through the other *at stuff.  Just make linkat
do the righ thing per posix spec for link, and then you can implement
a posix link based on it in glibc if the user compiles with XOPEN_SOURCE
or whatever.

Re: [PATCH] flags parameter for linkat

[Al Viro]

On Sat, Feb 25, 2006 at 09:16:07AM +0000, Christoph Hellwig wrote:
> Please stop adding these crappy flags argument everywhere, they're also
> creaping like a cancer through the other *at stuff.  Just make linkat
> do the righ thing per posix spec for link, and then you can implement
> a posix link based on it in glibc if the user compiles with XOPEN_SOURCE
> or whatever.

It's a bloody bad idea, since XOPEN_SOURCE is nowhere near fine-grained
enough and could be forced by any number of things.

The _real_ issue with flags is the lack of filtering.  I.e. we blindly
pass userland argument there, exposing both internal flag values _and_
flags that really shouldn't be exposed that way at all.