From: Aleksa Sarai <cyphar@cyphar.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
Jeff Layton <jlayton@kernel.org>,
"J. Bruce Fields" <bfields@fieldses.org>,
David Howells <dhowells@redhat.com>,
Shuah Khan <shuah@kernel.org>,
Shuah Khan <skhan@linuxfoundation.org>,
Christian Brauner <christian@brauner.io>,
Eric Biederman <ebiederm@xmission.com>,
Andy Lutomirski <luto@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Alexei Starovoitov <ast@kernel.org>,
Kees Cook <keescook@chromium.org>, Jann Horn <jannh@google.com>,
Tycho Andersen <tycho@tycho.ws>,
David Drysdale <drysdale@google.com>,
Chanho Min <chanho.min@lge.com>, Oleg Nesterov <oleg@redhat.com>,
Aleksa Sarai <asarai@suse.de>,
Linus Torvalds <torvalds@linux-foundation.org>,
containers@lists.linux-foundation.org,
alpha <linux-alpha@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
linux-arch <linux-arch@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
linux-ia64@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
linux-m68k <linux-m68k@lists.linux-m68k.org>,
linux-mips@vger.kernel.org,
Parisc List <linux-parisc@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
linux-s390 <linux-s390@vger.kernel.org>,
Linux-sh list <linux-sh@vger.kernel.org>,
linux-xtensa@linux-xtensa.org,
sparclinux <sparclinux@vger.kernel.org>
Subject: Re: [PATCH v9 08/10] open: openat2(2) syscall
Date: Fri, 19 Jul 2019 02:12:31 +1000 [thread overview]
Message-ID: <20190718161231.xcno272nvqpln3wj@yavin> (raw)
In-Reply-To: <CAK8P3a33rGhPDFfRBAQyLTMG_WoEgX_toDgWR2O7rSwxKsZG+w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5083 bytes --]
On 2019-07-18, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sat, Jul 6, 2019 at 5:00 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> > diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> > index 9e7704e44f6d..1703d048c141 100644
> > --- a/arch/alpha/kernel/syscalls/syscall.tbl
> > +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> > @@ -461,6 +461,7 @@
> > 530 common getegid sys_getegid
> > 531 common geteuid sys_geteuid
> > 532 common getppid sys_getppid
> > +533 common openat2 sys_openat2
> > # all other architectures have common numbers for new syscall, alpha
> > # is the exception.
> > 534 common pidfd_send_signal sys_pidfd_send_signal
>
> My plan here was to add new syscalls in the same order as everwhere else,
> just with the number 110 higher. In the long run, I hope we can automate
> this.
Alright, I will adjust this.
> > diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> > index aaf479a9e92d..4ad262698396 100644
> > --- a/arch/arm/tools/syscall.tbl
> > +++ b/arch/arm/tools/syscall.tbl
> > @@ -447,3 +447,4 @@
> > 431 common fsconfig sys_fsconfig
> > 432 common fsmount sys_fsmount
> > 433 common fspick sys_fspick
> > +434 common openat2 sys_openat2
>
> 434 is already used in linux-next, I suggest you use 437 (Palmer
> just submitted fchmodat4, which could become 436).
437 sounds good to me.
> > +/**
> > + * Arguments for how openat2(2) should open the target path. If @extra is zero,
> > + * then openat2(2) is identical to openat(2).
> > + *
> > + * @flags: O_* flags (unknown flags ignored).
> > + * @mode: O_CREAT file mode (ignored otherwise).
> > + * @upgrade_mask: restrict how the O_PATH may be re-opened (ignored otherwise).
> > + * @resolve: RESOLVE_* flags (-EINVAL on unknown flags).
> > + * @reserved: reserved for future extensions, must be zeroed.
> > + */
> > +struct open_how {
> > + __u32 flags;
> > + union {
> > + __u16 mode;
> > + __u16 upgrade_mask;
> > + };
> > + __u16 resolve;
> > + __u64 reserved[7]; /* must be zeroed */
> > +};
>
> We can have system calls with up to six arguments on all architectures, so
> this could still be done more conventionally without the indirection: like
>
> long openat2(int dfd, const char __user * filename, int flags, mode_t
> mode_mask, __u16 resolve);
>
> In fact, that seems similar enough to the existing openat() that I think
> you could also just add the fifth argument to the existing call when
> a newly defined flag is set, similarly to how we only use the 'mode'
> argument when O_CREAT or O_TMPFILE are set.
I considered doing this (and even had a preliminary version of it), but
I discovered that I was not in favour of this idea -- once I started to
write tests using it -- for a few reasons:
1. It doesn't really allow for clean extension for a future 6th
argument (because you are using up O_* flags to signify "use the
next argument", and O_* flags don't give -EINVAL if they're
unknown). Now, yes you can do the on-start runtime check that
everyone does -- but I've never really liked having to do it.
Having reserved padding for later extensions (that is actually
checked and gives -EINVAL) matches more modern syscall designs.
2. I really was hoping that the variadic openat(2) could be done away
using this union setup (Linus said he didn't like it, and suggested
using something like 'struct stat' as an argument for openat(2) --
though personally I am not sure I would personally like to use an
interface like that).
3. In order to avoid wasting a syscall argument for mode/mask you need
to either have something like your suggested mode_mask (which makes
the syscall arguments less consistent) or have some sort of
mode-like argument that is treated specially (which is really awful
on multiple levels -- this one I also tried and even wrote my
original tests using). And in both cases, the shims for
open{,at}(2) are somewhat less clean.
All of that being said, I'd be happy to switch to whatever you think
makes the most sense. As long as it's possible to get an O_PATH with
RESOLVE_IN_ROOT set, I'm happy.
> > --- a/include/linux/syscalls.h
> > +++ b/include/linux/syscalls.h
>
> This file seems to lack a declaration for the system call, which means it
> will cause a build failure on some architectures, e.g. arch/arc/kernel/sys.c:
>
> #define __SYSCALL(nr, call) [nr] = (call),
> void *sys_call_table[NR_syscalls] = {
> [0 ... NR_syscalls-1] = sys_ni_syscall,
> #include <asm/unistd.h>
> };
Thanks, I will fix this.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2019-07-18 16:12 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-06 14:57 [PATCH v9 00/10] namei: openat2(2) path resolution restrictions Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 01/10] namei: obey trailing magic-link DAC permissions Aleksa Sarai
2019-07-12 4:14 ` Al Viro
2019-07-12 6:36 ` Al Viro
2019-07-12 12:20 ` Aleksa Sarai
2019-07-12 13:10 ` Al Viro
2019-07-14 7:11 ` Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 02/10] procfs: switch magic-link modes to be more sane Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 03/10] open: O_EMPTYPATH: procfs-less file descriptor re-opening Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 04/10] namei: split out nd->dfd handling to dirfd_path_init Aleksa Sarai
2019-07-12 4:20 ` Al Viro
2019-07-12 12:07 ` Aleksa Sarai
2019-07-12 12:12 ` Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags Aleksa Sarai
2019-07-12 4:33 ` Al Viro
2019-07-12 10:57 ` Aleksa Sarai
2019-07-12 12:39 ` Al Viro
2019-07-12 12:55 ` Al Viro
2019-07-12 13:25 ` Al Viro
2019-07-12 15:00 ` Al Viro
2019-07-13 2:41 ` Al Viro
2019-07-14 3:58 ` Al Viro
2019-07-16 8:03 ` Aleksa Sarai
2019-07-14 7:00 ` Aleksa Sarai
2019-07-14 14:36 ` Al Viro
2019-07-18 3:17 ` Aleksa Sarai
2019-07-14 10:31 ` Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 06/10] namei: LOOKUP_IN_ROOT: chroot-like path resolution Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 07/10] namei: aggressively check for nd->root escape on ".." resolution Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 08/10] open: openat2(2) syscall Aleksa Sarai
2019-07-18 14:48 ` Rasmus Villemoes
2019-07-18 15:21 ` Aleksa Sarai
2019-07-18 15:10 ` Arnd Bergmann
2019-07-18 16:12 ` Aleksa Sarai [this message]
2019-07-18 21:29 ` Arnd Bergmann
2019-07-19 2:12 ` Dmitry V. Levin
2019-07-19 10:29 ` Christian Brauner
2019-07-19 1:59 ` Dmitry V. Levin
2019-07-19 2:19 ` Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 09/10] kselftest: save-and-restore errno to allow for %m formatting Aleksa Sarai
2019-07-06 14:57 ` [PATCH v9 10/10] selftests: add openat2(2) selftests Aleksa Sarai
2019-07-08 1:15 ` Michael Ellerman
2019-07-08 5:47 ` Aleksa Sarai
2019-07-12 15:11 ` [PATCH v9 00/10] namei: openat2(2) path resolution restrictions Al Viro
2019-07-12 15:32 ` Aleksa Sarai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190718161231.xcno272nvqpln3wj@yavin \
--to=cyphar@cyphar.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=asarai@suse.de \
--cc=ast@kernel.org \
--cc=bfields@fieldses.org \
--cc=chanho.min@lge.com \
--cc=christian@brauner.io \
--cc=containers@lists.linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=drysdale@google.com \
--cc=ebiederm@xmission.com \
--cc=jannh@google.com \
--cc=jlayton@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux-xtensa@linux-xtensa.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=luto@kernel.org \
--cc=oleg@redhat.com \
--cc=shuah@kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=sparclinux@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=tycho@tycho.ws \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).