Re: [PATCH] Implement fchmodat4 system call

Re: [PATCH] Implement fchmodat4 system call

[Michael Kerrisk]

[CC += linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-o9VNkMWGZsBWk0Htik3J/w@public.gmane.org> 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-o9VNkMWGZsBWk0Htik3J/w@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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/