[PATCH] Implement fchmodat4 system call

[PATCH] Implement fchmodat4 system call

[William Orr]

Hey,

Currently, Linux's fchmodat(2) doesn't honor the flags argument. To bring it
more in-line with POSIX and other implementations, this patch adds fchmodat4,
which honors the flags argument, and implements the same flags as fchownat(2).

This makes it possible to chmod a file without following symlinks, without
having to call open(2) on a file with O_NOFOLLOW. This is heavily based off
of Andrew Ayer's work in 2012, and is sent with his permission. Let me know
if this can be applied.

Please CC me, since I'm not subscribed to this list.

Thanks,
William Orr

[PATCH 1/2] Implement fchmodat4 syscall

[William Orr]

Adds fchmodat4 which more closely matches POSIX by taking 4 arguments,
including the flags argument. flags are the same as fchownat(2), implementing
both AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH

Based heavily off of Andrew Ayer's patch from 2012.
---
 fs/open.c                | 19 +++++++++++++++++--
 include/linux/syscalls.h |  2 ++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 98e5a52..00dd0f7 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -504,6 +504,9 @@ static int chmod_common(struct path *path, umode_t mode)
 	struct iattr newattrs;
 	int error;
 
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+
 	error = mnt_want_write(path->mnt);
 	if (error)
 		return error;
