2015-05-11 02:14:20

by William Orr

[permalink] [raw]
Subject: [PATCH] Implement fchmodat4 system call

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


2015-05-11 02:14:26

by William Orr

[permalink] [raw]
Subject: [PATCH 1/2] Implement fchmodat4 syscall

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

2015-05-11 02:14:32

by William Orr

[permalink] [raw]
Subject: [PATCH 2/2] Define syscall number for fchmodat4

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

Subject: Re: [PATCH] Implement fchmodat4 system call

[CC += [email protected]]

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 [email protected], 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 <[email protected]> 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/

Subject: Re: [PATCH 1/2] Implement fchmodat4 syscall

[expanding CC]

On Mon, May 11, 2015 at 4:07 AM, William Orr <[email protected]> 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 [email protected]
> 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/

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

[expanding CC]

On Mon, May 11, 2015 at 4:07 AM, William Orr <[email protected]> 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 [email protected]
> 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/