v3 -> v4:
- Rename the syscall: getumask becomes umask2.
- Add flags parameter, with one flag (UMASK_GET_MASK).
- Expand the rationale for this change in the first commit message.
- Add a selftest.
- Retest everything.
--------------------
It's not possible to read the process umask without also modifying it,
which is what umask(2) does. A library cannot read umask safely,
especially if the main program might be multithreaded.
This patch series adds a new system call "umask2". This adds a flags
parameter. Specifying flags=UMASK_GET_MASK allows the umask of the
current process to be read without modifying it.
This leaves open the possibility in future of adding a per-thread
umask, set or read with other flags. This is not implemented.
Another approach to this has been attempted before, adding something
to /proc, although it didn't go anywhere. See:
http://comments.gmane.org/gmane.linux.kernel/1292109
Another way to solve this would be to add a thread-safe getumask to
glibc. Since glibc could own the mutex, this would permit libraries
linked to this glibc to read umask safely. I should also note that
man-pages documents getumask(3), but no version of glibc has ever
implemented it.
Rich.
Before this change it was not possible to read the umask of the
current process from a shared library in a way that is safe if the
process uses multiple threads.
Define a variation of the umask system call with a flags parameter.
If flags=0 it behaves like the old umask call. If flags=UMASK_GET_MASK
it reads the umask of the current process without modifying it.
Signed-off-by: Richard W.M. Jones <[email protected]>
---
include/linux/syscalls.h | 1 +
include/uapi/asm-generic/fcntl.h | 2 ++
include/uapi/asm-generic/unistd.h | 4 +++-
kernel/sys.c | 18 +++++++++++++++++-
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index d795472..939e969 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -659,6 +659,7 @@ asmlinkage long sys_prlimit64(pid_t pid, unsigned int resource,
struct rlimit64 __user *old_rlim);
asmlinkage long sys_getrusage(int who, struct rusage __user *ru);
asmlinkage long sys_umask(int mask);
+asmlinkage long sys_umask2(int mask, int flags);
asmlinkage long sys_msgget(key_t key, int msgflg);
asmlinkage long sys_msgsnd(int msqid, struct msgbuf __user *msgp,
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063eff..f6c5e5a 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -217,4 +217,6 @@ struct flock64 {
};
#endif
+#define UMASK_GET_MASK 1 /* umask2 should only return current umask */
+
#endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 2622b33..8a61471 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -717,9 +717,11 @@ __SYSCALL(__NR_membarrier, sys_membarrier)
__SYSCALL(__NR_mlock2, sys_mlock2)
#define __NR_copy_file_range 285
__SYSCALL(__NR_copy_file_range, sys_copy_file_range)
+#define __NR_umask2 286
+__SYSCALL(__NR_umask2, sys_umask2)
#undef __NR_syscalls
-#define __NR_syscalls 286
+#define __NR_syscalls 287
/*
* All syscalls below here should go away really,
diff --git a/kernel/sys.c b/kernel/sys.c
index cf8ba54..cf60091 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1643,12 +1643,28 @@ COMPAT_SYSCALL_DEFINE2(getrusage, int, who, struct compat_rusage __user *, ru)
}
#endif
-SYSCALL_DEFINE1(umask, int, mask)
+static int do_umask(int mask, int flags)
{
+ if (flags & ~UMASK_GET_MASK)
+ return -EINVAL;
+
+ if (flags & UMASK_GET_MASK)
+ return current_umask();
+
mask = xchg(¤t->fs->umask, mask & S_IRWXUGO);
return mask;
}
+SYSCALL_DEFINE1(umask, int, mask)
+{
+ return do_umask(mask, 0);
+}
+
+SYSCALL_DEFINE2(umask2, int, mask, int, flags)
+{
+ return do_umask(mask, flags);
+}
+
static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
{
struct fd exe;
--
2.7.4
Signed-off-by: Richard W.M. Jones <[email protected]>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index b30dd81..ccf75ba 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -386,3 +386,4 @@
377 i386 copy_file_range sys_copy_file_range
378 i386 preadv2 sys_preadv2
379 i386 pwritev2 sys_pwritev2
+380 i386 umask2 sys_umask2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index cac6d17..e566c64 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -335,6 +335,7 @@
326 common copy_file_range sys_copy_file_range
327 64 preadv2 sys_preadv2
328 64 pwritev2 sys_pwritev2
+329 common umask2 sys_umask2
#
# x32-specific system call numbers start at 512 to avoid cache impact
--
2.7.4
Signed-off-by: Richard W.M. Jones <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/fs/.gitignore | 1 +
tools/testing/selftests/fs/Makefile | 9 ++++
tools/testing/selftests/fs/umask2-tests.c | 77 +++++++++++++++++++++++++++++++
4 files changed, 88 insertions(+)
create mode 100644 tools/testing/selftests/fs/.gitignore
create mode 100644 tools/testing/selftests/fs/Makefile
create mode 100644 tools/testing/selftests/fs/umask2-tests.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index b04afc3..9e2eb24 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -4,6 +4,7 @@ TARGETS += cpu-hotplug
TARGETS += efivarfs
TARGETS += exec
TARGETS += firmware
+TARGETS += fs
TARGETS += ftrace
TARGETS += futex
TARGETS += ipc
diff --git a/tools/testing/selftests/fs/.gitignore b/tools/testing/selftests/fs/.gitignore
new file mode 100644
index 0000000..057dced
--- /dev/null
+++ b/tools/testing/selftests/fs/.gitignore
@@ -0,0 +1 @@
+umask2-tests
diff --git a/tools/testing/selftests/fs/Makefile b/tools/testing/selftests/fs/Makefile
new file mode 100644
index 0000000..6f231d7
--- /dev/null
+++ b/tools/testing/selftests/fs/Makefile
@@ -0,0 +1,9 @@
+BINARIES := umask2-tests
+
+all: $(BINARIES)
+
+clean:
+ $(RM) $(BINARIES)
+
+include ../lib.mk
+
diff --git a/tools/testing/selftests/fs/umask2-tests.c b/tools/testing/selftests/fs/umask2-tests.c
new file mode 100644
index 0000000..3e01575
--- /dev/null
+++ b/tools/testing/selftests/fs/umask2-tests.c
@@ -0,0 +1,77 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <syscall.h>
+#include <linux/unistd.h>
+
+#ifndef UMASK_GET_MASK
+#define UMASK_GET_MASK 1
+#endif
+
+static int umask2_(int mask, int flags)
+{
+#ifdef __NR_umask2
+ return syscall(__NR_umask2, mask, flags);
+#else
+ errno = ENOSYS;
+ return -1;
+#endif
+}
+
+int main(int argc, char **argv)
+{
+ int r;
+
+ /* umask2 available in current kernel? */
+ r = umask2_(0, UMASK_GET_MASK);
+ if (r == -1 && errno == ENOSYS) {
+ fprintf(stderr,
+ "umask2 not available in current kernel or headers, skipping test\n");
+ exit(0);
+ }
+
+ /* Check that old umask still works. */
+ r = umask(022);
+ if (r == -1) {
+ perror("umask");
+ exit(1);
+ }
+
+ /* Call umask2 to emulate old umask. */
+ r = umask2_(023, 0);
+ if (r == -1) {
+ perror("umask2");
+ exit(1);
+ }
+ if (r != 022) {
+ fprintf(stderr, "umask2: expected %o, got %o\n", 022, r);
+ exit(1);
+ }
+
+ /* Call umask2 to read current umask without modifying it. */
+ r = umask2_(0777, UMASK_GET_MASK);
+ if (r == -1) {
+ perror("umask2");
+ exit(1);
+ }
+ if (r != 023) {
+ fprintf(stderr, "umask2: expected %o, got %o\n", 023, r);
+ exit(1);
+ }
+
+ /* Call it again to make sure we didn't modify umask. */
+ r = umask2_(0777, UMASK_GET_MASK);
+ if (r == -1) {
+ perror("umask2");
+ exit(1);
+ }
+ if (r != 023) {
+ fprintf(stderr, "umask2: expected %o, got %o\n", 023, r);
+ exit(1);
+ }
+
+ exit(0);
+}
--
2.7.4
UMASK(2) Linux Programmer's Manual UMASK(2)
NAME
umask, umask2 - get and set file mode creation mask
SYNOPSIS
#include <sys/types.h>
#include <sys/stat.h>
mode_t umask(mode_t mask);
#define _GNU_SOURCE
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
mode_t umask2(mode_t mask, int flags);
DESCRIPTION
umask() sets the calling process's file mode creation mask (umask) to
mask & 0777 (i.e., only the file permission bits of mask are used), and
returns the previous value of the mask.
If flags is 0, then umask2() is the same as umask().
If flags is UMASK_GET_MASK then umask2() ignores the mask parameter and
returns the process's current umask. The process's current mask is not
modified in this case.
The umask is used by open(2), mkdir(2), and other system calls that
create files to modify the permissions placed on newly created files or
directories. Specifically, permissions in the umask are turned off
from the mode argument to open(2) and mkdir(2).
Alternatively, if the parent directory has a default ACL (see acl(5)),
the umask is ignored, the default ACL is inherited, the permission bits
are set based on the inherited ACL, and permission bits absent in the
mode argument are turned off. For example, the following default ACL
is equivalent to a umask of 022:
u::rwx,g::r-x,o::r-x
Combining the effect of this default ACL with a mode argument of 0666
(rw-rw-rw-), the resulting file permissions would be 0644 (rw-r--r--).
The constants that should be used to specify mask are described under
stat(2).
The typical default value for the process umask is S_IWGRP | S_IWOTH
(octal 022). In the usual case where the mode argument to open(2) is
specified as:
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH
(octal 0666) when creating a new file, the permissions on the resulting
file will be:
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
(because 0666 & ~022 = 0644; i.e., rw-r--r--).
RETURN VALUE
The umask() system call always succeeds and the previous value of the
mask is returned.
The umask2() system call returns the process's current umask on suc‐
cess. On error it returns -1 and sets errno appropriately.
CONFORMING TO
SVr4, 4.3BSD, POSIX.1-2001.
NOTES
A child process created via fork(2) inherits its parent's umask. The
umask is left unchanged by execve(2).
The umask setting also affects the permissions assigned to POSIX IPC
objects (mq_open(3), sem_open(3), shm_open(3)), FIFOs (mkfifo(3)), and
UNIX domain sockets (unix(7)) created by the process. The umask does
not affect the permissions assigned to System V IPC objects created by
the process (using msgget(2), semget(2), shmget(2)).
SEE ALSO
chmod(2), mkdir(2), open(2), stat(2), acl(5)
COLOPHON
This page is part of release 4.00 of the Linux man-pages project. A
description of the project, information about reporting bugs, and the
latest version of this page, can be found at
http://www.kernel.org/doc/man-pages/.
Linux 2016-04-13 UMASK(2)
On 04/13/16 12:05, Richard W.M. Jones wrote:
> v3 -> v4:
>
> - Rename the syscall: getumask becomes umask2.
>
> - Add flags parameter, with one flag (UMASK_GET_MASK).
>
> - Expand the rationale for this change in the first commit message.
>
>
> It's not possible to read the process umask without also modifying it,
> which is what umask(2) does. A library cannot read umask safely,
> especially if the main program might be multithreaded.
>
I wouldn't say "if"; that is the case when it matters.
I have to say I'm skeptic to the need for umask2() as opposed to
getumask(). I would also like to be able to get the umask of another
process, which would argue for adding it to /proc anyway.
-hpa
Hi Richard,
[auto build test WARNING on v4.6-rc3]
[also build test WARNING on next-20160413]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Richard-W-M-Jones/vfs-Define-new-syscall-umask2-formerly-getumask/20160414-030818
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
<stdin>:1298:2: warning: #warning syscall userfaultfd not implemented [-Wcpp]
<stdin>:1301:2: warning: #warning syscall membarrier not implemented [-Wcpp]
<stdin>:1304:2: warning: #warning syscall mlock2 not implemented [-Wcpp]
<stdin>:1307:2: warning: #warning syscall copy_file_range not implemented [-Wcpp]
<stdin>:1310:2: warning: #warning syscall preadv2 not implemented [-Wcpp]
<stdin>:1313:2: warning: #warning syscall pwritev2 not implemented [-Wcpp]
>> <stdin>:1316:2: warning: #warning syscall umask2 not implemented [-Wcpp]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
* H. Peter Anvin:
> I have to say I'm skeptic to the need for umask2() as opposed to
> getumask().
I find the extension with a set-the-thread umask somewhat unlikely.
How would a potential per-thread umask interact with CLONE_FS?
Have a per-thread umask that, when active, overrides the global
one, similar to what uselocale provides? That seems rather messy,
and I'm not aware of any precedent.
On Wed, Apr 13, 2016 at 10:45:05PM +0200, Florian Weimer wrote:
> * H. Peter Anvin:
>
> > I have to say I'm skeptic to the need for umask2() as opposed to
> > getumask().
>
> I find the extension with a set-the-thread umask somewhat unlikely.
> How would a potential per-thread umask interact with CLONE_FS?
> Have a per-thread umask that, when active, overrides the global
> one, similar to what uselocale provides? That seems rather messy,
> and I'm not aware of any precedent.
The flags parameter is for extensions we can't envisage now ...
... although since umask has been around since Unix V6 in 1978, we
might be waiting a long time.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
Hi Richard,
On Wed, 13 Apr 2016 20:05:33 +0100 "Richard W.M. Jones" <[email protected]> wrote:
>
> It's not possible to read the process umask without also modifying it,
> which is what umask(2) does. A library cannot read umask safely,
> especially if the main program might be multithreaded.
I was wondering if you really need to read the umask, or would just a
"ignore umask" flag to open{,at} do what you want?
--
Cheers,
Stephen Rothwell
On Thu, Apr 14, 2016 at 09:09:38AM +1000, Stephen Rothwell wrote:
> Hi Richard,
>
> On Wed, 13 Apr 2016 20:05:33 +0100 "Richard W.M. Jones" <[email protected]> wrote:
> >
> > It's not possible to read the process umask without also modifying it,
> > which is what umask(2) does. A library cannot read umask safely,
> > especially if the main program might be multithreaded.
>
> I was wondering if you really need to read the umask, or would just a
> "ignore umask" flag to open{,at} do what you want?
This would be very useful, although I think being able to read umask
is also useful.
---
FWIW I am currently developing a patch to add umask to
/proc/PID/status. Will post it shortly once I've tested it properly.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top