@@ -541,9 +544,21 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd, umode_t, mode)
 
 SYSCALL_DEFINE3(fchmodat, int, dfd, const char __user *, filename, umode_t, mode)
 {
+	return sys_fchmodat4(dfd, filename, mode, 0);
+}
+
+SYSCALL_DEFINE4(fchmodat4, int, dfd, const char __user *, filename, umode_t, mode, int, flags)
+{
 	struct path path;
 	int error;
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
+	unsigned int lookup_flags;
+
+	if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
+		return -EINVAL;
+
+	lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	if (flags & AT_EMPTY_PATH)
+		lookup_flags |= LOOKUP_EMPTY;
 retry:
 	error = user_path_at(dfd, filename, lookup_flags, &path);
 	if (!error) {
@@ -559,7 +574,7 @@ retry:
 
 SYSCALL_DEFINE2(chmod, const char __user *, filename, umode_t, mode)
 {
-	return sys_fchmodat(AT_FDCWD, filename, mode);
+	return sys_fchmodat4(AT_FDCWD, filename, mode, 0);
 }
 
 static int chown_common(struct path *path, uid_t user, gid_t group)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 76d1e38..d6e9602 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -769,6 +769,8 @@ asmlinkage long sys_futimesat(int dfd, const char __user *filename,
 asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode);
 asmlinkage long sys_fchmodat(int dfd, const char __user * filename,
 			     umode_t mode);
+asmlinkage long sys_fchmodat4(int dfd, const char __user * filename,
+			     umode_t mode, int flags);
 asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t user,
 			     gid_t group, int flag);
 asmlinkage long sys_openat(int dfd, const char __user *filename, int flags,
-- 
2.1.0

[PATCH 2/2] Define syscall number for fchmodat4

[William Orr]

---
 arch/x86/syscalls/syscall_32.tbl  | 1 +
 arch/x86/syscalls/syscall_64.tbl  | 1 +
 include/uapi/asm-generic/unistd.h | 4 +++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index ef8187f..cc8ada8 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -365,3 +365,4 @@
 356	i386	memfd_create		sys_memfd_create
 357	i386	bpf			sys_bpf
 358	i386	execveat		sys_execveat			stub32_execveat
+359	i386	fchmodat4		sys_fchmodat4
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 9ef32d5..bbf8c6b 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -329,6 +329,7 @@
 320	common	kexec_file_load		sys_kexec_file_load
 321	common	bpf			sys_bpf
 322	64	execveat		stub_execveat
+323	common	fchmodat4		sys_fchmodat4
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index e016bd9..6e362a2 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
 __SYSCALL(__NR_bpf, sys_bpf)
 #define __NR_execveat 281
 __SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
+#define __NR_fchmodat4 282
+__SYSCALL(__NR_fchmodat4, sys_fchmodat4);
 
 #undef __NR_syscalls
-#define __NR_syscalls 282
+#define __NR_syscalls 283
 
 /*
  * All syscalls below here should go away really,
-- 
2.1.0

Re: [PATCH] Implement fchmodat4 system call

[Michael Kerrisk]

[CC += linux-api@vger.kernel.org]

Hello William,

Since this is a kernel-user-space API change, please CC linux-api@.
The kernel source file Documentation/SubmitChecklist notes that all
Linux kernel patches that change userspace interfaces should be CCed
to linux-api@vger.kernel.org, so that the various parties who are
interested in API changes are informed. For further information, see
https://www.kernel.org/doc/man-pages/linux-api-ml.html

Aside from that, I've added a few other people/lists to CC that seem
likely to be relevant. Some additional comments below.

On Mon, May 11, 2015 at 4:07 AM, William Orr <will@worrbase.com> wrote:
> Hey,
>
> Currently, Linux's fchmodat(2) doesn't honor the flags argument. To bring it
> more in-line with POSIX and other implementations, this patch adds fchmodat4,
> which honors the flags argument, and implements the same flags as fchownat(2).
>
> This makes it possible to chmod a file without following symlinks, without
> having to call open(2) on a file with O_NOFOLLOW. This is heavily based off
> of Andrew Ayer's work in 2012, and is sent with his permission. Let me know
> if this can be applied.
>
> Please CC me, since I'm not subscribed to this list.

This seems an obviously sensible addition, and it was an obvious
mistake not to have a 'flags' argument in the original system call
(https://lwn.net/Articles/585415/). As well as getting us a proper
POSIX compliant API (and one that is consistent with other
implementations, such as FreeBSD), it allows for future behavior
expansion via the 'flags' argument.

Cheers,

Michael

-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

Re: [PATCH 1/2] Implement fchmodat4 syscall

[Michael Kerrisk]

[expanding CC]

On Mon, May 11, 2015 at 4:07 AM, William Orr <will@worrbase.com> wrote:
> Adds fchmodat4 which more closely matches POSIX by taking 4 arguments,
> including the flags argument. flags are the same as fchownat(2), implementing
> both AT_SYMLINK_NOFOLLOW and AT_EMPTY_PATH
>
> Based heavily off of Andrew Ayer's patch from 2012.
> ---
>  fs/open.c                | 19 +++++++++++++++++--
>  include/linux/syscalls.h |  2 ++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/open.c b/fs/open.c
> index 98e5a52..00dd0f7 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -504,6 +504,9 @@ static int chmod_common(struct path *path, umode_t mode)
>         struct iattr newattrs;
>         int error;
>
> +       if (S_ISLNK(inode->i_mode))
> +               return -EOPNOTSUPP;
> +
>         error = mnt_want_write(path->mnt);
>         if (error)
>                 return error;
> @@ -541,9 +544,21 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd, umode_t, mode)
>
>  SYSCALL_DEFINE3(fchmodat, int, dfd, const char __user *, filename, umode_t, mode)
>  {
> +       return sys_fchmodat4(dfd, filename, mode, 0);
> +}
> +
> +SYSCALL_DEFINE4(fchmodat4, int, dfd, const char __user *, filename, umode_t, mode, int, flags)
> +{
>         struct path path;
>         int error;
> -       unsigned int lookup_flags = LOOKUP_FOLLOW;
> +       unsigned int lookup_flags;
> +
> +       if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
> +               return -EINVAL;
> +
> +       lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> +       if (flags & AT_EMPTY_PATH)
> +               lookup_flags |= LOOKUP_EMPTY;
>  retry:
>         error = user_path_at(dfd, filename, lookup_flags, &path);
>         if (!error) {
> @@ -559,7 +574,7 @@ retry:
>
>  SYSCALL_DEFINE2(chmod, const char __user *, filename, umode_t, mode)
>  {
> -       return sys_fchmodat(AT_FDCWD, filename, mode);
> +       return sys_fchmodat4(AT_FDCWD, filename, mode, 0);
>  }
>
>  static int chown_common(struct path *path, uid_t user, gid_t group)
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 76d1e38..d6e9602 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -769,6 +769,8 @@ asmlinkage long sys_futimesat(int dfd, const char __user *filename,
>  asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode);
>  asmlinkage long sys_fchmodat(int dfd, const char __user * filename,
>                              umode_t mode);
> +asmlinkage long sys_fchmodat4(int dfd, const char __user * filename,
> +                            umode_t mode, int flags);
>  asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t user,
>                              gid_t group, int flag);
>  asmlinkage long sys_openat(int dfd, const char __user *filename, int flags,
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

Re: [PATCH 2/2] Define syscall number for fchmodat4

[Michael Kerrisk]

[expanding CC]

On Mon, May 11, 2015 at 4:07 AM, William Orr <will@worrbase.com> wrote:
> ---
>  arch/x86/syscalls/syscall_32.tbl  | 1 +
>  arch/x86/syscalls/syscall_64.tbl  | 1 +
>  include/uapi/asm-generic/unistd.h | 4 +++-
>  3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
> index ef8187f..cc8ada8 100644
> --- a/arch/x86/syscalls/syscall_32.tbl
> +++ b/arch/x86/syscalls/syscall_32.tbl
> @@ -365,3 +365,4 @@
>  356    i386    memfd_create            sys_memfd_create
>  357    i386    bpf                     sys_bpf
>  358    i386    execveat                sys_execveat                    stub32_execveat
> +359    i386    fchmodat4               sys_fchmodat4
> diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
> index 9ef32d5..bbf8c6b 100644
> --- a/arch/x86/syscalls/syscall_64.tbl
> +++ b/arch/x86/syscalls/syscall_64.tbl
> @@ -329,6 +329,7 @@
>  320    common  kexec_file_load         sys_kexec_file_load
>  321    common  bpf                     sys_bpf
>  322    64      execveat                stub_execveat
> +323    common  fchmodat4               sys_fchmodat4
>
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index e016bd9..6e362a2 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
>  __SYSCALL(__NR_bpf, sys_bpf)
>  #define __NR_execveat 281
>  __SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
> +#define __NR_fchmodat4 282
> +__SYSCALL(__NR_fchmodat4, sys_fchmodat4);
>
>  #undef __NR_syscalls
> -#define __NR_syscalls 282
> +#define __NR_syscalls 283
>
>  /*
>   * All syscalls below here should go away really,
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/