* Re: Can we drop upstream Linux x32 support?
From: Richard Weinberger @ 2018-12-14 15:17 UTC (permalink / raw)
To: David Laight
Cc: 'Richard Weinberger', kevin@guarana.org,
glaubitz@physik.fu-berlin.de, Andy Lutomirski, x86@kernel.org,
LKML, open list:ABI/API, H. Peter Anvin, Peter Zijlstra,
Borislav Petkov, fweimer@redhat.com, Mike Frysinger, H.J. Lu,
dalias@libc.org, x32@buildd.debian.org, Arnd Bergmann,
Will Deacon, Catalin Marinas, Linus Torvalds
In-Reply-To: <d54d7ea7c558474583dfed0d26f05ccd@AcuMS.aculab.com>
Am Freitag, 14. Dezember 2018, 15:38:53 CET schrieb David Laight:
> From: Richard Weinberger
> > Sent: 13 December 2018 09:05
> > On Thu, Dec 13, 2018 at 6:03 AM Kevin Easton <kevin@guarana.org> wrote:
> > >
> > > On Tue, Dec 11, 2018 at 11:29:14AM +0100, John Paul Adrian Glaubitz wrote:
> > > ...
> > > > I can't say anything about the syscall interface. However, what I do know
> > > > is that the weird combination of a 32-bit userland with a 64-bit kernel
> > > > interface is sometimes causing issues. For example, application code usually
> > > > expects things like time_t to be 32-bit on a 32-bit system. However, this
> > > > isn't the case for x32 which is why code fails to build.
> > >
> > > OpenBSD and NetBSD both have 64-bit time_t on 32-bit systems and have
> > > had for four or five years at this point.
> >
> > They can also do flag-day changes and break existing applications, Linux not.
>
> Not true at all.
> The binary compatibility in NetBSD is probably better than that in Linux
> and goes back a long way.
>
> For the time_t changes new system calls numbers were assigned where needed.
> The system headers and libc were updated so that recompiled code would
> use the new system calls.
>
> The only real advantage that NetBSD has is that its libc (and standard
> utilities) are released with the kernel making it much easier to get
> applications to use the new features.
>
> This was also done a very long time ago when file offsets were extended
> to 64 bits.
>
> Some of the system calls have quite a few 'compatibility' versions.
> As well as the ones for emulations of other operating systems.
> It has been possible to run copies of firefox compiled for Linux
> under NetBSD.
I stand corrected, I was under the impression that NetBSD went the same
path as OpenBSD did. Thanks for pointing this out.
Thanks,
//richard
^ permalink raw reply
* RE: Can we drop upstream Linux x32 support?
From: David Laight @ 2018-12-14 14:38 UTC (permalink / raw)
To: 'Richard Weinberger', kevin@guarana.org
Cc: glaubitz@physik.fu-berlin.de, Andy Lutomirski, x86@kernel.org,
LKML, open list:ABI/API, H. Peter Anvin, Peter Zijlstra,
Borislav Petkov, fweimer@redhat.com, Mike Frysinger, H.J. Lu,
dalias@libc.org, x32@buildd.debian.org, Arnd Bergmann,
Will Deacon, Catalin Marinas, Linus Torvalds
In-Reply-To: <CAFLxGvy2sMW1nyMpxGN7xHPdUea713g89MdE3EcUNHJDCvZ3jA@mail.gmail.com>
From: Richard Weinberger
> Sent: 13 December 2018 09:05
> On Thu, Dec 13, 2018 at 6:03 AM Kevin Easton <kevin@guarana.org> wrote:
> >
> > On Tue, Dec 11, 2018 at 11:29:14AM +0100, John Paul Adrian Glaubitz wrote:
> > ...
> > > I can't say anything about the syscall interface. However, what I do know
> > > is that the weird combination of a 32-bit userland with a 64-bit kernel
> > > interface is sometimes causing issues. For example, application code usually
> > > expects things like time_t to be 32-bit on a 32-bit system. However, this
> > > isn't the case for x32 which is why code fails to build.
> >
> > OpenBSD and NetBSD both have 64-bit time_t on 32-bit systems and have
> > had for four or five years at this point.
>
> They can also do flag-day changes and break existing applications, Linux not.
Not true at all.
The binary compatibility in NetBSD is probably better than that in Linux
and goes back a long way.
For the time_t changes new system calls numbers were assigned where needed.
The system headers and libc were updated so that recompiled code would
use the new system calls.
The only real advantage that NetBSD has is that its libc (and standard
utilities) are released with the kernel making it much easier to get
applications to use the new features.
This was also done a very long time ago when file offsets were extended
to 64 bits.
Some of the system calls have quite a few 'compatibility' versions.
As well as the ones for emulations of other operating systems.
It has been possible to run copies of firefox compiled for Linux
under NetBSD.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Bernd Petrovitsch @ 2018-12-14 14:13 UTC (permalink / raw)
To: Rich Felker, John Paul Adrian Glaubitz
Cc: Andy Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, Mike Frysinger,
H. J. Lu, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <20181213160242.GV23599@brightrain.aerifal.cx>
[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]
On 13/12/2018 17:02, Rich Felker wrote:
> On Tue, Dec 11, 2018 at 11:29:14AM +0100, John Paul Adrian Glaubitz wrote:
>> I can't say anything about the syscall interface. However, what I do know
>> is that the weird combination of a 32-bit userland with a 64-bit kernel
>> interface is sometimes causing issues. For example, application code usually
>> expects things like time_t to be 32-bit on a 32-bit system. However, this
IMHO this just historically grown (as in "it has been forever that way"
- it sounds way better in Viennese dialect though;-).
>> isn't the case for x32 which is why code fails to build.
>
> I don't see any basis for this claim about expecting time_t to be
> 32-bit. I've encountered some programs that "implicitly assume" this
> by virtue of assuming they can cast time_t to long to print it, or
> similar. IIRC this was an issue in busybox at one point; I'm not sure
> if it's been fixed. But any software that runs on non-Linux unices has
> long been corrected. If not, 2038 is sufficiently close that catching
> and correcting any such remaining bugs is more useful than covering
> them up and making the broken code work as expected.
Yup, unconditionally providing 64bit
time_t/timespec/timeval/...-equivalents with libc and syscall support
also for 32bit architectures (and deprecating all 32bit versions) should
be the way to go.
FWIW I have
---- snip ----
#if defined __x86_64__
# if defined __ILP32__ // x32
# define PRI_time_t "lld" // for time_t
# define PRI_nsec_t "lld" // for tv_nsec in struct timespec
# else // x86_64
# define PRI_time_t "ld" // for time_t
# define PRI_nsec_t "ld" // for tv_nsec in struct timespec
# endif
#else // i[3-6]68
# define PRI_time_t "ld" // for time_t
# define PRI_nsec_t "ld" // for tv_nsec in struct timespec
#endif
---- snip ----
in my userspace code for printf() and friends - I don't know how libc's
react to such a patch (and I don't care for the name of the macros as
long it's obviously clear for which type they are).
I assume/fear we won't get additional modifiers into the relevant
standards for libc types (as they are far more like pid_t, uid_t etc.).
And casting to u/intmaxptr_t to get a defined printf()-modifier doesn't
look appealing to me to "solve" such issues.
MfG,
Bernd
--
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
- Linus Torvalds
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 2513 bytes --]
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Florian Weimer @ 2018-12-14 11:42 UTC (permalink / raw)
To: Rich Felker
Cc: Catalin Marinas, Andy Lutomirski, tg, Linus Torvalds, X86 ML,
LKML, Linux API, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
Mike Frysinger, H. J. Lu, x32, Arnd Bergmann, Will Deacon
In-Reply-To: <20181213162828.GW23599@brightrain.aerifal.cx>
* Rich Felker:
> On Thu, Dec 13, 2018 at 05:04:59PM +0100, Florian Weimer wrote:
>> * Rich Felker:
>>
>> >> If the compiler can handle the zeroing, that would be great, though not
>> >> sure how (some __attribute__((zero)) which generates a type constructor
>> >> for such structure; it kind of departs from what the C language offers).
>> >
>> > The compiler fundamentally can't. At the very least it would require
>> > effective type tracking, which requires shadow memory and is even more
>> > controversial than -fstrict-aliasing (because in a sense it's a
>> > stronger version thereof).
>>
>> It's possible to do it with the right types. See _Bool on 32-bit Darwin
>> PowerPC for an example, which is four bytes instead of the usual one.
>>
>> Similarly, we could have integer types with trap representations.
>> Whether it is a good idea is a different matter, but the amount of
>> compiler magic required is actually limited.
>
> If you do this you just have LP64 with value range restricted to
> 32-bit.
You have to a type different from long int for the relevant struct
fields. This type would have zero padding.
Florian
^ permalink raw reply
* Re: [PATCH v5 1/1] signal: add pidfd_send_signal() syscall
From: Serge E. Hallyn @ 2018-12-13 22:23 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, linux-api, luto, arnd, ebiederm, serge, keescook,
jannh, akpm, oleg, cyphar, viro, linux-fsdevel, dancol, timmurray,
fweimer, tglx, x86
In-Reply-To: <20181208054059.19813-2-christian@brauner.io>
On Sat, Dec 08, 2018 at 06:40:59AM +0100, Christian Brauner wrote:
> The kill() syscall operates on process identifiers (pid). After a process
> has exited its pid can be reused by another process. If a caller sends a
> signal to a reused pid it will end up signaling the wrong process. This
> issue has often surfaced and there has been a push to address this problem [1].
>
> This patch uses file descriptors (fd) from proc/<pid> as stable handles on
> struct pid. Even if a pid is recycled the handle will not change. The fd
> can be used to send signals to the process it refers to.
> Thus, the new syscall pidfd_send_signal() is introduced to solve this
> problem. Instead of pids it operates on process fds (pidfd).
>
> /* prototype and argument /*
> long pidfd_send_signal(int pidfd, int sig, siginfo_t *info, unsigned int flags);
>
> In addition to the pidfd and signal argument it takes an additional
> siginfo_t and flags argument. If the siginfo_t argument is NULL then
> pidfd_send_signal() is equivalent to kill(<positive-pid>, <signal>). If it
> is not NULL pidfd_send_signal() is equivalent to rt_sigqueueinfo().
> The flags argument is added to allow for future extensions of this syscall.
> It currently needs to be passed as 0. Failing to do so will cause EINVAL.
>
> /* pidfd_send_signal() replaces multiple pid-based syscalls */
> The pidfd_send_signal() syscall currently takes on the job of
> rt_sigqueueinfo(2) and parts of the functionality of kill(2), Namely, when a
> positive pid is passed to kill(2). It will however be possible to also
> replace tgkill(2) and rt_tgsigqueueinfo(2) if this syscall is extended.
>
> /* sending signals to threads (tid) and process groups (pgid) */
> Specifically, the pidfd_send_signal() syscall does currently not operate on
> process groups or threads. This is left for future extensions.
> In order to extend the syscall to allow sending signal to threads and
> process groups appropriately named flags (e.g. PIDFD_TYPE_PGID, and
> PIDFD_TYPE_TID) should be added. This implies that the flags argument will
> determine what is signaled and not the file descriptor itself. Put in other
> words, grouping in this api is a property of the flags argument not a
> property of the file descriptor (cf. [13]).
> When appropriate extensions through the flags argument are added then
> pidfd_send_signal() can additionally replace the part of kill(2) which
> operates on process groups as well as the tgkill(2) and
> rt_tgsigqueueinfo(2) syscalls.
> How such an extension could be implemented has been very roughly sketched
> in [14], [15], and [16]. However, this should not be taken as a commitment
> to a particular implementation. There might be better ways to do it.
> Right now this is intentionally left out to keep this patchset as simple as
> possible (cf. [4]). For example, if a pidfd for a tid from
> /proc/<pid>/task/<tid> is passed EOPNOTSUPP will be returned to give
> userspace a way to detect when I add support for signaling to threads (cf. [10]).
>
> /* naming */
> The syscall had various names throughout iterations of this patchset:
> - procfd_signal()
> - procfd_send_signal()
> - taskfd_send_signal()
> In the last round of reviews it was pointed out that given that if the
> flags argument decides the scope of the signal instead of different types
> of fds it might make sense to either settle for "procfd_" or "pidfd_" as
> prefix. The community was willing to accept either (cf. [17] and [18]).
> Given that one developer expressed strong preference for the "pidfd_"
> prefix (cf. [13] and with other developers less opinionated about the name
> we should settle for "pidfd_" to avoid further bikeshedding.
>
> The "_send_signal" suffix was chosen to reflect the fact that the syscall
> takes on the job of multiple syscalls. It is therefore intentional that the
> name is not reminiscent of neither kill(2) nor rt_sigqueueinfo(2). Not the
> fomer because it might imply that pidfd_send_signal() is a replacement for
> kill(2), and not the latter because it is a hassle to remember the correct
> spelling - especially for non-native speakers - and because it is not
> descriptive enough of what the syscall actually does. The name
> "pidfd_send_signal" makes it very clear that its job is to send signals.
>
> /* O_PATH file descriptors */
> pidfds opened as O_PATH fds cannot be used to send signals to a process
> (cf. [2]). Signaling processes through pidfds is the equivalent of writing
> to a file. Thus, this is not an operation that operates "purely at the file
> descriptor level" as required by the open(2) manpage.
>
> /* zombies */
> Zombies can be signaled just as any other process. No special error will be
> reported since a zombie state is an unreliable state (cf. [3]). However,
> this can be added as an extension through the @flags argument if the need
> ever arises.
>
> /* cross-namespace signals */
> The patch currently enforces that the signaler and signalee either are in
> the same pid namespace or that the signaler's pid namespace is an ancestor
> of the signalee's pid namespace. This is done for the sake of simplicity
> and because it is unclear to what values certain members of struct
> siginfo_t would need to be set to (cf. [5], [6]).
>
> /* compat syscalls */
> It became clear that we would like to avoid adding compat syscalls
> (cf. [7]). The compat syscall handling is now done in kernel/signal.c
> itself by adding __copy_siginfo_from_user_generic() which lets us avoid
> compat syscalls (cf. [8]). It should be noted that the addition of
> __copy_siginfo_from_user_any() is caused by a bug in the original
> implementation of rt_sigqueueinfo(2) (cf. 12).
> With upcoming rework for syscall handling things might improve
> significantly (cf. [11]) and __copy_siginfo_from_user_any() will not gain
> any additional callers.
>
> /* testing */
> This patch was tested on x64 and x86.
>
> /* userspace usage */
> An asciinema recording for the basic functionality can be found under [9].
> With this patch a process can be killed via:
>
> #define _GNU_SOURCE
> #include <errno.h>
> #include <fcntl.h>
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <unistd.h>
>
> static inline int do_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
> unsigned int flags)
> {
> #ifdef __NR_pidfd_send_signal
> return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
> #else
> return -ENOSYS;
> #endif
> }
>
> int main(int argc, char *argv[])
> {
> int fd, ret, saved_errno, sig;
>
> if (argc < 3)
> exit(EXIT_FAILURE);
>
> fd = open(argv[1], O_DIRECTORY | O_CLOEXEC);
> if (fd < 0) {
> printf("%s - Failed to open \"%s\"\n", strerror(errno), argv[1]);
> exit(EXIT_FAILURE);
> }
>
> sig = atoi(argv[2]);
>
> printf("Sending signal %d to process %s\n", sig, argv[1]);
> ret = do_pidfd_send_signal(fd, sig, NULL, 0);
>
> saved_errno = errno;
> close(fd);
> errno = saved_errno;
>
> if (ret < 0) {
> printf("%s - Failed to send signal %d to process %s\n",
> strerror(errno), sig, argv[1]);
> exit(EXIT_FAILURE);
> }
>
> exit(EXIT_SUCCESS);
> }
>
> [1]: https://lore.kernel.org/lkml/20181029221037.87724-1-dancol@google.com/
> [2]: https://lore.kernel.org/lkml/874lbtjvtd.fsf@oldenburg2.str.redhat.com/
> [3]: https://lore.kernel.org/lkml/20181204132604.aspfupwjgjx6fhva@brauner.io/
> [4]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
> [5]: https://lore.kernel.org/lkml/20181121213946.GA10795@mail.hallyn.com/
> [6]: https://lore.kernel.org/lkml/20181120103111.etlqp7zop34v6nv4@brauner.io/
> [7]: https://lore.kernel.org/lkml/36323361-90BD-41AF-AB5B-EE0D7BA02C21@amacapital.net/
> [8]: https://lore.kernel.org/lkml/87tvjxp8pc.fsf@xmission.com/
> [9]: https://asciinema.org/a/IQjuCHew6bnq1cr78yuMv16cy
> [10]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
> [11]: https://lore.kernel.org/lkml/F53D6D38-3521-4C20-9034-5AF447DF62FF@amacapital.net/
> [12]: https://lore.kernel.org/lkml/87zhtjn8ck.fsf@xmission.com/
> [13]: https://lore.kernel.org/lkml/871s6u9z6u.fsf@xmission.com/
> [14]: https://lore.kernel.org/lkml/20181206231742.xxi4ghn24z4h2qki@brauner.io/
> [15]: https://lore.kernel.org/lkml/20181207003124.GA11160@mail.hallyn.com/
> [16]: https://lore.kernel.org/lkml/20181207015423.4miorx43l3qhppfz@brauner.io/
> [17]: https://lore.kernel.org/lkml/CAGXu5jL8PciZAXvOvCeCU3wKUEB_dU-O3q0tDw4uB_ojMvDEew@mail.gmail.com/
> [18]: https://lore.kernel.org/lkml/20181206222746.GB9224@mail.hallyn.com/
>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Hi Eric,
have you had a chance to look at the latest version?
-serge
> Cc: Jann Horn <jannh@google.com>
> Cc: Andy Lutomirsky <luto@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Christian Brauner <christian@brauner.io>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Acked-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> Changelog:
> v5:
> - s/may_signal_taskfd/access_taskfd_pidns/g
> - make it clear that process grouping is a property of the @flags argument
> Eric has argued that he would like to know when we add thread and process
> group signal support whether grouping will be a property of the file
> descriptor or the flag argument and he would oppose this until a
> commitment has been made. It seems that the cleanest strategy is to make
> grouping a property of the @flags argument.
> He also argued that in this case the prefix of the syscall should be
> "pidfd_" (cf. https://lore.kernel.org/lkml/871s6u9z6u.fsf@xmission.com/).
> - use "pidfd_" as prefix for the syscall since grouping will be a property
> of the @flags argument
> - substantial rewrite of the commit message to reflect the discussion
> v4:
> - updated asciinema to use "taskfd_" prefix
> - s/procfd_send_signal/taskfd_send_signal/g
> - s/proc_is_tgid_procfd/tgid_taskfd_to_pid/b
> - s/proc_is_tid_procfd/tid_taskfd_to_pid/b
> - s/__copy_siginfo_from_user_generic/__copy_siginfo_from_user_any/g
> - make it clear that __copy_siginfo_from_user_any() is a workaround caused
> by a bug in the original implementation of rt_sigqueueinfo()
> - when spoofing signals turn them into regular kill signals if si_code is
> set to SI_USER
> - make proc_is_t{g}id_procfd() return struct pid to allow proc_pid() to
> stay private to fs/proc/
> v3:
> - add __copy_siginfo_from_user_generic() to avoid adding compat syscalls
> - s/procfd_signal/procfd_send_signal/g
> - change type of flags argument from int to unsigned int
> - add comment about what happens to zombies
> - add proc_is_tid_procfd()
> - return EOPNOTSUPP when /proc/<pid>/task/<tid> fd is passed so userspace
> has a way of knowing that tidfds are not supported currently.
> v2:
> - define __NR_procfd_signal in unistd.h
> - wire up compat syscall
> - s/proc_is_procfd/proc_is_tgid_procfd/g
> - provide stubs when CONFIG_PROC_FS=n
> - move proc_pid() to linux/proc_fs.h header
> - use proc_pid() to grab struct pid from /proc/<pid> fd
> v1:
> - patch introduced
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> fs/proc/base.c | 20 +++-
> include/linux/proc_fs.h | 12 +++
> include/linux/syscalls.h | 3 +
> include/uapi/asm-generic/unistd.h | 4 +-
> kernel/signal.c | 141 +++++++++++++++++++++++--
> 7 files changed, 173 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 3cf7b533b3d1..6804c1e84b36 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -398,3 +398,4 @@
> 384 i386 arch_prctl sys_arch_prctl __ia32_compat_sys_arch_prctl
> 385 i386 io_pgetevents sys_io_pgetevents __ia32_compat_sys_io_pgetevents
> 386 i386 rseq sys_rseq __ia32_sys_rseq
> +387 i386 pidfd_send_signal sys_pidfd_send_signal __ia32_sys_pidfd_send_signal
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index f0b1709a5ffb..aa4b858fa0f1 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -343,6 +343,7 @@
> 332 common statx __x64_sys_statx
> 333 common io_pgetevents __x64_sys_io_pgetevents
> 334 common rseq __x64_sys_rseq
> +335 common pidfd_send_signal __x64_sys_pidfd_send_signal
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ce3465479447..bf680b7b603a 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -716,8 +716,6 @@ static int proc_pid_permission(struct inode *inode, int mask)
> return generic_permission(inode, mask);
> }
>
> -
> -
> static const struct inode_operations proc_def_inode_operations = {
> .setattr = proc_setattr,
> };
> @@ -3038,6 +3036,15 @@ static const struct file_operations proc_tgid_base_operations = {
> .llseek = generic_file_llseek,
> };
>
> +struct pid *tgid_pidfd_to_pid(const struct file *file)
> +{
> + if (!d_is_dir(file->f_path.dentry) ||
> + (file->f_op != &proc_tgid_base_operations))
> + return ERR_PTR(-EBADF);
> +
> + return proc_pid(file_inode(file));
> +}
> +
> static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> {
> return proc_pident_lookup(dir, dentry,
> @@ -3422,6 +3429,15 @@ static const struct file_operations proc_tid_base_operations = {
> .llseek = generic_file_llseek,
> };
>
> +struct pid *tid_pidfd_to_pid(const struct file *file)
> +{
> + if (!d_is_dir(file->f_path.dentry) ||
> + (file->f_op != &proc_tid_base_operations))
> + return ERR_PTR(-EBADF);
> +
> + return proc_pid(file_inode(file));
> +}
> +
> static const struct inode_operations proc_tid_base_inode_operations = {
> .lookup = proc_tid_base_lookup,
> .getattr = pid_getattr,
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index d0e1f1522a78..eb150e5c0ab8 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -73,6 +73,8 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
> int (*show)(struct seq_file *, void *),
> proc_write_t write,
> void *data);
> +extern struct pid *tgid_pidfd_to_pid(const struct file *file);
> +extern struct pid *tid_pidfd_to_pid(const struct file *file);
>
> #else /* CONFIG_PROC_FS */
>
> @@ -114,6 +116,16 @@ static inline int remove_proc_subtree(const char *name, struct proc_dir_entry *p
> #define proc_create_net(name, mode, parent, state_size, ops) ({NULL;})
> #define proc_create_net_single(name, mode, parent, show, data) ({NULL;})
>
> +static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
> +{
> + return ERR_PTR(-EBADF);
> +}
> +
> +static inline struct pid *tid_pidfd_to_pid(const struct file *file)
> +{
> + return ERR_PTR(-EBADF);
> +}
> +
> #endif /* CONFIG_PROC_FS */
>
> struct net;
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 2ac3d13a915b..fd85b9045a9f 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -907,6 +907,9 @@ asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
> unsigned mask, struct statx __user *buffer);
> asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
> int flags, uint32_t sig);
> +asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
> + siginfo_t __user *info,
> + unsigned int flags);
>
> /*
> * Architecture-specific system calls
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 538546edbfbd..0822abc5927a 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -738,9 +738,11 @@ __SYSCALL(__NR_statx, sys_statx)
> __SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
> #define __NR_rseq 293
> __SYSCALL(__NR_rseq, sys_rseq)
> +#define __NR_pidfd_send_signal 294
> +__SYSCALL(__NR_pidfd_send_signal, sys_pidfd_send_signal)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 294
> +#define __NR_syscalls 295
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 9a32bc2088c9..3c83d3a5c7c5 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -19,7 +19,9 @@
> #include <linux/sched/task.h>
> #include <linux/sched/task_stack.h>
> #include <linux/sched/cputime.h>
> +#include <linux/file.h>
> #include <linux/fs.h>
> +#include <linux/proc_fs.h>
> #include <linux/tty.h>
> #include <linux/binfmts.h>
> #include <linux/coredump.h>
> @@ -3286,6 +3288,16 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
> }
> #endif
>
> +static inline void prepare_kill_siginfo(int sig, struct kernel_siginfo *info)
> +{
> + clear_siginfo(info);
> + info->si_signo = sig;
> + info->si_errno = 0;
> + info->si_code = SI_USER;
> + info->si_pid = task_tgid_vnr(current);
> + info->si_uid = from_kuid_munged(current_user_ns(), current_uid());
> +}
> +
> /**
> * sys_kill - send a signal to a process
> * @pid: the PID of the process
> @@ -3295,16 +3307,133 @@ SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
> {
> struct kernel_siginfo info;
>
> - clear_siginfo(&info);
> - info.si_signo = sig;
> - info.si_errno = 0;
> - info.si_code = SI_USER;
> - info.si_pid = task_tgid_vnr(current);
> - info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
> + prepare_kill_siginfo(sig, &info);
>
> return kill_something_info(sig, &info, pid);
> }
>
> +/*
> + * Verify that the signaler and signalee either are in the same pid namespace
> + * or that the signaler's pid namespace is an ancestor of the signalee's pid
> + * namespace.
> + */
> +static bool access_pidfd_pidns(struct pid *pid)
> +{
> + struct pid_namespace *active = task_active_pid_ns(current);
> + struct pid_namespace *p = ns_of_pid(pid);
> +
> + for (;;) {
> + if (!p)
> + return false;
> + if (p == active)
> + break;
> + p = p->parent;
> + }
> +
> + return true;
> +}
> +
> +static int copy_siginfo_from_user_any(kernel_siginfo_t *kinfo, siginfo_t *info)
> +{
> +#ifdef CONFIG_COMPAT
> + /*
> + * Avoid hooking up compat syscalls and instead handle necessary
> + * conversions here. Note, this is a stop-gap measure and should not be
> + * considered a generic solution.
> + */
> + if (in_compat_syscall())
> + return copy_siginfo_from_user32(
> + kinfo, (struct compat_siginfo __user *)info);
> +#endif
> + return copy_siginfo_from_user(kinfo, info);
> +}
> +
> +/**
> + * sys_pidfd_send_signal - send a signal to a process through a task file
> + * descriptor
> + * @pidfd: the file descriptor of the process
> + * @sig: signal to be sent
> + * @info: the signal info
> + * @flags: future flags to be passed
> + *
> + * The syscall currently only signals via PIDTYPE_PID which covers
> + * kill(<positive-pid>, <signal>. It does not signal threads or process
> + * groups.
> + * In order to extend the syscall to threads and process groups the @flags
> + * argument should be used. In essence, the @flags argument will determine
> + * what is signaled and not the file descriptor itself. Put in other words,
> + * grouping is a property of the flags argument not a property of the file
> + * descriptor.
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
> + siginfo_t __user *, info, unsigned int, flags)
> +{
> + int ret;
> + struct fd f;
> + struct pid *pid;
> + kernel_siginfo_t kinfo;
> +
> + /* Enforce flags be set to 0 until we add an extension. */
> + if (flags)
> + return -EINVAL;
> +
> + f = fdget_raw(pidfd);
> + if (!f.file)
> + return -EBADF;
> +
> + pid = tid_pidfd_to_pid(f.file);
> + if (!IS_ERR(pid)) {
> + /*
> + * Give userspace a way to detect /proc/<pid>/task/<tid>
> + * support when we add it.
> + */
> + ret = -EOPNOTSUPP;
> + goto err;
> + }
> +
> + /* Is this a pidfd? */
> + pid = tgid_pidfd_to_pid(f.file);
> + if (IS_ERR(pid)) {
> + ret = PTR_ERR(pid);
> + goto err;
> + }
> +
> + ret = -EINVAL;
> + if (!access_pidfd_pidns(pid))
> + goto err;
> +
> + if (info) {
> + ret = copy_siginfo_from_user_any(&kinfo, info);
> + if (unlikely(ret))
> + goto err;
> +
> + ret = -EINVAL;
> + if (unlikely(sig != kinfo.si_signo))
> + goto err;
> +
> + if ((task_pid(current) != pid) &&
> + (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) {
> + /* Only allow sending arbitrary signals to yourself. */
> + ret = -EPERM;
> + if (kinfo.si_code != SI_USER)
> + goto err;
> +
> + /* Turn this into a regular kill signal. */
> + prepare_kill_siginfo(sig, &kinfo);
> + }
> + } else {
> + prepare_kill_siginfo(sig, &kinfo);
> + }
> +
> + ret = kill_pid_info(sig, &kinfo, pid);
> +
> +err:
> + fdput(f);
> + return ret;
> +}
> +
> static int
> do_send_specific(pid_t tgid, pid_t pid, int sig, struct kernel_siginfo *info)
> {
> --
> 2.19.1
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Joseph Myers @ 2018-12-13 18:42 UTC (permalink / raw)
To: Florian Weimer
Cc: Rich Felker, Catalin Marinas, Andy Lutomirski, tg, Linus Torvalds,
X86 ML, LKML, Linux API, H. Peter Anvin, Peter Zijlstra,
Borislav Petkov, Mike Frysinger, H. J. Lu, x32, Arnd Bergmann,
Will Deacon
In-Reply-To: <87o99pl8es.fsf@oldenburg2.str.redhat.com>
On Thu, 13 Dec 2018, Florian Weimer wrote:
> Similarly, we could have integer types with trap representations.
C++2a will require two's complement representation for integer types, with
no trap representations (other than for bool, where only 0 and 1 are valid
representations). It seems very plausible that C will follow.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Matthew Wilcox @ 2018-12-13 17:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mimi Zohar, Philippe Trébuchet, Shuah Khan,
Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
kernel-hardening, linux-api, linux-security-module, linux-fsdevel
In-Reply-To: <ba523381-1b98-a8bf-ee5b-94a71caee48c@ssi.gouv.fr>
On Thu, Dec 13, 2018 at 06:36:15PM +0100, Mickaël Salaün wrote:
> On 13/12/2018 18:13, Matthew Wilcox wrote:
> > On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
> >> Adding a new syscall for this simple use case seems excessive. I think
> >
> > We have somewhat less than 400 syscalls today. We have 20 O_ bits defined.
> > Obviously there's a lower practical limit on syscalls, but in principle
> > we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.
> >
> >> that the open/openat syscall familly are the right place to do an atomic
> >> open and permission check, the same way the kernel does for other file
> >> access. Moreover, it will be easier to patch upstream interpreters
> >> without the burden of handling a (new) syscall that may not exist on the
> >> running system, whereas unknown open flags are ignored.
> >
> > Ah, but that's the problem. The interpreter can see an -ENOSYS response
> > and handle it appropriately. If the flag is silently ignored, the
> > interpreter has no idea whether it can do a racy check or whether to
> > skip even trying to do the check.
>
> Right, but the interpreter should interpret the script if the open with
> O_MAYEXEC succeed (but not otherwise): it may be because the flag is
> known by the kernel and the system policy allow this call, or because
> the (old) kernel doesn't known about this flag (which is fine and needed
> for backward compatibility). The script interpretation must not failed
> if the kernel doesn't support O_MAYEXEC, it is then useless for the
> interpreter to do any additional check.
If that's the way interpreters want to work, then that's fine. They
can just call the verify() syscall and ignore the -ENOSYS. Done.
Or somebody who cares very, very deeply can change the interpreter to
decline to run any scripts if the kernel returns -ENOSYS.
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2018-12-13 17:36 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mimi Zohar, Philippe Trébuchet, Shuah Khan,
Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
kernel-hardening, linux-api, linux-security-module, linux-fsdevel
In-Reply-To: <20181213171310.GR6830@bombadil.infradead.org>
On 13/12/2018 18:13, Matthew Wilcox wrote:
> On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
>> On 13/12/2018 04:02, Matthew Wilcox wrote:
>>> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
>>>> The goal of this patch series is to control script interpretation. A
>>>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>>>> interpreter to delegate to the kernel (and thus the system security
>>>> policy) the permission to interpret scripts or other files containing
>>>> what can be seen as commands.
>>>
>>> I don't have a problem with the concept, but we're running low on O_ bits.
>>> Does this have to be done before the process gets a file descriptor,
>>> or could we have a new syscall? Since we're going to be changing the
>>> interpreters anyway, it doesn't seem like too much of an imposition to
>>> ask them to use:
>>>
>>> int verify_for_exec(int fd)
>>>
>>> instead of adding an O_MAYEXEC.
>>
>> Adding a new syscall for this simple use case seems excessive. I think
>
> We have somewhat less than 400 syscalls today. We have 20 O_ bits defined.
> Obviously there's a lower practical limit on syscalls, but in principle
> we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.
>
>> that the open/openat syscall familly are the right place to do an atomic
>> open and permission check, the same way the kernel does for other file
>> access. Moreover, it will be easier to patch upstream interpreters
>> without the burden of handling a (new) syscall that may not exist on the
>> running system, whereas unknown open flags are ignored.
>
> Ah, but that's the problem. The interpreter can see an -ENOSYS response
> and handle it appropriately. If the flag is silently ignored, the
> interpreter has no idea whether it can do a racy check or whether to
> skip even trying to do the check.
Right, but the interpreter should interpret the script if the open with
O_MAYEXEC succeed (but not otherwise): it may be because the flag is
known by the kernel and the system policy allow this call, or because
the (old) kernel doesn't known about this flag (which is fine and needed
for backward compatibility). The script interpretation must not failed
if the kernel doesn't support O_MAYEXEC, it is then useless for the
interpreter to do any additional check.
^ permalink raw reply
* [PATCH v6 26/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2018-12-13 17:24 UTC (permalink / raw)
To: Andy Lutomirski, Oleg Nesterov
Cc: Eugene Syromyatnikov, Kees Cook, Jann Horn, linux-api,
strace-devel, linux-kernel
In-Reply-To: <20181213171833.GA5240@altlinux.org>
From: Elvira Khabirova <lineprinter@altlinux.org>
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
ptrace(2) man page:
long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
Retrieve information about the syscall that caused the stop.
The information is placed into the buffer pointed by "data"
argument, which should be a pointer to a buffer of type
"struct ptrace_syscall_info".
The "addr" argument contains the size of the buffer pointed to
by "data" argument (i.e., sizeof(struct ptrace_syscall_info)).
The return value contains the number of bytes available
to be written by the kernel.
If the size of data to be written by the kernel exceeds the size
specified by "addr" argument, the output is truncated.
Co-authored-by: Dmitry V. Levin <ldv@altlinux.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Eugene Syromyatnikov <esyr@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: linux-api@vger.kernel.org
Cc: strace-devel@lists.strace.io
Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
Notes:
v6:
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
include/linux/tracehook.h | 9 ++--
include/uapi/linux/ptrace.h | 35 +++++++++++++
kernel/ptrace.c | 101 +++++++++++++++++++++++++++++++++++-
3 files changed, 141 insertions(+), 4 deletions(-)
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index df20f8bdbfa3..6bc7a3d58e2f 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
/*
* ptrace report for syscall entry and exit looks identical.
*/
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+ unsigned long message)
{
int ptrace = current->ptrace;
if (!(ptrace & PT_PTRACED))
return 0;
+ current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
+ current->ptrace_message = 0;
return fatal_signal_pending(current);
}
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
{
- return ptrace_report_syscall(regs);
+ return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
}
/**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
- ptrace_report_syscall(regs);
+ ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
}
/**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..a71b6e3b03eb 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,41 @@ struct seccomp_metadata {
__u64 flags; /* Output: filter's flags */
};
+#define PTRACE_GET_SYSCALL_INFO 0x420e
+#define PTRACE_SYSCALL_INFO_NONE 0
+#define PTRACE_SYSCALL_INFO_ENTRY 1
+#define PTRACE_SYSCALL_INFO_EXIT 2
+#define PTRACE_SYSCALL_INFO_SECCOMP 3
+
+struct ptrace_syscall_info {
+ __u8 op; /* PTRACE_SYSCALL_INFO_* */
+ __u32 arch __attribute__((__aligned__(sizeof(__u32))));
+ __u64 instruction_pointer;
+ __u64 stack_pointer;
+ union {
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ } entry;
+ struct {
+ __s64 rval;
+ __u8 is_error;
+ } exit;
+ struct {
+ __u64 nr;
+ __u64 args[6];
+ __u32 ret_data;
+ } seccomp;
+ };
+};
+
+/*
+ * These values are stored in task->ptrace_message
+ * by tracehook_report_syscall_* to describe the current syscall-stop.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1
+#define PTRACE_EVENTMSG_SYSCALL_EXIT 2
+
/* Read signals from a shared (process wide) queue */
#define PTRACE_PEEKSIGINFO_SHARED (1 << 0)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index c2cee9db5204..2e5faeeaf4cd 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,8 @@
#include <linux/cn_proc.h>
#include <linux/compat.h>
+#include <asm/syscall.h> /* For syscall_get_* */
+
/*
* Access another process' address space via ptrace.
* Source/target buffer must be kernel space,
@@ -878,7 +880,100 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
* to ensure no machine forgets it.
*/
EXPORT_SYMBOL_GPL(task_user_regset_view);
-#endif
+#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
+
+static unsigned long
+ptrace_get_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ unsigned long args[ARRAY_SIZE(info->entry.args)];
+ int i;
+
+ info->op = PTRACE_SYSCALL_INFO_ENTRY;
+ info->entry.nr = syscall_get_nr(child, regs);
+ syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args);
+ for (i = 0; i < ARRAY_SIZE(args); i++)
+ info->entry.args[i] = args[i];
+
+ /* args is the last field in struct ptrace_syscall_info.entry */
+ return offsetofend(struct ptrace_syscall_info, entry.args);
+}
+
+static unsigned long
+ptrace_get_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ /*
+ * As struct ptrace_syscall_info.entry is currently a subset
+ * of struct ptrace_syscall_info.seccomp, it makes sense to
+ * initialize that subset using ptrace_get_syscall_info_entry().
+ * This can be reconsidered in the future if these structures
+ * diverge significantly enough.
+ */
+ ptrace_get_syscall_info_entry(child, regs, info);
+ info->op = PTRACE_SYSCALL_INFO_SECCOMP;
+ info->seccomp.ret_data = child->ptrace_message;
+
+ /* ret_data is the last field in struct ptrace_syscall_info.seccomp */
+ return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
+}
+
+static unsigned long
+ptrace_get_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+ struct ptrace_syscall_info *info)
+{
+ info->op = PTRACE_SYSCALL_INFO_EXIT;
+ info->exit.rval = syscall_get_error(child, regs);
+ info->exit.is_error = !!info->exit.rval;
+ if (!info->exit.is_error)
+ info->exit.rval = syscall_get_return_value(child, regs);
+
+ /* is_error is the last field in struct ptrace_syscall_info.exit */
+ return offsetofend(struct ptrace_syscall_info, exit.is_error);
+}
+
+static int
+ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
+ void __user *datavp)
+{
+ struct pt_regs *regs = task_pt_regs(child);
+ struct ptrace_syscall_info info = {
+ .op = PTRACE_SYSCALL_INFO_NONE,
+ .arch = syscall_get_arch(child),
+ .instruction_pointer = instruction_pointer(regs),
+ .stack_pointer = user_stack_pointer(regs),
+ };
+ unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
+ unsigned long write_size;
+
+ /*
+ * This does not need lock_task_sighand() to access
+ * child->last_siginfo because ptrace_freeze_traced()
+ * called earlier by ptrace_check_attach() ensures that
+ * the tracee cannot go away and clear its last_siginfo.
+ */
+ switch (child->last_siginfo ? child->last_siginfo->si_code : 0) {
+ case SIGTRAP | 0x80:
+ switch (child->ptrace_message) {
+ case PTRACE_EVENTMSG_SYSCALL_ENTRY:
+ actual_size = ptrace_get_syscall_info_entry(child, regs,
+ &info);
+ break;
+ case PTRACE_EVENTMSG_SYSCALL_EXIT:
+ actual_size = ptrace_get_syscall_info_exit(child, regs,
+ &info);
+ break;
+ }
+ break;
+ case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
+ actual_size = ptrace_get_syscall_info_seccomp(child, regs,
+ &info);
+ break;
+ }
+
+ write_size = min(actual_size, user_size);
+ return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
+}
int ptrace_request(struct task_struct *child, long request,
unsigned long addr, unsigned long data)
@@ -1095,6 +1190,10 @@ int ptrace_request(struct task_struct *child, long request,
ret = seccomp_get_metadata(child, addr, datavp);
break;
+ case PTRACE_GET_SYSCALL_INFO:
+ ret = ptrace_get_syscall_info(child, addr, datavp);
+ break;
+
default:
break;
}
--
ldv
^ permalink raw reply related
* [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2018-12-13 17:18 UTC (permalink / raw)
To: Oleg Nesterov, Andy Lutomirski
Cc: linux-s390, Rich Felker, linux-ia64, linux-sh,
Benjamin Herrenschmidt, Alexey Brodkin, Heiko Carstens, linux-api,
James E.J. Bottomley, Max Filippov, Guo Ren, Ralf Baechle,
linux-kselftest, H. Peter Anvin, Breno Leitao, Russell King,
linux-riscv, Vincent Chen, Shuah Khan, Thomas Gleixner,
Paul Mackerras, Jonas Bonn, Elvira Khabirova, sparclinux,
linux-arch
PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.
There are two reasons for a special syscall-related ptrace request.
Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.
Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.
PTRACE_GET_SYSCALL_INFO returns the following structure:
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
__u32 arch __attribute__((__aligned__(sizeof(__u32))));
__u64 instruction_pointer;
__u64 stack_pointer;
union {
struct {
__u64 nr;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
} exit;
struct {
__u64 nr;
__u64 args[6];
__u32 ret_data;
} seccomp;
};
};
The structure was chosen according to [2], except for the following
changes:
* seccomp substructure was added as a superset of entry substructure;
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer field was added along with instruction_pointer field
since it is readily available and can save the tracer from extra
PTRACE_GETREGS/PTRACE_GETREGSET calls;
* arch is always initialized to aid with tracing system calls
* such as execve();
* instruction_pointer and stack_pointer are always initialized
so they could be easily obtained for non-syscall stops;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.
strace has been ported to PTRACE_GET_SYSCALL_INFO, you can find it
in [3] and [4].
[1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/
[2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/
[3] https://github.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO
[4] https://gitlab.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO
---
Notes:
v6:
* Add syscall_get_arguments and syscall_set_arguments wrappers
to asm-generic/syscall.h, requested by Geert.
* Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
into account, use the end of the last field of the structure being written.
* Change struct ptrace_syscall_info:
* remove .frame_pointer field, is is not needed and not portable;
* make .arch field explicitly aligned, remove no longer needed
padding before .arch field;
* remove trailing pads, they are no longer needed.
v5:
* Merge separate series and patches into the single series.
* Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
* Change struct ptrace_syscall_info: generalize instruction_pointer,
stack_pointer, and frame_pointer fields by moving them from
ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
and initializing them for all stops.
* Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
instruction_pointer when the tracee is in a signal stop.
* Patch all remaining architectures to provide all necessary
syscall_get_* functions.
* Make available for all architectures: do not conditionalize on
CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
are implemented on all architectures.
* Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
v4:
* Do not introduce task_struct.ptrace_event,
use child->last_siginfo->si_code instead.
* Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
ptrace_syscall_info.{entry,exit}.
v3:
* Change struct ptrace_syscall_info.
* Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
* Add proper defines for ptrace_syscall_info.op values.
* Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
* and move them to uapi.
v2:
* Do not use task->ptrace.
* Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
* Use addr argument of sys_ptrace to get expected size of the struct;
return full size of the struct.
Dmitry V. Levin (25):
asm-generic/syscall.h: prepare for inclusion by other files
asm-generic/syscall.h: turn syscall_[gs]et_arguments into wrappers
alpha: define remaining syscall_get_* functions
Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
arc: define syscall_get_arch()
c6x: define syscall_get_arch()
elf-em.h: add EM_CSKY
csky: define syscall_get_arch()
h8300: define remaining syscall_get_* functions
Move EM_HEXAGON to uapi/linux/elf-em.h
hexagon: define remaining syscall_get_* functions
Move EM_NDS32 to uapi/linux/elf-em.h
nds32: define syscall_get_arch()
nios2: define syscall_get_arch()
m68k: add asm/syscall.h
mips: define syscall_get_error()
parisc: define syscall_get_error()
powerpc: define syscall_get_error()
riscv: define syscall_get_arch()
Move EM_XTENSA to uapi/linux/elf-em.h
xtensa: define syscall_get_* functions
Move EM_UNICORE to uapi/linux/elf-em.h
unicore32: add asm/syscall.h
syscall_get_arch: add "struct task_struct *" argument
selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO
Elvira Khabirova (2):
powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
ptrace: add PTRACE_GET_SYSCALL_INFO request
arch/alpha/include/asm/syscall.h | 31 +-
arch/arc/include/asm/elf.h | 6 +-
arch/arc/include/asm/syscall.h | 11 +
arch/arm/include/asm/syscall.h | 2 +-
arch/arm64/include/asm/syscall.h | 4 +-
arch/c6x/include/asm/syscall.h | 7 +
arch/csky/include/asm/syscall.h | 7 +
arch/h8300/include/asm/syscall.h | 19 ++
arch/hexagon/include/asm/elf.h | 6 +-
arch/hexagon/include/asm/syscall.h | 22 ++
arch/ia64/include/asm/syscall.h | 2 +-
arch/m68k/include/asm/syscall.h | 42 +++
arch/microblaze/include/asm/syscall.h | 2 +-
arch/mips/include/asm/syscall.h | 12 +-
arch/mips/kernel/ptrace.c | 2 +-
arch/nds32/include/asm/elf.h | 3 +-
arch/nds32/include/asm/syscall.h | 8 +
arch/nios2/include/asm/syscall.h | 6 +
arch/openrisc/include/asm/syscall.h | 2 +-
arch/parisc/include/asm/syscall.h | 11 +-
arch/powerpc/include/asm/syscall.h | 20 +-
arch/powerpc/kernel/ptrace.c | 7 +-
arch/riscv/include/asm/syscall.h | 10 +
arch/s390/include/asm/syscall.h | 4 +-
arch/sh/include/asm/syscall_32.h | 2 +-
arch/sh/include/asm/syscall_64.h | 2 +-
arch/sparc/include/asm/syscall.h | 5 +-
arch/unicore32/include/asm/elf.h | 3 +-
arch/unicore32/include/asm/syscall.h | 46 +++
arch/x86/include/asm/syscall.h | 8 +-
arch/x86/um/asm/syscall.h | 2 +-
arch/xtensa/include/asm/elf.h | 2 +-
arch/xtensa/include/asm/syscall.h | 65 +++++
include/asm-generic/syscall.h | 85 ++++--
include/linux/tracehook.h | 9 +-
include/uapi/linux/audit.h | 16 ++
include/uapi/linux/elf-em.h | 8 +
include/uapi/linux/ptrace.h | 35 +++
kernel/auditsc.c | 4 +-
kernel/ptrace.c | 101 ++++++-
kernel/seccomp.c | 4 +-
tools/testing/selftests/ptrace/.gitignore | 1 +
tools/testing/selftests/ptrace/Makefile | 2 +-
.../selftests/ptrace/get_syscall_info.c | 271 ++++++++++++++++++
44 files changed, 851 insertions(+), 66 deletions(-)
create mode 100644 arch/m68k/include/asm/syscall.h
create mode 100644 arch/unicore32/include/asm/syscall.h
create mode 100644 tools/testing/selftests/ptrace/get_syscall_info.c
--
ldv
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Matthew Wilcox @ 2018-12-13 17:13 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mimi Zohar, Philippe Trébuchet, Shuah Khan,
Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
kernel-hardening, linux-api, linux-security-module, linux-fsdevel
In-Reply-To: <374ea88c-edc5-f1a6-3637-748635e1e7df@ssi.gouv.fr>
On Thu, Dec 13, 2018 at 04:17:29PM +0100, Mickaël Salaün wrote:
> On 13/12/2018 04:02, Matthew Wilcox wrote:
> > On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
> >> The goal of this patch series is to control script interpretation. A
> >> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> >> interpreter to delegate to the kernel (and thus the system security
> >> policy) the permission to interpret scripts or other files containing
> >> what can be seen as commands.
> >
> > I don't have a problem with the concept, but we're running low on O_ bits.
> > Does this have to be done before the process gets a file descriptor,
> > or could we have a new syscall? Since we're going to be changing the
> > interpreters anyway, it doesn't seem like too much of an imposition to
> > ask them to use:
> >
> > int verify_for_exec(int fd)
> >
> > instead of adding an O_MAYEXEC.
>
> Adding a new syscall for this simple use case seems excessive. I think
We have somewhat less than 400 syscalls today. We have 20 O_ bits defined.
Obviously there's a lower practical limit on syscalls, but in principle
we could have up to 2^32 syscalls, and there are only 12 O_ bits remaining.
> that the open/openat syscall familly are the right place to do an atomic
> open and permission check, the same way the kernel does for other file
> access. Moreover, it will be easier to patch upstream interpreters
> without the burden of handling a (new) syscall that may not exist on the
> running system, whereas unknown open flags are ignored.
Ah, but that's the problem. The interpreter can see an -ENOSYS response
and handle it appropriately. If the flag is silently ignored, the
interpreter has no idea whether it can do a racy check or whether to
skip even trying to do the check.
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Rich Felker @ 2018-12-13 16:28 UTC (permalink / raw)
To: Florian Weimer
Cc: Catalin Marinas, Andy Lutomirski, tg, Linus Torvalds, X86 ML,
LKML, Linux API, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
Mike Frysinger, H. J. Lu, x32, Arnd Bergmann, Will Deacon
In-Reply-To: <87o99pl8es.fsf@oldenburg2.str.redhat.com>
On Thu, Dec 13, 2018 at 05:04:59PM +0100, Florian Weimer wrote:
> * Rich Felker:
>
> >> If the compiler can handle the zeroing, that would be great, though not
> >> sure how (some __attribute__((zero)) which generates a type constructor
> >> for such structure; it kind of departs from what the C language offers).
> >
> > The compiler fundamentally can't. At the very least it would require
> > effective type tracking, which requires shadow memory and is even more
> > controversial than -fstrict-aliasing (because in a sense it's a
> > stronger version thereof).
>
> It's possible to do it with the right types. See _Bool on 32-bit Darwin
> PowerPC for an example, which is four bytes instead of the usual one.
>
> Similarly, we could have integer types with trap representations.
> Whether it is a good idea is a different matter, but the amount of
> compiler magic required is actually limited.
If you do this you just have LP64 with value range restricted to
32-bit. This does not accomplish anything useful since it uses just as
much memory as normal LP64 but you don't get anything beneficial out
of it.
Rich
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Richard Purdie @ 2018-12-13 16:11 UTC (permalink / raw)
To: Linus Torvalds, Andrew Lutomirski
Cc: the arch/x86 maintainers, Linux List Kernel Mailing, linux-api,
Peter Anvin, Peter Zijlstra, bp, fweimer, vapier, hjl.tools,
dalias, x32, Arnd Bergmann, Will Deacon, Catalin Marinas
In-Reply-To: <CAHk-=wi_Kp=3XmGDdzmadzFSPFvuL+aAJ6ZPAR=o4z=KwYT2vw@mail.gmail.com>
On Mon, 2018-12-10 at 17:40 -0800, Linus Torvalds wrote:
> On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org>
> wrote:
> > I'm seriously considering sending a patch to remove x32 support
> > from
> > upstream Linux. Here are some problems with it:
>
> I talked to Arnd (I think - we were talking about all the crazy
> ABI's,
> but maybe it was with somebody else) about exactly this in Edinburgh.
>
> Apparently the main real use case is for extreme benchmarking. It's
> the only use-case where the complexity of maintaining a whole
> development environment and distro is worth it, it seems. Apparently
> a
> number of Spec submissions have been done with the x32 model.
>
> I'm not opposed to trying to sunset the support, but let's see who
> complains..
FWIW, the Yocto Project has working x32 support which it regularly
builds, tests and has as a supported configuration. We did help
identify and fix many of the places userspace didn't support it.
As with open source in general, its hard to know how many people are
actually using it. I don't think there are huge numbers but we do
occasionally get bug reports.
Cheers,
Richard
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Florian Weimer @ 2018-12-13 16:04 UTC (permalink / raw)
To: Rich Felker
Cc: Catalin Marinas, Andy Lutomirski, tg, Linus Torvalds, X86 ML,
LKML, Linux API, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
Mike Frysinger, H. J. Lu, x32, Arnd Bergmann, Will Deacon
In-Reply-To: <20181213155744.GU23599@brightrain.aerifal.cx>
* Rich Felker:
>> If the compiler can handle the zeroing, that would be great, though not
>> sure how (some __attribute__((zero)) which generates a type constructor
>> for such structure; it kind of departs from what the C language offers).
>
> The compiler fundamentally can't. At the very least it would require
> effective type tracking, which requires shadow memory and is even more
> controversial than -fstrict-aliasing (because in a sense it's a
> stronger version thereof).
It's possible to do it with the right types. See _Bool on 32-bit Darwin
PowerPC for an example, which is four bytes instead of the usual one.
Similarly, we could have integer types with trap representations.
Whether it is a good idea is a different matter, but the amount of
compiler magic required is actually limited.
Thanks,
Florian
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Rich Felker @ 2018-12-13 16:02 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: Andy Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, Mike Frysinger,
H. J. Lu, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <70bb54b2-8ed3-b5ee-c02d-6ef66c4f27eb@physik.fu-berlin.de>
On Tue, Dec 11, 2018 at 11:29:14AM +0100, John Paul Adrian Glaubitz wrote:
> I can't say anything about the syscall interface. However, what I do know
> is that the weird combination of a 32-bit userland with a 64-bit kernel
> interface is sometimes causing issues. For example, application code usually
> expects things like time_t to be 32-bit on a 32-bit system. However, this
> isn't the case for x32 which is why code fails to build.
I don't see any basis for this claim about expecting time_t to be
32-bit. I've encountered some programs that "implicitly assume" this
by virtue of assuming they can cast time_t to long to print it, or
similar. IIRC this was an issue in busybox at one point; I'm not sure
if it's been fixed. But any software that runs on non-Linux unices has
long been corrected. If not, 2038 is sufficiently close that catching
and correcting any such remaining bugs is more useful than covering
them up and making the broken code work as expected.
Rich
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Rich Felker @ 2018-12-13 15:57 UTC (permalink / raw)
To: Catalin Marinas
Cc: Andy Lutomirski, tg, Linus Torvalds, X86 ML, LKML, Linux API,
H. Peter Anvin, Peter Zijlstra, Borislav Petkov, Florian Weimer,
Mike Frysinger, H. J. Lu, x32, Arnd Bergmann, Will Deacon
In-Reply-To: <20181213124025.bczxzj6ez34joo6v@localhost>
On Thu, Dec 13, 2018 at 12:40:25PM +0000, Catalin Marinas wrote:
> On Wed, Dec 12, 2018 at 10:03:30AM -0800, Andy Lutomirski wrote:
> > On Wed, Dec 12, 2018 at 8:52 AM Rich Felker <dalias@libc.org> wrote:
> > > On Wed, Dec 12, 2018 at 08:39:53AM -0800, Andy Lutomirski wrote:
> > > > I'm proposing another alternative. Given that x32 already proves that
> > > > the user bitness model doesn't have to match the kernel model (in x32,
> > > > user "long" is 32-bit but the kernel ABI "long" is 64-bit), I'm
> > > > proposing extending this to just make the kernel ABI be LP64. So
> > > > __kernel_size_t would be 64-bit and pointers in kernel data structures
> > > > would be 64-bit. In other words, most or all of the kernel ABI would
> > > > just match x86_64.
> > > >
> > > > As far as I can tell, the only thing that really needs unusual
> > > > toolchain features here is that C doesn't have an extra-wide pointer
> > > > type. The kernel headers would need a way to say "this pointer is
> > > > still logically a pointer, and user code may assume that it's 32 bits,
> > > > but it has 8-byte alignment."
> > >
> > > None of this works on the userspace/C side, nor should any attempt be
> > > made to make it work. Types fundamentally cannot have alignments
> > > larger than their size. If you want to make the alignment of some
> > > pointers 8, you have to make their size 8, and then you just have LP64
> > > again if you did it for all pointers.
> > >
> > > If on the other hand you tried to make just some pointers "wide
> > > pointers", you'd also be completely breaking the specified API
> > > contracts of standard interfaces. For example in struct iovec's
> > > iov_base, &foo->iov_base is no longer a valid pointer to an object of
> > > type void* that you can pass to interfaces expecting void**. Sloppy
> > > misunderstandings like what you're making now are exactly why x32 is
> > > already broken and buggy (&foo->tv_nsec already has wrong type for
> > > struct timespec foo).
> >
> > I don't think it's quite that broken. For the struct iovec example,
> > we currently have:
> >
> > struct iovec {
> > void *iov_base; /* Starting address */
> > size_t iov_len; /* Number of bytes to transfer */
> > };
> >
> > we could have, instead: (pardon any whitespace damage)
> >
> > struct iovec {
> > void *iov_base; /* Starting address */
> > uint32_t __pad0;
> > size_t iov_len; /* Number of bytes to transfer */
> > uint32_t __pad1;
> > } __attribute__((aligned(8));
> >
> > or the same thing but where iov_len is uint64_t. A pointer to
> > iov_base still works exactly as expected. Something would need to be
> > done to ensure that the padding is all zeroed, which might be a real
> > problem.
>
> We looked at this approach briefly for arm64/ILP32 and zeroing the pads
> was the biggest problem. User programs would not explicitly zero the pad
> and I'm not sure the compiler would be any smarter. This means it's the
> kernel's responsibility to zero the pad (around get_user,
> copy_from_user), so it doesn't actually simplify the kernel side of the
> syscall interface.
>
> If the data flow goes the other way (kernel to user), this approach
> works fine.
>
> > No one wants to actually type all the macro gunk into the headers to
> > make this work, but this type of transformation is what I have in mind
> > when the compiler is asked to handle the headers. Or there could
> > potentially be a tool that automatically consumes the uapi headers and
> > spits out modified headers like this.
>
> If the compiler can handle the zeroing, that would be great, though not
> sure how (some __attribute__((zero)) which generates a type constructor
> for such structure; it kind of departs from what the C language offers).
The compiler fundamentally can't. At the very least it would require
effective type tracking, which requires shadow memory and is even more
controversial than -fstrict-aliasing (because in a sense it's a
stronger version thereof). But even effective type tracking would not
help, since you can have things like:
struct iovec *iov = malloc(sizeof *iov);
scanf("%p %zu", &iov->iov_base, &iov->iov_len);
where no store to the object via the struct type ever happens and the
only stores that do happen are invisible across translation unit
boundaries. (Ignore that scanf here is awful; it's just a canonical
example of a function that would store the members via pointers to
them.)
The kernel-side approach could work if the kernel had some markup for
fields that need to be zero- or sign-extended when copied from user in
a 32-bit process and applied them at copy time. That could also fix
the existing tv_nsec issue. I'm not sure how difficult/costly it would
be though.
Rich
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Lance Richardson @ 2018-12-13 15:46 UTC (permalink / raw)
To: olof
Cc: torvalds, luto, x86, linux-kernel, linux-api, hpa, peterz, bp,
fweimer, vapier, hjl.tools, dalias, x32, arnd, will.deacon,
catalin.marinas
In-Reply-To: <CAOesGMgu1i3p7XMZuCEtj63T-ST_jh+BfaHy-K6LhgqNriKHAA@mail.gmail.com>
On Thu, Dec 13, 2018 at 9:39 AM Olof Johansson <olof@lixom.net> wrote:
>
> On Tue, Dec 11, 2018 at 9:40 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org> wrote:
> > >
> > > I'm seriously considering sending a patch to remove x32 support from
> > > upstream Linux. Here are some problems with it:
> >
> > I talked to Arnd (I think - we were talking about all the crazy ABI's,
> > but maybe it was with somebody else) about exactly this in Edinburgh.
> >
> > Apparently the main real use case is for extreme benchmarking. It's
> > the only use-case where the complexity of maintaining a whole
> > development environment and distro is worth it, it seems. Apparently a
> > number of Spec submissions have been done with the x32 model.
> >
> > I'm not opposed to trying to sunset the support, but let's see who complains..
>
> I'm just a single user. I do rely on it though, FWIW.
>
> I hadn't finished my benchmarking in Edinburgh, but for my new machine
> that does kernel builds 24/7, I ended up going with x32 userspace (in
> a container).
>
> Main reason is that it's a free ~10% improvement in runtime over
> 64-bit. I.e. GCC-as-a-workload is quite a bit faster as x32,
> supposedly mostly due to smaller D cache footprints (I ran out of
> cycles to tinker with back and forth perf data collection and settled
> down on just running it).
>
> Running classic 32-bit (i386? i686? whatever it's called) is about
> half as good. I.e. even then I get a ~5% performance win. Less than
> x32, but still better than 64-bit userspace.
>
>
> -Olof
I'm familiar with two embedded Linux systems using x32 ABI for the
following reasons:
- Significant performance benefits over i386 ABI.
- Smaller memory footprint than x86_64 (pointer-heavy data structures)
- Large legacy software base with many ILP32 assumptions, much
smaller effort to move to x32 than x86_64.
Some examples of (relatively minor) problems encountered with x32:
- Time-related data type mismatch in socket options (fixed)
https://patchwork.ozlabs.org/patch/904254/
- Userspace overrun with select() (patch proposed)
https://patchwork.kernel.org/patch/10245677/
- glibc get_phys_pages() doesn't work correctly with x32
(assumes that struct sysinfo fields are not wider than "long").
So, one small vote for keeping x32 with the hope that support for it
can continue to be improved...
- Lance
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2018-12-13 15:17 UTC (permalink / raw)
To: Matthew Wilcox, Mickaël Salaün
Cc: linux-kernel, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
Matthew Garrett, Michael Kerrisk, Mimi Zohar,
Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
linux-security-module, linux-fsdevel
In-Reply-To: <20181213030228.GM6830@bombadil.infradead.org>
On 13/12/2018 04:02, Matthew Wilcox wrote:
> On Wed, Dec 12, 2018 at 09:17:07AM +0100, Mickaël Salaün wrote:
>> The goal of this patch series is to control script interpretation. A
>> new O_MAYEXEC flag used by sys_open() is added to enable userland script
>> interpreter to delegate to the kernel (and thus the system security
>> policy) the permission to interpret scripts or other files containing
>> what can be seen as commands.
>
> I don't have a problem with the concept, but we're running low on O_ bits.
> Does this have to be done before the process gets a file descriptor,
> or could we have a new syscall? Since we're going to be changing the
> interpreters anyway, it doesn't seem like too much of an imposition to
> ask them to use:
>
> int verify_for_exec(int fd)
>
> instead of adding an O_MAYEXEC.
>
Adding a new syscall for this simple use case seems excessive. I think
that the open/openat syscall familly are the right place to do an atomic
open and permission check, the same way the kernel does for other file
access. Moreover, it will be easier to patch upstream interpreters
without the burden of handling a (new) syscall that may not exist on the
running system, whereas unknown open flags are ignored.
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2018-12-13 14:57 UTC (permalink / raw)
To: Florian Weimer, James Morris
Cc: Mickaël Salaün, linux-kernel, Al Viro, Jonathan Corbet,
Kees Cook, Matthew Garrett, Michael Kerrisk, Mimi Zohar,
Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
linux-security-module, linux-fsdevel
In-Reply-To: <87ftv2ovp8.fsf@oldenburg2.str.redhat.com>
On 13/12/2018 06:13, Florian Weimer wrote:
> * James Morris:
>
>> On Wed, 12 Dec 2018, Florian Weimer wrote:
>>
>>> * James Morris:
>>>
>>>> If you're depending on the script interpreter to flag that the user may
>>>> execute code, this seems to be equivalent in security terms to depending
>>>> on the user. e.g. what if the user uses ptrace and clears O_MAYEXEC?
This security mechanism makes sense in an hardened system where the user
is not allowed to import and execute new file (write xor execute
policy). This can be enforced with appropriate mount points a more
advanced access control policy.
>>>
>>> The argument I've heard is this: Using ptrace (and adding the +x
>>> attribute) are auditable events.
>>
>> I guess you could also preload a modified libc which strips the flag.
>
> My understanding is that this new libc would have to come somewhere, and
> making it executable would be an auditable even as well.
Auditing is a possible use case as well, but the W^X idea is to deny use
of libraries which are not in an executable mount point, i.e. only
execute trusted code.
^ permalink raw reply
* Re: [RFC PATCH v1 3/5] Yama: Enforces noexec mounts or file executability through O_MAYEXEC
From: Mickaël Salaün @ 2018-12-13 14:49 UTC (permalink / raw)
To: Jann Horn, Mickaël Salaün
Cc: kernel list, Al Viro, James Morris, Jonathan Corbet, Kees Cook,
Matthew Garrett, Michael Kerrisk-manpages, zohar,
philippe.trebuchet, shuah, thibaut.sautereau, vincent.strubel,
yves-alexis.perez, Kernel Hardening, Linux API,
linux-security-module, linux-fsdevel
In-Reply-To: <CAG48ez3SAW8EyaJ9T1U3qPoRhYwe4CCyL9bAxuc3GxjrXipi-A@mail.gmail.com>
On 12/12/2018 18:09, Jann Horn wrote:
> On Wed, Dec 12, 2018 at 9:18 AM Mickaël Salaün <mic@digikod.net> wrote:
>> Enable to either propagate the mount options from the underlying VFS
>> mount to prevent execution, or to propagate the file execute permission.
>> This may allow a script interpreter to check execution permissions
>> before reading commands from a file.
>>
>> The main goal is to be able to protect the kernel by restricting
>> arbitrary syscalls that an attacker could perform with a crafted binary
>> or certain script languages. It also improves multilevel isolation
>> by reducing the ability of an attacker to use side channels with
>> specific code. These restrictions can natively be enforced for ELF
>> binaries (with the noexec mount option) but require this kernel
>> extension to properly handle scripts (e.g., Python, Perl).
>>
>> Add a new sysctl kernel.yama.open_mayexec_enforce to control this
>> behavior. A following patch adds documentation.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>> Reviewed-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>> ---
> [...]
>> +/**
>> + * yama_inode_permission - check O_MAYEXEC permission before accessing an inode
>> + * @inode: inode structure to check
>> + * @mask: permission mask
>> + *
>> + * Return 0 if access is permitted, -EACCES otherwise.
>> + */
>> +int yama_inode_permission(struct inode *inode, int mask)
>
> This should be static, no?
Right, it will be in the next series. The previous function
(yama_ptrace_traceme) is not static though.
>
>> +{
>> + if (!(mask & MAY_OPENEXEC))
>> + return 0;
>> + /*
>> + * Match regular files and directories to make it easier to
>> + * modify script interpreters.
>> + */
>> + if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
>> + return 0;
>
> So files are subject to checks, but loading code from things like
> sockets is always fine?
As I said in a previous email, these checks do not handle fifo either.
This is relevant in a threat model targeting persistent attacks (and
with additional protections/restrictions). We may want to only whitelist
fifo, but I don't get how a socket is relevant here. Can you please clarify?
>
>> + if ((open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_MOUNT) &&
>> + !(mask & MAY_EXECMOUNT))
>> + return -EACCES;
>> +
>> + /*
>> + * May prefer acl_permission_check() instead of generic_permission(),
>> + * to not be bypassable with CAP_DAC_READ_SEARCH.
>> + */
>> + if (open_mayexec_enforce & YAMA_OMAYEXEC_ENFORCE_FILE)
>> + return generic_permission(inode, MAY_EXEC);
>> +
>> + return 0;
>> +}
>> +
>> static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
>> + LSM_HOOK_INIT(inode_permission, yama_inode_permission),
>> LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>> LSM_HOOK_INIT(task_prctl, yama_task_prctl),
>> @@ -447,6 +489,37 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write,
>> return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos);
>> }
>>
>> +static int yama_dointvec_bitmask_macadmin(struct ctl_table *table, int write,
>> + void __user *buffer, size_t *lenp,
>> + loff_t *ppos)
>> +{
>> + int error;
>> +
>> + if (write) {
>> + struct ctl_table table_copy;
>> + int tmp_mayexec_enforce;
>> +
>> + if (!capable(CAP_MAC_ADMIN))
>> + return -EPERM;
>
> Don't put capable() checks in sysctls, it doesn't work.
>
I tested it and the root user can indeed open the file even if the
process doesn't have CAP_MAC_ADMIN, however writing in the sysctl file
is denied. Btw there is a similar check in the previous function
(yama_dointvec_minmax).
Thanks
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Olof Johansson @ 2018-12-13 14:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: luto, x86, Linux Kernel Mailing List, linux-api, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, fweimer, Mike Frysinger,
hjl.tools, dalias, x32, Arnd Bergmann, Will Deacon,
Catalin Marinas
In-Reply-To: <CAHk-=wi_Kp=3XmGDdzmadzFSPFvuL+aAJ6ZPAR=o4z=KwYT2vw@mail.gmail.com>
On Tue, Dec 11, 2018 at 9:40 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > I'm seriously considering sending a patch to remove x32 support from
> > upstream Linux. Here are some problems with it:
>
> I talked to Arnd (I think - we were talking about all the crazy ABI's,
> but maybe it was with somebody else) about exactly this in Edinburgh.
>
> Apparently the main real use case is for extreme benchmarking. It's
> the only use-case where the complexity of maintaining a whole
> development environment and distro is worth it, it seems. Apparently a
> number of Spec submissions have been done with the x32 model.
>
> I'm not opposed to trying to sunset the support, but let's see who complains..
I'm just a single user. I do rely on it though, FWIW.
I hadn't finished my benchmarking in Edinburgh, but for my new machine
that does kernel builds 24/7, I ended up going with x32 userspace (in
a container).
Main reason is that it's a free ~10% improvement in runtime over
64-bit. I.e. GCC-as-a-workload is quite a bit faster as x32,
supposedly mostly due to smaller D cache footprints (I ran out of
cycles to tinker with back and forth perf data collection and settled
down on just running it).
Running classic 32-bit (i386? i686? whatever it's called) is about
half as good. I.e. even then I get a ~5% performance win. Less than
x32, but still better than 64-bit userspace.
-Olof
^ permalink raw reply
* Re: [RFC PATCH v1 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
From: Mickaël Salaün @ 2018-12-13 14:23 UTC (permalink / raw)
To: Matthew Bobrowski, Jan Kara
Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mimi Zohar, Philippe Trébuchet, Shuah Khan,
Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
kernel-hardening, linux-api, linux-security-module, linux-fsdevel,
sgrubb
In-Reply-To: <20181213094658.GA996@lithium.mbobrowski.org>
On 13/12/2018 10:47, Matthew Bobrowski wrote:
> On Wed, Dec 12, 2018 at 03:43:06PM +0100, Jan Kara wrote:
>>> When the O_MAYEXEC flag is passed, sys_open() may be subject to
>>> additional restrictions depending on a security policy implemented by an
>>> LSM through the inode_permission hook.
>>>
>>> The underlying idea is to be able to restrict scripts interpretation
>>> according to a policy defined by the system administrator. For this to
>>> be possible, script interpreters must use the O_MAYEXEC flag
>>> appropriately. To be fully effective, these interpreters also need to
>>> handle the other ways to execute code (for which the kernel can't help):
>>> command line parameters (e.g., option -e for Perl), module loading
>>> (e.g., option -m for Python), stdin, file sourcing, environment
>>> variables, configuration files... According to the threat model, it may
>>> be acceptable to allow some script interpreters (e.g. Bash) to interpret
>>> commands from stdin, may it be a TTY or a pipe, because it may not be
>>> enough to (directly) perform syscalls.
>>>
>>> A simple security policy implementation is available in a following
>>> patch for Yama.
>>>
>>> This is an updated subset of the patch initially written by Vincent
>>> Strubel for CLIP OS:
>>> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
>>> This patch has been used for more than 10 years with customized script
>>> interpreters. Some examples can be found here:
>>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>>>
>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> Signed-off-by: Thibaut Sautereau <thibaut.sautereau@ssi.gouv.fr>
>>> Signed-off-by: Vincent Strubel <vincent.strubel@ssi.gouv.fr>
>>> Reviewed-by: Philippe Trébuchet <philippe.trebuchet@ssi.gouv.fr>
>>> Cc: Al Viro <viro@zeniv.linux.org.uk>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Mickaël Salaün <mickael.salaun@ssi.gouv.fr>
>>
>> ...
>>
>>> diff --git a/fs/open.c b/fs/open.c
>>> index 0285ce7dbd51..75479b79a58f 100644
>>> --- a/fs/open.c
>>> +++ b/fs/open.c
>>> @@ -974,6 +974,10 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
>>> if (flags & O_APPEND)
>>> acc_mode |= MAY_APPEND;
>>>
>>> + /* Check execution permissions on open. */
>>> + if (flags & O_MAYEXEC)
>>> + acc_mode |= MAY_OPENEXEC;
>>> +
>>> op->acc_mode = acc_mode;
>>>
>>> op->intent = flags & O_PATH ? 0 : LOOKUP_OPEN;
>>
>> I don't feel experienced enough in security to tell whether we want this
>> functionality or not. But if we do this, shouldn't we also set FMODE_EXEC
>> on the resulting struct file? That way also security_file_open() can be
>> used to arbitrate such executable opens and in particular
>> fanotify permission event FAN_OPEN_EXEC will get properly generated which I
>> guess is desirable (support for it is sitting in my tree waiting for the
>> merge window) - adding some audit people involved in FAN_OPEN_EXEC to
>> CC. Just an idea...
>
> If I'm understanding this patch series correctly, without an enforced LSM
> policy there's realistically no added benefit from a security perspective,
> right?
That's correct. The kernel knows the semantic but the enforcement is
delegated to an LSM and its policy.
> Also, I'm in agreement with what Jan has mentioned in regards to setting
> the __FMODE_EXEC flag when O_MAYEXEC has been specified. This is something that
> would work quite nicely in conjunction with some of the new file access
> notification events.
OK, I will add it in the next patch series (for the new FAN_OPEN_EXEC
support).
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Catalin Marinas @ 2018-12-13 12:40 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Rich Felker, tg, Linus Torvalds, X86 ML, LKML, Linux API,
H. Peter Anvin, Peter Zijlstra, Borislav Petkov, Florian Weimer,
Mike Frysinger, H. J. Lu, x32, Arnd Bergmann, Will Deacon
In-Reply-To: <CALCETrV6+YAazq7vY_aR=4kXc4ykXb1Se7hgvHeEVJtbZ91=Qg@mail.gmail.com>
On Wed, Dec 12, 2018 at 10:03:30AM -0800, Andy Lutomirski wrote:
> On Wed, Dec 12, 2018 at 8:52 AM Rich Felker <dalias@libc.org> wrote:
> > On Wed, Dec 12, 2018 at 08:39:53AM -0800, Andy Lutomirski wrote:
> > > I'm proposing another alternative. Given that x32 already proves that
> > > the user bitness model doesn't have to match the kernel model (in x32,
> > > user "long" is 32-bit but the kernel ABI "long" is 64-bit), I'm
> > > proposing extending this to just make the kernel ABI be LP64. So
> > > __kernel_size_t would be 64-bit and pointers in kernel data structures
> > > would be 64-bit. In other words, most or all of the kernel ABI would
> > > just match x86_64.
> > >
> > > As far as I can tell, the only thing that really needs unusual
> > > toolchain features here is that C doesn't have an extra-wide pointer
> > > type. The kernel headers would need a way to say "this pointer is
> > > still logically a pointer, and user code may assume that it's 32 bits,
> > > but it has 8-byte alignment."
> >
> > None of this works on the userspace/C side, nor should any attempt be
> > made to make it work. Types fundamentally cannot have alignments
> > larger than their size. If you want to make the alignment of some
> > pointers 8, you have to make their size 8, and then you just have LP64
> > again if you did it for all pointers.
> >
> > If on the other hand you tried to make just some pointers "wide
> > pointers", you'd also be completely breaking the specified API
> > contracts of standard interfaces. For example in struct iovec's
> > iov_base, &foo->iov_base is no longer a valid pointer to an object of
> > type void* that you can pass to interfaces expecting void**. Sloppy
> > misunderstandings like what you're making now are exactly why x32 is
> > already broken and buggy (&foo->tv_nsec already has wrong type for
> > struct timespec foo).
>
> I don't think it's quite that broken. For the struct iovec example,
> we currently have:
>
> struct iovec {
> void *iov_base; /* Starting address */
> size_t iov_len; /* Number of bytes to transfer */
> };
>
> we could have, instead: (pardon any whitespace damage)
>
> struct iovec {
> void *iov_base; /* Starting address */
> uint32_t __pad0;
> size_t iov_len; /* Number of bytes to transfer */
> uint32_t __pad1;
> } __attribute__((aligned(8));
>
> or the same thing but where iov_len is uint64_t. A pointer to
> iov_base still works exactly as expected. Something would need to be
> done to ensure that the padding is all zeroed, which might be a real
> problem.
We looked at this approach briefly for arm64/ILP32 and zeroing the pads
was the biggest problem. User programs would not explicitly zero the pad
and I'm not sure the compiler would be any smarter. This means it's the
kernel's responsibility to zero the pad (around get_user,
copy_from_user), so it doesn't actually simplify the kernel side of the
syscall interface.
If the data flow goes the other way (kernel to user), this approach
works fine.
> No one wants to actually type all the macro gunk into the headers to
> make this work, but this type of transformation is what I have in mind
> when the compiler is asked to handle the headers. Or there could
> potentially be a tool that automatically consumes the uapi headers and
> spits out modified headers like this.
If the compiler can handle the zeroing, that would be great, though not
sure how (some __attribute__((zero)) which generates a type constructor
for such structure; it kind of departs from what the C language offers).
> Realistically, I think a much better model would be to use true ILP32
> code, where all the memory layouts in the uapi match i386.
The conclusion we came to on arm64 was that an ILP32 ABI should not
really be any different from a _new_ 32-bit architecture ABI. It differs
from arm32 a bit (different syscall numbers, off_t is 64-bit,
sigcontext) but not significantly as it is still able to use the
majority of the compat_sys_* wrappers.
--
Catalin
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Mimi Zohar @ 2018-12-13 12:16 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Wilcox, Mickaël Salaün, linux-kernel, Al Viro,
James Morris, Jonathan Corbet, Kees Cook, Matthew Garrett,
Michael Kerrisk, Mickaël Salaün,
Philippe Trébuchet, Shuah Khan, Thibaut Sautereau,
Vincent Strubel, Yves-Alexis Perez, kernel-hardening, linux-api,
linux-security-module
In-Reply-To: <874lbhoef3.fsf@oldenburg2.str.redhat.com>
[Cc'ing linux-integrity]
On Thu, 2018-12-13 at 12:26 +0100, Florian Weimer wrote:
> * Mimi Zohar:
>
> > The indication needs to be set during file open, before the open
> > returns to the caller. This is the point where ima_file_check()
> > verifies the file's signature. On failure, access to the file is
> > denied.
>
> Does this verification happen for open with O_PATH?
Interesting! According to the manpage, userspace cannot read/write to
the file. It looks like do_o_path() intentionally skips do_last(),
with the call to ima_file_check(). If the file data isn't being
accessed, does the file's integrity need to be verified?
Mimi
^ permalink raw reply
* Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC
From: Matthew Wilcox @ 2018-12-13 12:16 UTC (permalink / raw)
To: Mimi Zohar
Cc: Mickaël Salaün, linux-kernel, Al Viro, James Morris,
Jonathan Corbet, Kees Cook, Matthew Garrett, Michael Kerrisk,
Mickaël Salaün, Philippe Trébuchet, Shuah Khan,
Thibaut Sautereau, Vincent Strubel, Yves-Alexis Perez,
kernel-hardening, linux-api, linux-security-module, linux-fsdevel
In-Reply-To: <1544699060.6703.11.camel@linux.ibm.com>
On Thu, Dec 13, 2018 at 06:04:20AM -0500, Mimi Zohar wrote:
> > I don't have a problem with the concept, but we're running low on O_ bits.
> > Does this have to be done before the process gets a file descriptor,
> > or could we have a new syscall? Since we're going to be changing the
> > interpreters anyway, it doesn't seem like too much of an imposition to
> > ask them to use:
> >
> > int verify_for_exec(int fd)
> >
> > instead of adding an O_MAYEXEC.
>
> The indication needs to be set during file open, before the open
> returns to the caller. This is the point where ima_file_check()
> verifies the file's signature. On failure, access to the file is
> denied.
I understand that's what happens today, but do we need to do it that way?
There's no harm in the interpreter having an fd to a file if it knows
not to execute it. This is different from a program opening a file and
having the LSM deny access to it because it violates the security model.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox