* Re: Can we drop upstream Linux x32 support?
From: Rich Felker @ 2018-12-11 23:33 UTC (permalink / raw)
To: Thorsten Glaser
Cc: John Paul Adrian Glaubitz, 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: <Pine.BSM.4.64L.1812112155170.21176@herc.mirbsd.org>
On Tue, Dec 11, 2018 at 09:59:48PM +0000, Thorsten Glaser wrote:
> John Paul Adrian Glaubitz dixit:
>
> >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
>
> Yes, but more and more ${foo}64ilp32 architectures are popping up.
>
> >Additionally, x32 support in many applications is either rudimentary
>
> If a signal is sent that this kind of architectures will stay, some
> people might be convinced to fix that.
>
> >It's also that the performance benefits of x32 are often eaten up by
> >the fact that none of the scripted languages that I know of provide
>
> Non-JITted languages like yours truly’s shell do benefit from it,
> though. (mksh works just fine on LP64 but its internal structures
> pack massively better on ILP32, for example.)
gcc also benefits massively, up to 2x or more performance improvement
from ILP32, just because you can double your -jN to make. The "or
more" comes from cache utilization also improving. Actually I often
use i486 gcc even on 64-bit systems for this reason, and have observed
this order of improvement despite the lack of registers and worse call
ABI. Presumably x32 would be better, but I've avoided it because of
how buggy it's been in the past.
> >If x32 is eventually to be removed, we should also take care of removing
> >x32 support from userland code. From the top of my head, this would at least
>
> I don’t think so. The patches also contain
> – stuff to support 64-bit time_t on 32-bit architectures, e.g:
> - bugfixes like printf("%lld", (long long)timet_value) instead
> of assuming time_t fits into a long (also important for other
> operating systems…)
> - generally switching from generic types like long to specific
> types like size_t, ptrdiff_t, etc.
> - there was one more but after having written two eMails I forgot it
> - oh and, of course, they lay the base for e.g. amd64ilp32 support
I would not classify any of that as part of "removing x32 support",
but rather as "reintroducing broken assumptions". I think "removing
x32 support" was intended to mean places where there's
explicitly-x32-specific code or tooling, like asm functions/fragments
in libc, x32 versions of the dispatch asm in mesa, x32 code
generation, etc. Obviously (at least I hope it's obvious) nonportable
assumptions should not be reintroduced regardless of whether x32 is
dropped.
FWIW x32 is nice as a test case for catching such nonportable
assumptions and getting them fixed.
Rich
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Andy Lutomirski @ 2018-12-11 23:22 UTC (permalink / raw)
To: tg
Cc: Linus Torvalds, Andrew Lutomirski, X86 ML, LKML, Linux API,
H. Peter Anvin, Peter Zijlstra, Borislav Petkov, Florian Weimer,
Mike Frysinger, H. J. Lu, Rich Felker, x32, Arnd Bergmann,
Will Deacon, Catalin Marinas
In-Reply-To: <Pine.BSM.4.64L.1812112150480.21176@herc.mirbsd.org>
On Tue, Dec 11, 2018 at 2:14 PM Thorsten Glaser <tg@mirbsd.de> wrote:
>
> Note: please keep me in Cc because I am not subscribed.
>
> Linus Torvalds dixit:
>
> >I'm not opposed to trying to sunset the support, but let's see who complains..
>
> I will hereby complain.
>
> I’m using Debian/x32 on my main desktop at work, and do
> occasionally help out with porting issues. It’s a good
> way to make more out of 64-bit machines without going
> all 64 bit; it’s also helped me find bugs in software.
> It’s a nice architectural idea, and a way forward for
> things that are constricted to 32 bits while opening
> up stuff like 64-bit time_t without taking up half the
> available CPU registers (while more than doubling the
> number of the available CPU registers, too).
Thanks for responding!
I suppose the question is: are you enough of a user to justify the
continued maintenance effort.
>
> I was also considering investing a nontrivial amount of
> work into porting klibc to x32, since hpa does not wish
> to do it himself. Thankfully I have only done a bit yet.
>
> Furthermore, x32 was the first of the many *64ilp32
> architectures; I know I’ve seen amd64ilp32 and at least
> one other I don’t recall. It will have prototyped many
> of the problems users of these will run in, and I’d prefer
> to keep it (completely selfish because I don’t wish to
> have to crossgrade a whole system yet again).
it kind of seems like arm64's lesson is "don't do it like x32".
There's some effort going on right now to make it possible to add
syscalls without having to muck with every single architecture. I
don't really want x32 to derail that effort. I suppose we could say
that x32 stays but that it simply gets no new syscalls, but that seems
a bit lame. Unfortunately, on x86, x32 really is a third ABI that is
not compatible in a structure-memory-layout sense with the other two.
What happens if someone adds a struct like:
struct nasty_on_x32 {
__kernel_long_t a;
void * __user b;
};
On x86_64, that's two 8-byte fields. On x86_32, it's two four-byte
fields. On x32, it's an 8-byte field and a 4-byte field. Now what?
I'm sure we could have some magic gcc plugin or other nifty tool that gives us:
copy_from_user(struct struct_name, kernel_ptr, user_ptr);
where it automatically generates code for all possible ABIs to copy
over the struct and dispatches dynamically based on the current
syscall ABI, but I have trouble imagining anyone volunteering to
actually do this work. Instead we get ad hoc fixes for each syscall,
along the lines of preadv64v2(), which get done when somebody notices
a problem.
Linus, any advice here?
--Andy
^ permalink raw reply
* Re: [PATCH v10 4/4] samples: add an example of seccomp user trap
From: Serge E. Hallyn @ 2018-12-11 22:24 UTC (permalink / raw)
To: Tycho Andersen
Cc: Kees Cook, Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api
In-Reply-To: <20181209182414.30862-5-tycho@tycho.ws>
On Sun, Dec 09, 2018 at 11:24:14AM -0700, Tycho Andersen wrote:
> The idea here is just to give a demonstration of how one could safely use
> the SECCOMP_RET_USER_NOTIF feature to do mount policies. This particular
> policy is (as noted in the comment) not very interesting, but it serves to
> illustrate how one might apply a policy dodging the various TOCTOU issues.
>
> Signed-off-by: Tycho Andersen <tycho@tycho.ws>
> CC: Kees Cook <keescook@chromium.org>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Eric W. Biederman <ebiederm@xmission.com>
> CC: "Serge E. Hallyn" <serge@hallyn.com>
> CC: Christian Brauner <christian@brauner.io>
> CC: Tyler Hicks <tyhicks@canonical.com>
> CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
> ---
> v5: new in v5
> v7: updates for v7 API changes
> v8: * add some more comments about what's happening in main() (Kees)
> * move from ptrace API to SECCOMP_FILTER_FLAG_NEW_LISTENER
> v9: * s/mknod/mount in error message
> * switch to the SECCOMP_GET_NOTIF_SIZES API
> * add a note about getting ENOENT from SECCOMP_IOCTL_NOTIF_SEND
> ---
> samples/seccomp/.gitignore | 1 +
> samples/seccomp/Makefile | 7 +-
> samples/seccomp/user-trap.c | 375 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 382 insertions(+), 1 deletion(-)
>
> diff --git a/samples/seccomp/.gitignore b/samples/seccomp/.gitignore
> index 78fb78184291..d1e2e817d556 100644
> --- a/samples/seccomp/.gitignore
> +++ b/samples/seccomp/.gitignore
> @@ -1,3 +1,4 @@
> bpf-direct
> bpf-fancy
> dropper
> +user-trap
> diff --git a/samples/seccomp/Makefile b/samples/seccomp/Makefile
> index cf34ff6b4065..4920903c8009 100644
> --- a/samples/seccomp/Makefile
> +++ b/samples/seccomp/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> ifndef CROSS_COMPILE
> -hostprogs-$(CONFIG_SAMPLE_SECCOMP) := bpf-fancy dropper bpf-direct
> +hostprogs-$(CONFIG_SAMPLE_SECCOMP) := bpf-fancy dropper bpf-direct user-trap
>
> HOSTCFLAGS_bpf-fancy.o += -I$(objtree)/usr/include
> HOSTCFLAGS_bpf-fancy.o += -idirafter $(objtree)/include
> @@ -16,6 +16,10 @@ HOSTCFLAGS_bpf-direct.o += -I$(objtree)/usr/include
> HOSTCFLAGS_bpf-direct.o += -idirafter $(objtree)/include
> bpf-direct-objs := bpf-direct.o
>
> +HOSTCFLAGS_user-trap.o += -I$(objtree)/usr/include
> +HOSTCFLAGS_user-trap.o += -idirafter $(objtree)/include
> +user-trap-objs := user-trap.o
> +
> # Try to match the kernel target.
> ifndef CONFIG_64BIT
>
> @@ -33,6 +37,7 @@ HOSTCFLAGS_bpf-fancy.o += $(MFLAG)
> HOSTLDLIBS_bpf-direct += $(MFLAG)
> HOSTLDLIBS_bpf-fancy += $(MFLAG)
> HOSTLDLIBS_dropper += $(MFLAG)
> +HOSTLDLIBS_user-trap += $(MFLAG)
> endif
> always := $(hostprogs-m)
> endif
> diff --git a/samples/seccomp/user-trap.c b/samples/seccomp/user-trap.c
> new file mode 100644
> index 000000000000..61267cb59c8e
> --- /dev/null
> +++ b/samples/seccomp/user-trap.c
> @@ -0,0 +1,375 @@
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <stddef.h>
> +#include <sys/sysmacros.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <sys/socket.h>
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +#include <sys/syscall.h>
> +#include <sys/user.h>
> +#include <sys/ioctl.h>
> +#include <sys/ptrace.h>
> +#include <sys/mount.h>
> +#include <linux/limits.h>
> +#include <linux/filter.h>
> +#include <linux/seccomp.h>
> +
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
> +
> +static int seccomp(unsigned int op, unsigned int flags, void *args)
> +{
> + errno = 0;
> + return syscall(__NR_seccomp, op, flags, args);
> +}
> +
> +static int send_fd(int sock, int fd)
> +{
> + struct msghdr msg = {};
> + struct cmsghdr *cmsg;
> + char buf[CMSG_SPACE(sizeof(int))] = {0}, c = 'c';
> + struct iovec io = {
> + .iov_base = &c,
> + .iov_len = 1,
> + };
> +
> + msg.msg_iov = &io;
> + msg.msg_iovlen = 1;
> + msg.msg_control = buf;
> + msg.msg_controllen = sizeof(buf);
> + cmsg = CMSG_FIRSTHDR(&msg);
> + cmsg->cmsg_level = SOL_SOCKET;
> + cmsg->cmsg_type = SCM_RIGHTS;
> + cmsg->cmsg_len = CMSG_LEN(sizeof(int));
> + *((int *)CMSG_DATA(cmsg)) = fd;
> + msg.msg_controllen = cmsg->cmsg_len;
> +
> + if (sendmsg(sock, &msg, 0) < 0) {
> + perror("sendmsg");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int recv_fd(int sock)
> +{
> + struct msghdr msg = {};
> + struct cmsghdr *cmsg;
> + char buf[CMSG_SPACE(sizeof(int))] = {0}, c = 'c';
> + struct iovec io = {
> + .iov_base = &c,
> + .iov_len = 1,
> + };
> +
> + msg.msg_iov = &io;
> + msg.msg_iovlen = 1;
> + msg.msg_control = buf;
> + msg.msg_controllen = sizeof(buf);
> +
> + if (recvmsg(sock, &msg, 0) < 0) {
> + perror("recvmsg");
> + return -1;
> + }
> +
> + cmsg = CMSG_FIRSTHDR(&msg);
> +
> + return *((int *)CMSG_DATA(cmsg));
> +}
> +
> +static int user_trap_syscall(int nr, unsigned int flags)
> +{
> + struct sock_filter filter[] = {
> + BPF_STMT(BPF_LD+BPF_W+BPF_ABS,
> + offsetof(struct seccomp_data, nr)),
> + BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, nr, 0, 1),
> + BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_USER_NOTIF),
> + BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),
> + };
> +
> + struct sock_fprog prog = {
> + .len = (unsigned short)ARRAY_SIZE(filter),
> + .filter = filter,
> + };
> +
> + return seccomp(SECCOMP_SET_MODE_FILTER, flags, &prog);
> +}
> +
> +static int handle_req(struct seccomp_notif *req,
> + struct seccomp_notif_resp *resp, int listener)
> +{
> + char path[PATH_MAX], source[PATH_MAX], target[PATH_MAX];
> + int ret = -1, mem;
> +
> + resp->id = req->id;
> + resp->error = -EPERM;
> + resp->val = 0;
> +
> + if (req->data.nr != __NR_mount) {
> + fprintf(stderr, "huh? trapped something besides mount? %d\n", req->data.nr);
> + return -1;
> + }
> +
> + /* Only allow bind mounts. */
> + if (!(req->data.args[3] & MS_BIND))
> + return 0;
> +
> + /*
> + * Ok, let's read the task's memory to see where they wanted their
> + * mount to go.
> + */
> + snprintf(path, sizeof(path), "/proc/%d/mem", req->pid);
> + mem = open(path, O_RDONLY);
> + if (mem < 0) {
> + perror("open mem");
> + return -1;
> + }
> +
> + /*
> + * Now we avoid a TOCTOU: we referred to a pid by its pid, but since
> + * the pid that made the syscall may have died, we need to confirm that
> + * the pid is still valid after we open its /proc/pid/mem file. We can
> + * ask the listener fd this as follows.
> + *
> + * Note that this check should occur *after* any task-specific
> + * resources are opened, to make sure that the task has not died and
> + * we're not wrongly reading someone else's state in order to make
> + * decisions.
> + */
> + if (ioctl(listener, SECCOMP_IOCTL_NOTIF_ID_VALID, &req->id) < 0) {
> + fprintf(stderr, "task died before we could map its memory\n");
> + goto out;
> + }
> +
> + /*
> + * Phew, we've got the right /proc/pid/mem. Now we can read it. Note
> + * that to avoid another TOCTOU, we should read all of the pointer args
> + * before we decide to allow the syscall.
> + */
> + if (lseek(mem, req->data.args[0], SEEK_SET) < 0) {
> + perror("seek");
> + goto out;
> + }
> +
> + ret = read(mem, source, sizeof(source));
> + if (ret < 0) {
> + perror("read");
> + goto out;
> + }
> +
> + if (lseek(mem, req->data.args[1], SEEK_SET) < 0) {
> + perror("seek");
> + goto out;
> + }
> +
> + ret = read(mem, target, sizeof(target));
> + if (ret < 0) {
> + perror("read");
> + goto out;
> + }
> +
> + /*
> + * Our policy is to only allow bind mounts inside /tmp. This isn't very
> + * interesting, because we could do unprivlieged bind mounts with user
> + * namespaces already, but you get the idea.
> + */
> + if (!strncmp(source, "/tmp", 4) && !strncmp(target, "/tmp", 4)) {
Of course the host could have a /tmproot or somesuch, so you might want to
check the first 5 characters :)
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Thorsten Glaser @ 2018-12-11 21:59 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, Rich Felker, x32, Arnd Bergmann, Will Deacon,
Catalin Marinas, Linus Torvalds
In-Reply-To: <70bb54b2-8ed3-b5ee-c02d-6ef66c4f27eb@physik.fu-berlin.de>
John Paul Adrian Glaubitz dixit:
>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
Yes, but more and more ${foo}64ilp32 architectures are popping up.
>Additionally, x32 support in many applications is either rudimentary
If a signal is sent that this kind of architectures will stay, some
people might be convinced to fix that.
>It's also that the performance benefits of x32 are often eaten up by
>the fact that none of the scripted languages that I know of provide
Non-JITted languages like yours truly’s shell do benefit from it,
though. (mksh works just fine on LP64 but its internal structures
pack massively better on ILP32, for example.)
>If x32 is eventually to be removed, we should also take care of removing
>x32 support from userland code. From the top of my head, this would at least
I don’t think so. The patches also contain
– stuff to support 64-bit time_t on 32-bit architectures, e.g:
- bugfixes like printf("%lld", (long long)timet_value) instead
of assuming time_t fits into a long (also important for other
operating systems…)
- generally switching from generic types like long to specific
types like size_t, ptrdiff_t, etc.
- there was one more but after having written two eMails I forgot it
- oh and, of course, they lay the base for e.g. amd64ilp32 support
bye,
//mirabilos
--
FWIW, I'm quite impressed with mksh interactively. I thought it was much
*much* more bare bones. But it turns out it beats the living hell out of
ksh93 in that respect. I'd even consider it for my daily use if I hadn't
wasted half my life on my zsh setup. :-) -- Frank Terbeck in #!/bin/mksh
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Thorsten Glaser @ 2018-12-11 21:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Lutomirski, 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>
Note: please keep me in Cc because I am not subscribed.
Linus Torvalds dixit:
>I'm not opposed to trying to sunset the support, but let's see who complains..
I will hereby complain.
I’m using Debian/x32 on my main desktop at work, and do
occasionally help out with porting issues. It’s a good
way to make more out of 64-bit machines without going
all 64 bit; it’s also helped me find bugs in software.
It’s a nice architectural idea, and a way forward for
things that are constricted to 32 bits while opening
up stuff like 64-bit time_t without taking up half the
available CPU registers (while more than doubling the
number of the available CPU registers, too).
I was also considering investing a nontrivial amount of
work into porting klibc to x32, since hpa does not wish
to do it himself. Thankfully I have only done a bit yet.
Furthermore, x32 was the first of the many *64ilp32
architectures; I know I’ve seen amd64ilp32 and at least
one other I don’t recall. It will have prototyped many
of the problems users of these will run in, and I’d prefer
to keep it (completely selfish because I don’t wish to
have to crossgrade a whole system yet again).
Thanks for your consideration,
//mirabilos
--
When he found out that the m68k port was in a pretty bad shape, he did
not, like many before him, shrug and move on; instead, he took it upon
himself to start compiling things, just so he could compile his shell.
How's that for dedication. -- Wouter, about my Debian/m68k revival
^ permalink raw reply
* Re: [PATCH v5 24/25] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2018-12-11 20:27 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andy Lutomirski, Elvira Khabirova, Eugene Syromyatnikov,
Kees Cook, Jann Horn, linux-api, strace-devel, linux-kernel
In-Reply-To: <20181211162305.GA480@altlinux.org>
[-- Attachment #1: Type: text/plain, Size: 2478 bytes --]
On Tue, Dec 11, 2018 at 07:23:05PM +0300, Dmitry V. Levin wrote:
> On Tue, Dec 11, 2018 at 04:29:54PM +0100, Oleg Nesterov wrote:
> > On 12/10, Dmitry V. Levin wrote:
> > > On Mon, Dec 10, 2018 at 03:11:07PM +0100, Oleg Nesterov wrote:
> > > > On 12/10, Dmitry V. Levin wrote:
> > > > >
> > > > > +struct ptrace_syscall_info {
> > > > > + __u8 op; /* PTRACE_SYSCALL_INFO_* */
> > > > > + __u8 __pad0[3];
> > > > > + __u32 arch;
> > > > > + __u64 instruction_pointer;
> > > > > + __u64 stack_pointer;
> > > > > + __u64 frame_pointer;
> > > > > + union {
> > > > > + struct {
> > > > > + __u64 nr;
> > > > > + __u64 args[6];
> > > > > + } entry;
> > > > > + struct {
> > > > > + __s64 rval;
> > > > > + __u8 is_error;
> > > > > + __u8 __pad1[7];
> > > > > + } exit;
> > > > > + struct {
> > > > > + __u64 nr;
> > > > > + __u64 args[6];
> > > > > + __u32 ret_data;
> > > > > + __u8 __pad2[4];
> > > > > + } seccomp;
> > > > > + };
> > > > > +};
> > > >
> > > > Could you explain why ptrace_syscall_info needs __pad{0,1,2} ? I simply can't
> > > > understand why...
> > >
> > > I suppose the idea behind the use of these pads was to make the structure
> > > arch-independent.
> >
> > Still can't understand... are you saying that without (say) __pad2[4]
> > sizeof(ptrace_syscall_info) or offsetofend(ptrace_syscall_info, seccomp)
> > will depend on arch? Or what? I am just curious.
>
> Yes, without padding these sizes will depend on architecture:
>
> $ cat t.c
> #include <linux/types.h>
> int main() {
> struct s {
> __u64 nr;
> __u64 args[6];
> __u32 ret_data;
> };
> return sizeof(struct s);
> }
>
> $ gcc -m64 -Wall -O2 t.c && ./a.out; echo $?
> 64
> $ gcc -m32 -Wall -O2 t.c && ./a.out; echo $?
> 60
>
> This happens because __u64 has 32-bit alignment on some 32-bit
> architectures like x86.
>
> There is also m68k where __u32 has 16-bit alignment.
Said that, I think it would be better if PTRACE_GET_SYSCALL_INFO
did not take these trailing pads into account, e.g.
- return offsetofend(struct ptrace_syscall_info, seccomp);
+ return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
...
- return offsetofend(struct ptrace_syscall_info, exit);
+ return offsetofend(struct ptrace_syscall_info, exit.is_error);
The reason is that it would allow to fill these trailing pads with
something useful in the future.
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v5 24/25] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Dmitry V. Levin @ 2018-12-11 16:23 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Kees Cook, Jann Horn, linux-api-u79uwXL29TY76Z2rM5mHXA,
Eugene Syromyatnikov, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Andy Lutomirski, strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181211152953.GA8504-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2652 bytes --]
On Tue, Dec 11, 2018 at 04:29:54PM +0100, Oleg Nesterov wrote:
> On 12/10, Dmitry V. Levin wrote:
> > On Mon, Dec 10, 2018 at 03:11:07PM +0100, Oleg Nesterov wrote:
> > > On 12/10, Dmitry V. Levin wrote:
> > > >
> > > > +struct ptrace_syscall_info {
> > > > + __u8 op; /* PTRACE_SYSCALL_INFO_* */
> > > > + __u8 __pad0[3];
> > > > + __u32 arch;
> > > > + __u64 instruction_pointer;
> > > > + __u64 stack_pointer;
> > > > + __u64 frame_pointer;
> > > > + union {
> > > > + struct {
> > > > + __u64 nr;
> > > > + __u64 args[6];
> > > > + } entry;
> > > > + struct {
> > > > + __s64 rval;
> > > > + __u8 is_error;
> > > > + __u8 __pad1[7];
> > > > + } exit;
> > > > + struct {
> > > > + __u64 nr;
> > > > + __u64 args[6];
> > > > + __u32 ret_data;
> > > > + __u8 __pad2[4];
> > > > + } seccomp;
> > > > + };
> > > > +};
> > >
> > > Could you explain why ptrace_syscall_info needs __pad{0,1,2} ? I simply can't
> > > understand why...
> >
> > I suppose the idea behind the use of these pads was to make the structure
> > arch-independent.
>
> Still can't understand... are you saying that without (say) __pad2[4]
> sizeof(ptrace_syscall_info) or offsetofend(ptrace_syscall_info, seccomp)
> will depend on arch? Or what? I am just curious.
Yes, without padding these sizes will depend on architecture:
$ cat t.c
#include <linux/types.h>
int main() {
struct s {
__u64 nr;
__u64 args[6];
__u32 ret_data;
};
return sizeof(struct s);
}
$ gcc -m64 -Wall -O2 t.c && ./a.out; echo $?
64
$ gcc -m32 -Wall -O2 t.c && ./a.out; echo $?
60
This happens because __u64 has 32-bit alignment on some 32-bit
architectures like x86.
There is also m68k where __u32 has 16-bit alignment.
> > I don't think we really need to keep it exactly the same on all
> > architectures - the only practical requirement is to avoid any compat
> > issues, but I don't mind keeping the structure arch-independent.
>
> OK, but may be you can add a short comment to explain these pads.
Alternatively, we could use __attribute__((aligned(N))), e.g.
struct ptrace_syscall_info {
__u8 op; /* PTRACE_SYSCALL_INFO_* */
__u32 arch __attribute__((aligned(4)));
__u64 instruction_pointer;
__u64 stack_pointer;
union {
struct {
__u64 nr __attribute__((aligned(8)));
__u64 args[6];
} entry;
struct {
__s64 rval __attribute__((aligned(8)));
__u8 is_error;
} exit;
struct {
__u64 nr __attribute__((aligned(8)));
__u64 args[6];
__u32 ret_data;
} seccomp;
};
};
Do you prefer __attribute__((aligned(N))) to padding?
--
ldv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 137 bytes --]
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [PATCH v5 24/25] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Oleg Nesterov @ 2018-12-11 15:29 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Andy Lutomirski, Elvira Khabirova, Eugene Syromyatnikov,
Kees Cook, Jann Horn, linux-api, strace-devel, linux-kernel
In-Reply-To: <20181210162131.GG14149@altlinux.org>
On 12/10, Dmitry V. Levin wrote:
>
> On Mon, Dec 10, 2018 at 03:11:07PM +0100, Oleg Nesterov wrote:
> > On 12/10, Dmitry V. Levin wrote:
> > >
> > > +struct ptrace_syscall_info {
> > > + __u8 op; /* PTRACE_SYSCALL_INFO_* */
> > > + __u8 __pad0[3];
> > > + __u32 arch;
> > > + __u64 instruction_pointer;
> > > + __u64 stack_pointer;
> > > + __u64 frame_pointer;
> > > + union {
> > > + struct {
> > > + __u64 nr;
> > > + __u64 args[6];
> > > + } entry;
> > > + struct {
> > > + __s64 rval;
> > > + __u8 is_error;
> > > + __u8 __pad1[7];
> > > + } exit;
> > > + struct {
> > > + __u64 nr;
> > > + __u64 args[6];
> > > + __u32 ret_data;
> > > + __u8 __pad2[4];
> > > + } seccomp;
> > > + };
> > > +};
> >
> > Could you explain why ptrace_syscall_info needs __pad{0,1,2} ? I simply can't
> > understand why...
>
> I suppose the idea behind the use of these pads was to make the structure
> arch-independent.
Still can't understand... are you saying that without (say) __pad2[4]
sizeof(ptrace_syscall_info) or offsetofend(ptrace_syscall_info, seccomp)
will depend on arch? Or what? I am just curious.
> I don't think we really need to keep it exactly the same on all
> architectures - the only practical requirement is to avoid any compat
> issues, but I don't mind keeping the structure arch-independent.
OK, but may be you can add a short comment to explain these pads.
Oleg.
^ permalink raw reply
* [PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: Michal Hocko @ 2018-12-11 14:36 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-api, linux-mm, LKML, Michal Hocko, Vlastimil Babka
In-Reply-To: <20181211143641.3503-1-mhocko@kernel.org>
From: Michal Hocko <mhocko@suse.com>
David Rientjes has reported that 1860033237d4 ("mm: make
PR_SET_THP_DISABLE immediately active") has changed the way how
we report THPable VMAs to the userspace. Their monitoring tool is
triggering false alarms on PR_SET_THP_DISABLE tasks because it considers
an insufficient THP usage as a memory fragmentation resp. memory
pressure issue.
Before the said commit each newly created VMA inherited VM_NOHUGEPAGE
flag and that got exposed to the userspace via /proc/<pid>/smaps file.
This implementation had its downsides as explained in the commit message
but it is true that the userspace doesn't have any means to query for
the process wide THP enabled/disabled status.
PR_SET_THP_DISABLE is a process wide flag so it makes a lot of sense
to export in the process wide context rather than per-vma. Introduce
a new field to /proc/<pid>/status which export this status. If
PR_SET_THP_DISABLE is used then it reports false same as when the THP is
not compiled in. It doesn't consider the global THP status because we
already export that information via sysfs
Fixes: 1860033237d4 ("mm: make PR_SET_THP_DISABLE immediately active")
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Documentation/filesystems/proc.txt | 3 +++
fs/proc/array.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index cd465304bec4..b24fd9bccc99 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -182,6 +182,7 @@ For example, to get the status information of a process, all you have to do is
VmSwap: 0 kB
HugetlbPages: 0 kB
CoreDumping: 0
+ THP_enabled: 1
Threads: 1
SigQ: 0/28578
SigPnd: 0000000000000000
@@ -256,6 +257,8 @@ Table 1-2: Contents of the status files (as of 4.8)
HugetlbPages size of hugetlb memory portions
CoreDumping process's memory is currently being dumped
(killing the process may lead to a corrupted core)
+ THP_enabled process is allowed to use THP (returns 0 when
+ PR_SET_THP_DISABLE is set on the process
Threads number of threads
SigQ number of signals queued/max. number for queue
SigPnd bitmap of pending signals for the thread
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 0ceb3b6b37e7..9d428d5a0ac8 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -392,6 +392,15 @@ static inline void task_core_dumping(struct seq_file *m, struct mm_struct *mm)
seq_putc(m, '\n');
}
+static inline void task_thp_status(struct seq_file *m, struct mm_struct *mm)
+{
+ bool thp_enabled = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE);
+
+ if (thp_enabled)
+ thp_enabled = !test_bit(MMF_DISABLE_THP, &mm->flags);
+ seq_printf(m, "THP_enabled:\t%d\n", thp_enabled);
+}
+
int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
struct pid *pid, struct task_struct *task)
{
@@ -406,6 +415,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
if (mm) {
task_mem(m, mm);
task_core_dumping(m, mm);
+ task_thp_status(m, mm);
mmput(mm);
}
task_sig(m, task);
--
2.19.2
^ permalink raw reply related
* [PATCH 2/3] mm, thp, proc: report THP eligibility for each vma
From: Michal Hocko @ 2018-12-11 14:36 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-api, linux-mm, LKML, Michal Hocko, Vlastimil Babka
In-Reply-To: <20181211143641.3503-1-mhocko@kernel.org>
From: Michal Hocko <mhocko@suse.com>
Userspace falls short when trying to find out whether a specific memory
range is eligible for THP. There are usecases that would like to know
that
http://lkml.kernel.org/r/alpine.DEB.2.21.1809251248450.50347@chino.kir.corp.google.com
: This is used to identify heap mappings that should be able to fault thp
: but do not, and they normally point to a low-on-memory or fragmentation
: issue.
The only way to deduce this now is to query for hg resp. nh flags and
confronting the state with the global setting. Except that there is
also PR_SET_THP_DISABLE that might change the picture. So the final
logic is not trivial. Moreover the eligibility of the vma depends on
the type of VMA as well. In the past we have supported only anononymous
memory VMAs but things have changed and shmem based vmas are supported
as well these days and the query logic gets even more complicated
because the eligibility depends on the mount option and another global
configuration knob.
Simplify the current state and report the THP eligibility in
/proc/<pid>/smaps for each existing vma. Reuse transparent_hugepage_enabled
for this purpose. The original implementation of this function assumes
that the caller knows that the vma itself is supported for THP so make
the core checks into __transparent_hugepage_enabled and use it for
existing callers. __show_smap just use the new transparent_hugepage_enabled
which also checks the vma support status (please note that this one has
to be out of line due to include dependency issues).
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Documentation/filesystems/proc.txt | 3 +++
fs/proc/task_mmu.c | 2 ++
include/linux/huge_mm.h | 13 ++++++++++++-
mm/huge_memory.c | 12 +++++++++++-
mm/memory.c | 4 ++--
5 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 2a4e63f5122c..cd465304bec4 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -425,6 +425,7 @@ SwapPss: 0 kB
KernelPageSize: 4 kB
MMUPageSize: 4 kB
Locked: 0 kB
+THPeligible: 0
VmFlags: rd ex mr mw me dw
the first of these lines shows the same information as is displayed for the
@@ -462,6 +463,8 @@ replaced by copy-on-write) part of the underlying shmem object out on swap.
"SwapPss" shows proportional swap share of this mapping. Unlike "Swap", this
does not take into account swapped out page of underlying shmem objects.
"Locked" indicates whether the mapping is locked in memory or not.
+"THPeligible" indicates whether the mapping is eligible for THP pages - 1 if
+true, 0 otherwise.
"VmFlags" field deserves a separate description. This member represents the kernel
flags associated with the particular virtual memory area in two letter encoded
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 47c3764c469b..c9f160eb9fbc 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -790,6 +790,8 @@ static int show_smap(struct seq_file *m, void *v)
__show_smap(m, &mss);
+ seq_printf(m, "THPeligible: %d\n", transparent_hugepage_enabled(vma));
+
if (arch_pkeys_enabled())
seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
show_smap_vma_flags(m, vma);
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 4663ee96cf59..381e872bfde0 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -93,7 +93,11 @@ extern bool is_vma_temporary_stack(struct vm_area_struct *vma);
extern unsigned long transparent_hugepage_flags;
-static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+/*
+ * to be used on vmas which are known to support THP.
+ * Use transparent_hugepage_enabled otherwise
+ */
+static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
{
if (vma->vm_flags & VM_NOHUGEPAGE)
return false;
@@ -117,6 +121,8 @@ static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
return false;
}
+bool transparent_hugepage_enabled(struct vm_area_struct *vma);
+
#define transparent_hugepage_use_zero_page() \
(transparent_hugepage_flags & \
(1<<TRANSPARENT_HUGEPAGE_USE_ZERO_PAGE_FLAG))
@@ -257,6 +263,11 @@ static inline bool thp_migration_supported(void)
#define hpage_nr_pages(x) 1
+static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+ return false;
+}
+
static inline bool transparent_hugepage_enabled(struct vm_area_struct *vma)
{
return false;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 55478ab3c83b..f64733c23067 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -62,6 +62,16 @@ static struct shrinker deferred_split_shrinker;
static atomic_t huge_zero_refcount;
struct page *huge_zero_page __read_mostly;
+bool transparent_hugepage_enabled(struct vm_area_struct *vma)
+{
+ if (vma_is_anonymous(vma))
+ return __transparent_hugepage_enabled(vma);
+ if (shmem_mapping(vma->vm_file->f_mapping) && shmem_huge_enabled(vma))
+ return __transparent_hugepage_enabled(vma);
+
+ return false;
+}
+
static struct page *get_huge_zero_page(void)
{
struct page *zero_page;
@@ -1303,7 +1313,7 @@ vm_fault_t do_huge_pmd_wp_page(struct vm_fault *vmf, pmd_t orig_pmd)
get_page(page);
spin_unlock(vmf->ptl);
alloc:
- if (transparent_hugepage_enabled(vma) &&
+ if (__transparent_hugepage_enabled(vma) &&
!transparent_hugepage_debug_cow()) {
huge_gfp = alloc_hugepage_direct_gfpmask(vma, haddr);
new_page = alloc_pages_vma(huge_gfp, HPAGE_PMD_ORDER, vma,
diff --git a/mm/memory.c b/mm/memory.c
index 4ad2d293ddc2..3c2716ec7fbd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3830,7 +3830,7 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
vmf.pud = pud_alloc(mm, p4d, address);
if (!vmf.pud)
return VM_FAULT_OOM;
- if (pud_none(*vmf.pud) && transparent_hugepage_enabled(vma)) {
+ if (pud_none(*vmf.pud) && __transparent_hugepage_enabled(vma)) {
ret = create_huge_pud(&vmf);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
@@ -3856,7 +3856,7 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma,
vmf.pmd = pmd_alloc(mm, vmf.pud, address);
if (!vmf.pmd)
return VM_FAULT_OOM;
- if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
+ if (pmd_none(*vmf.pmd) && __transparent_hugepage_enabled(vma)) {
ret = create_huge_pmd(&vmf);
if (!(ret & VM_FAULT_FALLBACK))
return ret;
--
2.19.2
^ permalink raw reply related
* [PATCH 1/3] mm, proc: be more verbose about unstable VMA flags in /proc/<pid>/smaps
From: Michal Hocko @ 2018-12-11 14:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-api, linux-mm, LKML, Michal Hocko, Dan Williams,
David Rientjes, Jan Kara, Mike Rapoport, Vlastimil Babka
In-Reply-To: <20181211143641.3503-1-mhocko@kernel.org>
From: Michal Hocko <mhocko@suse.com>
Even though vma flags exported via /proc/<pid>/smaps are explicitly
documented to be not guaranteed for future compatibility the warning
doesn't go far enough because it doesn't mention semantic changes to
those flags. And they are important as well because these flags are
a deep implementation internal to the MM code and the semantic might
change at any time.
Let's consider two recent examples:
http://lkml.kernel.org/r/20181002100531.GC4135@quack2.suse.cz
: commit e1fb4a086495 "dax: remove VM_MIXEDMAP for fsdax and device dax" has
: removed VM_MIXEDMAP flag from DAX VMAs. Now our testing shows that in the
: mean time certain customer of ours started poking into /proc/<pid>/smaps
: and looks at VMA flags there and if VM_MIXEDMAP is missing among the VMA
: flags, the application just fails to start complaining that DAX support is
: missing in the kernel.
http://lkml.kernel.org/r/alpine.DEB.2.21.1809241054050.224429@chino.kir.corp.google.com
: Commit 1860033237d4 ("mm: make PR_SET_THP_DISABLE immediately active")
: introduced a regression in that userspace cannot always determine the set
: of vmas where thp is ineligible.
: Userspace relies on the "nh" flag being emitted as part of /proc/pid/smaps
: to determine if a vma is eligible to be backed by hugepages.
: Previous to this commit, prctl(PR_SET_THP_DISABLE, 1) would cause thp to
: be disabled and emit "nh" as a flag for the corresponding vmas as part of
: /proc/pid/smaps. After the commit, thp is disabled by means of an mm
: flag and "nh" is not emitted.
: This causes smaps parsing libraries to assume a vma is eligible for thp
: and ends up puzzling the user on why its memory is not backed by thp.
In both cases userspace was relying on a semantic of a specific VMA
flag. The primary reason why that happened is a lack of a proper
internface. While this has been worked on and it will be fixed properly,
it seems that our wording could see some refinement and be more vocal
about semantic aspect of these flags as well.
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: David Rientjes <rientjes@google.com>
Acked-by: Jan Kara <jack@suse.cz>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: David Rientjes <rientjes@google.com>
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Documentation/filesystems/proc.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 12a5e6e693b6..2a4e63f5122c 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -496,7 +496,9 @@ flags associated with the particular virtual memory area in two letter encoded
Note that there is no guarantee that every flag and associated mnemonic will
be present in all further kernel releases. Things get changed, the flags may
-be vanished or the reverse -- new added.
+be vanished or the reverse -- new added. Interpretation of their meaning
+might change in future as well. So each consumer of these flags has to
+follow each specific kernel version for the exact semantic.
This file is only present if the CONFIG_MMU kernel configuration option is
enabled.
--
2.19.2
^ permalink raw reply related
* [PATCH 0/3] THP eligibility reporting via proc
From: Michal Hocko @ 2018-12-11 14:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-api, linux-mm, LKML, Dan Williams, David Rientjes, Jan Kara,
Michal Hocko, Mike Rapoport, Vlastimil Babka
Hi,
I've posted this as an RFC [1] and there didn't seem to be any pushback
so I am posting it for inclusion. If there are any concerns, please
speak up.
Original cover:
This series of three patches aims at making THP eligibility reporting
much more robust and long term sustainable. The trigger for the change
is a regression report [2] and the long follow up discussion. In short
the specific application didn't have good API to query whether a particular
mapping can be backed by THP so it has used VMA flags to workaround that.
These flags represent a deep internal state of VMAs and as such they should
be used by userspace with a great deal of caution.
A similar has happened for [3] when users complained that VM_MIXEDMAP is
no longer set on DAX mappings. Again a lack of a proper API led to an
abuse.
The first patch in the series tries to emphasise that that the semantic
of flags might change and any application consuming those should be really
careful.
The remaining two patches provide a more suitable interface to address [2]
and provide a consistent API to query the THP status both for each VMA
and process wide as well.
[1] http://lkml.kernel.org/r/20181120103515.25280-1-mhocko@kernel.org
[2] http://lkml.kernel.org/r/http://lkml.kernel.org/r/alpine.DEB.2.21.1809241054050.224429@chino.kir.corp.google.com
[3] http://lkml.kernel.org/r/20181002100531.GC4135@quack2.suse.cz
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Catalin Marinas @ 2018-12-11 11:52 UTC (permalink / raw)
To: Florian Weimer
Cc: Arnd Bergmann, Andy Lutomirski, H.J. Lu, the arch/x86 maintainers,
Linux Kernel Mailing List, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, vapier, Rich Felker, x32,
Will Deacon, Linus Torvalds
In-Reply-To: <87efaoxpix.fsf@oldenburg2.str.redhat.com>
On Tue, Dec 11, 2018 at 12:37:42PM +0100, Florian Weimer wrote:
> * Catalin Marinas:
> > On Tue, Dec 11, 2018 at 10:02:45AM +0100, Arnd Bergmann wrote:
> >> On Tue, Dec 11, 2018 at 6:35 AM Andy Lutomirski <luto@kernel.org> wrote:
> >> > I tried to understand what's going on. As far as I can tell, most of
> >> > the magic is the fact that __kernel_long_t and __kernel_ulong_t are
> >> > 64-bit as seen by x32 user code. This means that a decent number of
> >> > uapi structures are the same on x32 and x86_64. Syscalls that only
> >> > use structures like this should route to the x86_64 entry points. But
> >> > the implementation is still highly dubious -- in_compat_syscall() will
> >> > be *true* in such system calls,
> >>
> >> I think the fundamental issue was that the intention had always been
> >> to use only the 64-bit entry points for system calls, but the most
> >> complex one we have -- ioctl() -- has to use the compat entry point
> >> because device drivers define their own data structures using 'long'
> >> and pointer members and they need translation, as well as
> >> matching in_compat_syscall() checks. This in turn breaks down
> >> again whenever a driver defines an ioctl command that takes
> >> a __kernel_long_t or a derived type like timespec as its argument.
> >
> > With arm64 ILP32 we tried to avoid the ioctl() problem by having
> > __kernel_long_t 32-bit, IOW mimicking the arm32 ABI (compat). The
> > biggest pain point is signals where the state is completely different
> > from arm32 (more, wider registers) and can't be dealt with by the compat
> > layer.
>
> I would expect to approach this from the opposite direction: use 64-bit
> types in places where the 64-bit kernel interface uses 64-bit types.
> After all, not everyone who is interested in ILP32 has a companion
> 32-bit architecture which could serve as a model for the application
> ABI.
I fully agree with you that if someone wants ILP32 for a 64-bit only
architecture, they should use the 64-bit kernel interface and ensure
POSIX is adjusted.
In the arm64 context, both options were discussed with the libc
community complaining that a partial 64-bit syscall ABI breaks POSIX
while the potential users were just asking for a 32-bit ABI to run their
existing software stack on ARMv8 machines without native 32-bit support
(until they complete the migration to 64-bit).
> (If there are conflicts with POSIX, then POSIX needs to be fixed to
> support this.)
This would have been nice but no-one volunteered and, more importantly,
there was no conclusive argument that ARM ILP32 is better than LP64
(well, apart from a minority of benchmarks) and something that people
would want to migrate to. Given that the only credible case made was
about legacy code, we decided to go ahead with a (mostly) compat 32-bit
ABI.
--
Catalin
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Florian Weimer @ 2018-12-11 11:37 UTC (permalink / raw)
To: Catalin Marinas
Cc: Arnd Bergmann, Andy Lutomirski, H.J. Lu, the arch/x86 maintainers,
Linux Kernel Mailing List, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, vapier, Rich Felker, x32,
Will Deacon, Linus Torvalds
In-Reply-To: <20181211113230.GB35824@arrakis.emea.arm.com>
* Catalin Marinas:
> On Tue, Dec 11, 2018 at 10:02:45AM +0100, Arnd Bergmann wrote:
>> On Tue, Dec 11, 2018 at 6:35 AM Andy Lutomirski <luto@kernel.org> wrote:
>> > I tried to understand what's going on. As far as I can tell, most of
>> > the magic is the fact that __kernel_long_t and __kernel_ulong_t are
>> > 64-bit as seen by x32 user code. This means that a decent number of
>> > uapi structures are the same on x32 and x86_64. Syscalls that only
>> > use structures like this should route to the x86_64 entry points. But
>> > the implementation is still highly dubious -- in_compat_syscall() will
>> > be *true* in such system calls,
>>
>> I think the fundamental issue was that the intention had always been
>> to use only the 64-bit entry points for system calls, but the most
>> complex one we have -- ioctl() -- has to use the compat entry point
>> because device drivers define their own data structures using 'long'
>> and pointer members and they need translation, as well as
>> matching in_compat_syscall() checks. This in turn breaks down
>> again whenever a driver defines an ioctl command that takes
>> a __kernel_long_t or a derived type like timespec as its argument.
>
> With arm64 ILP32 we tried to avoid the ioctl() problem by having
> __kernel_long_t 32-bit, IOW mimicking the arm32 ABI (compat). The
> biggest pain point is signals where the state is completely different
> from arm32 (more, wider registers) and can't be dealt with by the compat
> layer.
I would expect to approach this from the opposite direction: use 64-bit
types in places where the 64-bit kernel interface uses 64-bit types.
After all, not everyone who is interested in ILP32 has a companion
32-bit architecture which could serve as a model for the application
ABI.
(If there are conflicts with POSIX, then POSIX needs to be fixed to
support this.)
Thanks,
Florian
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Catalin Marinas @ 2018-12-11 11:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andy Lutomirski, H.J. Lu, the arch/x86 maintainers,
Linux Kernel Mailing List, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, vapier,
Rich Felker, x32, Will Deacon, Linus Torvalds
In-Reply-To: <CAK8P3a0adxnXr9JLfpR5MBeEJXSCK2Rt4w7NFLUUwO6s6LBVPQ@mail.gmail.com>
On Tue, Dec 11, 2018 at 10:02:45AM +0100, Arnd Bergmann wrote:
> On Tue, Dec 11, 2018 at 6:35 AM Andy Lutomirski <luto@kernel.org> wrote:
> > I tried to understand what's going on. As far as I can tell, most of
> > the magic is the fact that __kernel_long_t and __kernel_ulong_t are
> > 64-bit as seen by x32 user code. This means that a decent number of
> > uapi structures are the same on x32 and x86_64. Syscalls that only
> > use structures like this should route to the x86_64 entry points. But
> > the implementation is still highly dubious -- in_compat_syscall() will
> > be *true* in such system calls,
>
> I think the fundamental issue was that the intention had always been
> to use only the 64-bit entry points for system calls, but the most
> complex one we have -- ioctl() -- has to use the compat entry point
> because device drivers define their own data structures using 'long'
> and pointer members and they need translation, as well as
> matching in_compat_syscall() checks. This in turn breaks down
> again whenever a driver defines an ioctl command that takes
> a __kernel_long_t or a derived type like timespec as its argument.
With arm64 ILP32 we tried to avoid the ioctl() problem by having
__kernel_long_t 32-bit, IOW mimicking the arm32 ABI (compat). The
biggest pain point is signals where the state is completely different
from arm32 (more, wider registers) and can't be dealt with by the compat
layer.
Fortunately, we haven't merge it yet as we have the same dilemma about
real users and who's going to regularly test the ABI in the long run. In
the meantime, watching this thread with interest ;).
--
Catalin
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: John Paul Adrian Glaubitz @ 2018-12-11 10:44 UTC (permalink / raw)
To: Florian Weimer
Cc: Andy Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Mike Frysinger, H. J. Lu,
Rich Felker, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <87in00z6ux.fsf@oldenburg2.str.redhat.com>
On 12/11/18 11:37 AM, Florian Weimer wrote:
> Note that OpenJDK (well, Hotspot) has its own 32-bit pointer support for
> the Java heap (compressed oops), so only the native code parts (and JNI)
> benefit from x32 anyway.
Yeah, I was actually only talking about this single commit by me to add
Zero build support for x86_64-linux-gnux32:
> http://hg.openjdk.java.net/jdk/jdk/rev/9ce4a0d718c7
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: Security modules and sending signals within the same process
From: Florian Weimer @ 2018-12-11 10:42 UTC (permalink / raw)
To: Stephen Smalley
Cc: Arnd Bergmann, selinux-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, apparmor-nLRlyDuq1AZFpShjVBNYrg,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin
In-Reply-To: <e754fa24-899b-f77e-f226-0f070ebad132-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
* Stephen Smalley:
> Looks like commit 065add3941bd ("signals: check_kill_permission():
> don't check creds if same_thread_group()") skipped the uid-based
> checks if the sender and target were in the same thread group, but not
> the security hook call. One could argue that the security hook call
> ought to be skipped in that case as well using the same rationale
> given in that commit. Nothing appears to guarantee the property you
> state above for security_task_kill implementations, although none of
> the in-tree users are based on uids or gids so setresuid/setresgid
> shouldn't affect them.
Okay, thanks, so it looks like I don't have to do anything special to
support thread cancellation.
Florian
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
^ permalink raw reply
* Re: [RFC PATCH glibc 1/4] glibc: Perform rseq(2) registration at nptl init and thread creation (v4)
From: Florian Weimer @ 2018-12-11 10:40 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Carlos O'Donell, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, Rich Felker,
linux-kernel, linux-api
In-Reply-To: <20181204192141.4684-1-mathieu.desnoyers@efficios.com>
* Mathieu Desnoyers:
> I want to keep the __rseq_refcount symbol so out-of-libc users can
> register rseq if they are linked against a pre-2.29 libc.
Sorry, I was confused.
> diff --git a/csu/Makefile b/csu/Makefile
> index 88fc77662e..81d471587f 100644
> --- a/csu/Makefile
> +++ b/csu/Makefile
> @@ -28,7 +28,7 @@ include ../Makeconfig
>
> routines = init-first libc-start $(libc-init) sysdep version check_fds \
> libc-tls elf-init dso_handle
> -aux = errno
> +aux = errno rseq
> elide-routines.os = libc-tls
> static-only-routines = elf-init
> csu-dummies = $(filter-out $(start-installed-name),crt1.o Mcrt1.o)
Do we plan to add Hurd support for this?
> diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
> new file mode 100644
> index 0000000000..2367926def
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/rseq-internal.h
> +#define RSEQ_SIG 0x53053053
What's this? This needs a comment.
> +extern __thread volatile struct rseq __rseq_abi
> +__attribute__ ((tls_model ("initial-exec")));
> +
> +extern __thread volatile uint32_t __rseq_refcount
> +__attribute__ ((tls_model ("initial-exec")));
The volatile qualifier needs justification in a comment. (Usually,
volatile is wrong. and it is difficult to get rid of it.)
We need to document these public symbols somewhere. There should be an
installed header file.
> diff --git a/nptl/Versions b/nptl/Versions
> index e7f691da7a..f7890f73fc 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -277,6 +277,10 @@ libpthread {
> cnd_timedwait; cnd_wait; tss_create; tss_delete; tss_get; tss_set;
> }
>
> + GLIBC_2.29 {
> + __rseq_refcount;
> + }
Why put this into libpthread, and __rseq_abi into libc?
What, exactly, is the benefit of having __rseq_refcount defined by
glibc? Have you actually got this working? If an rseq library is
linked against glibc 2.29, it will reference the GLIBC_2.29 symbol
version, so it cannot be loaded by older glibcs. In this case,
__rseq_refcount is not needed.
If you build against pre-2.29, then the __rseq_refcount symbol will be
unversioned. But then you don't need it glibc, either.
So it seems to me that the addition to glibc is useless in both
scenarios. Am I missing something?
By the way, you could avoid the need for unregistration if you allocated
the rseq areas persistently, index by TID. They are quite small, so
with the typical PID range, maybe the wasted memory due to changing TIDs
would be acceptable?
I guess things would be so much easier if the kernel simply provided a
means to obtain the address of a previously registered rseq area.
Thanks,
Florian
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Florian Weimer @ 2018-12-11 10:37 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: Andy Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Mike Frysinger, H. J. Lu,
Rich Felker, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <70bb54b2-8ed3-b5ee-c02d-6ef66c4f27eb@physik.fu-berlin.de>
* John Paul Adrian Glaubitz:
> As for the enterprise support, this seems to be correct. I don't know
> of any enterprise distribution with x32 support either.
Me neither. I would expect a pure userspace port, with limitations in
what ioctls you can use, and perhaps support from GCC to share more APIs
with the kernel.
(I do not know of any plans to work on this.)
> I'm not terribly opposed to this change. I'm usually for keeping support
> for things that people are using, but the maintenance is a huge burden
> to upstream projects, I'm fine with letting it go.
Thank you for sharing your perspective.
> If x32 is eventually to be removed, we should also take care of removing
> x32 support from userland code. From the top of my head, this would at least
> concern:
>
> * OpenJDK
Note that OpenJDK (well, Hotspot) has its own 32-bit pointer support for
the Java heap (compressed oops), so only the native code parts (and JNI)
benefit from x32 anyway.
Thanks,
Florian
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: John Paul Adrian Glaubitz @ 2018-12-11 10:29 UTC (permalink / raw)
To: Andy Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, Mike Frysinger,
H. J. Lu, Rich Felker, x32, Arnd Bergmann, Will Deacon,
Catalin Marinas, Linus Torvalds
In-Reply-To: <CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com>
Hello!
I'm Debian's principal maintainer of the x32 port.
On 12/11/18 2:23 AM, Andy Lutomirski wrote:
> 1. It's not entirely clear that it has users. As far as I know, it's
> supported on Gentoo and Debian, and the Debian popcon graph for x32
> has been falling off dramatically. I don't think that any enterprise
> distro has ever supported x32.
There are definitely some users of this port. I don't know the actual
number, but I hear from users from time to time. As for the popcon
curve, I wouldn't say it has dropped dramatically as it was never
high in the first place.
> https://popcon.debian.org/stat/sub-x32.png
It seems that the highest number of recorded users was 18 and it's
now down to 7. Keep in mind though that popcon participation is
opt-in, so the actual number of users should be higher. According
to popcon, there are also only 172331 Debian installations on x86_64:
> https://popcon.debian.org/
As for the enterprise support, this seems to be correct. I don't know
of any enterprise distribution with x32 support either.
> 2. The way that system calls work is very strange. Most syscalls on
> x32 enter through their *native* (i.e. not COMPAT_SYSCALL_DEFINE)
> entry point, and this is intentional. For example, adjtimex() uses
> the native entry, not the compat entry, because x32's struct timex
> matches the x86_64 layout. But a handful of syscalls have separate
> entry points -- these are the syscalls starting at 512. These enter
> through the COMPAT_SYSCALL_DEFINE entry points.
>
> The x32 syscalls that are *not* in the 512 range violate all semblance
> of kernel syscall convention. In the syscall handlers,
> in_compat_syscall() returns true, but the COMPAT_SYSCALL_DEFINE entry
> is not invoked. This is nutty and risks breaking things when people
> refactor their syscall implementations. And no one tests these
> things. Similarly, if someone calls any of the syscalls below 512 but
> sets bit 31 in RAX, then the native entry will be called with
> in_compat_set().
>
> Conversely, if you call a syscall in the 512 range with bit 31
> *clear*, then the compat entry is set with in_compat_syscall()
> *clear*. This is also nutty.
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.
Additionally, x32 support in many applications is either rudimentary
or broken. For example, while LLVM has support for x32, the backend
isn't really stable on this target meaning that compilers like clang
or Rust are partially broken or crash. I'm not sure whether anyone
is interested in fixing this.
It's also that the performance benefits of x32 are often eaten up by
the fact that none of the scripted languages that I know of provide
a JIT that supports x32. Thus, things like Javascript are either
unsupported or slow on x32.
> I propose that we make CONFIG_X86_X32 depend on BROKEN for a release
> or two and then remove all the code if no one complains. If anyone
> wants to re-add it, IMO they're welcome to do so, but they need to do
> it in a way that is maintainable.
I'm not terribly opposed to this change. I'm usually for keeping support
for things that people are using, but the maintenance is a huge burden
to upstream projects, I'm fine with letting it go.
There are other architectures in the kernel like Alpha, HPPA, M68K, PowerPC,
SH and SPARC that I care much more about than x32.
If x32 is eventually to be removed, we should also take care of removing
x32 support from userland code. From the top of my head, this would at least
concern:
* OpenJDK
* LLVM
* gcc
* glibc
* Rust
* binutils
I can take care of these once I know about the decision regarding the kernel.
Usually, it's a matter of grepping the commit history for "x32" and revert
the corresponding commits.
Thanks,
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Arnd Bergmann @ 2018-12-11 9:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: H.J. Lu, the arch/x86 maintainers, Linux Kernel Mailing List,
Linux API, H. Peter Anvin, Peter Zijlstra, Borislav Petkov,
Florian Weimer, vapier, Rich Felker, x32, Will Deacon,
Catalin Marinas, Linus Torvalds
In-Reply-To: <CALCETrU5xw61=me1YcTO3VPzjNEZahMPpi-20-oiSGs+CHcj4g@mail.gmail.com>
On Tue, Dec 11, 2018 at 6:35 AM Andy Lutomirski <luto@kernel.org> wrote:
> On Mon, Dec 10, 2018 at 7:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org> wrote:
> Right. My question wasn't whether x32 had developers -- it was
> whether it had users. If the only users are a small handful of people
> who keep the toolchain and working and some people who benchmark it,
> then I think the case for keeping it in upstream Linux is a bit weak.
+1
> > > Conversely, if you call a syscall in the 512 range with bit 31
> > > *clear*, then the compat entry is set with in_compat_syscall()
> > > *clear*. This is also nutty.
> >
> > This is to share syscalls between LP64 and ILP32 (x32) in x86-64 kernel.
> >
>
> I tried to understand what's going on. As far as I can tell, most of
> the magic is the fact that __kernel_long_t and __kernel_ulong_t are
> 64-bit as seen by x32 user code. This means that a decent number of
> uapi structures are the same on x32 and x86_64. Syscalls that only
> use structures like this should route to the x86_64 entry points. But
> the implementation is still highly dubious -- in_compat_syscall() will
> be *true* in such system calls,
I think the fundamental issue was that the intention had always been
to use only the 64-bit entry points for system calls, but the most
complex one we have -- ioctl() -- has to use the compat entry point
because device drivers define their own data structures using 'long'
and pointer members and they need translation, as well as
matching in_compat_syscall() checks. This in turn breaks down
again whenever a driver defines an ioctl command that takes
a __kernel_long_t or a derived type like timespec as its argument.
> which means that, if someone changes:
>
...
> where one argument has x32 and x86_64 matching but the other has x32
> and x86_32 matching.
>
> This whole thing seems extremely fragile.
It definitely is. We have lots of workarounds specifically for
x32 in device drivers, but in the time_t conversion for y2038
I still found ones that had not been caught earlier, and for each
y2038 conversion that someone did to a driver or syscall, we have
to make sure that it doesn't break x32 in the process.
Arnd
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Florian Weimer @ 2018-12-11 8:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Lutomirski, the arch/x86 maintainers,
Linux List Kernel Mailing, linux-api, Peter Anvin, Peter Zijlstra,
bp, vapier, hjl.tools, dalias, x32, Arnd Bergmann, Will Deacon,
Catalin Marinas
In-Reply-To: <CAHk-=wi_Kp=3XmGDdzmadzFSPFvuL+aAJ6ZPAR=o4z=KwYT2vw@mail.gmail.com>
* Linus Torvalds:
> 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.
Are you sure GCC was used for that?
ICC has a 32-bit pointer mode which does not change the (64-bit) kernel
interface.
Thanks,
Florian
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Christian Brauner @ 2018-12-11 5:46 UTC (permalink / raw)
To: Andy Lutomirski
Cc: X86 ML, LKML, Linux API, H. Peter Anvin, Peter Zijlstra,
Borislav Petkov, Florian Weimer, Mike Frysinger, H. J. Lu,
Rich Felker, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com>
On Mon, Dec 10, 2018 at 05:23:39PM -0800, Andy Lutomirski wrote:
> Hi all-
>
> I'm seriously considering sending a patch to remove x32 support from
> upstream Linux. Here are some problems with it:
>
> 1. It's not entirely clear that it has users. As far as I know, it's
> supported on Gentoo and Debian, and the Debian popcon graph for x32
> has been falling off dramatically. I don't think that any enterprise
> distro has ever supported x32.
>
> 2. The way that system calls work is very strange. Most syscalls on
> x32 enter through their *native* (i.e. not COMPAT_SYSCALL_DEFINE)
> entry point, and this is intentional. For example, adjtimex() uses
> the native entry, not the compat entry, because x32's struct timex
> matches the x86_64 layout. But a handful of syscalls have separate
> entry points -- these are the syscalls starting at 512. These enter
> through the COMPAT_SYSCALL_DEFINE entry points.
>
> The x32 syscalls that are *not* in the 512 range violate all semblance
> of kernel syscall convention. In the syscall handlers,
> in_compat_syscall() returns true, but the COMPAT_SYSCALL_DEFINE entry
> is not invoked. This is nutty and risks breaking things when people
> refactor their syscall implementations. And no one tests these
> things. Similarly, if someone calls any of the syscalls below 512 but
> sets bit 31 in RAX, then the native entry will be called with
> in_compat_set().
>
> Conversely, if you call a syscall in the 512 range with bit 31
> *clear*, then the compat entry is set with in_compat_syscall()
> *clear*. This is also nutty.
>
> Finally, the kernel has a weird distinction between CONFIG_X86_X32_ABI
> and and CONFIG_X86_X32, which I suspect results in incorrect builds if
> the host doesn't have an x32 toolchain installed.
>
> I propose that we make CONFIG_X86_X32 depend on BROKEN for a release
> or two and then remove all the code if no one complains. If anyone
Based on the discussion we had at the beginning of the pidfd_send_signal
syscall patchset I think this is a good idea. For once, the complex
compat handling can make adding new syscalls that need to rely on compat
types because of precedent established by older syscalls icky.
> wants to re-add it, IMO they're welcome to do so, but they need to do
> it in a way that is maintainable.
>
> --Andy
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: Andy Lutomirski @ 2018-12-11 5:35 UTC (permalink / raw)
To: H. J. Lu
Cc: Andrew Lutomirski, X86 ML, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, Mike Frysinger,
Rich Felker, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <CAMe9rOp0m5sn59AsD2F2i2Ad2tb7ufPU1mMg-vUOfh+TnGMeaw@mail.gmail.com>
On Mon, Dec 10, 2018 at 7:15 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > Hi all-
> >
> > I'm seriously considering sending a patch to remove x32 support from
> > upstream Linux. Here are some problems with it:
> >
> > 1. It's not entirely clear that it has users. As far as I know, it's
> > supported on Gentoo and Debian, and the Debian popcon graph for x32
> > has been falling off dramatically. I don't think that any enterprise
> > distro has ever supported x32.
>
> I have been posting x32 GCC results for years:
>
> https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg01358.html
Right. My question wasn't whether x32 had developers -- it was
whether it had users. If the only users are a small handful of people
who keep the toolchain and working and some people who benchmark it,
then I think the case for keeping it in upstream Linux is a bit weak.
>
> > 2. The way that system calls work is very strange. Most syscalls on
> > x32 enter through their *native* (i.e. not COMPAT_SYSCALL_DEFINE)
> > entry point, and this is intentional. For example, adjtimex() uses
> > the native entry, not the compat entry, because x32's struct timex
> > matches the x86_64 layout. But a handful of syscalls have separate
>
> This becomes less an issue with 64-bit time_t.
>
> > entry points -- these are the syscalls starting at 512. These enter
> > throuh the COMPAT_SYSCALL_DEFINE entry points.
> >
> > The x32 syscalls that are *not* in the 512 range violate all semblance
> > of kernel syscall convention. In the syscall handlers,
> > in_compat_syscall() returns true, but the COMPAT_SYSCALL_DEFINE entry
> > is not invoked. This is nutty and risks breaking things when people
> > refactor their syscall implementations. And no one tests these
> > things. Similarly, if someone calls any of the syscalls below 512 but
> > sets bit 31 in RAX, then the native entry will be called with
> > in_compat_set().
> >
> > Conversely, if you call a syscall in the 512 range with bit 31
> > *clear*, then the compat entry is set with in_compat_syscall()
> > *clear*. This is also nutty.
>
> This is to share syscalls between LP64 and ILP32 (x32) in x86-64 kernel.
>
I tried to understand what's going on. As far as I can tell, most of
the magic is the fact that __kernel_long_t and __kernel_ulong_t are
64-bit as seen by x32 user code. This means that a decent number of
uapi structures are the same on x32 and x86_64. Syscalls that only
use structures like this should route to the x86_64 entry points. But
the implementation is still highly dubious -- in_compat_syscall() will
be *true* in such system calls, which means that, if someone changes:
SYSCALL_DEFINE1(some_func, struct some_struct __user *, ptr)
{
/* x32 goes here, but it's entirely non-obvious unless you read the
x86 syscall table */
native impl;
}
COMPAT_SYSCALL_DEFINE1(some_func, struct compat_some_struct __user *, ptr)
{
compat impl;
}
to the Obviously Equivalent (tm):
SYSCALL_DEFINE1(some_func, struct some_struct __user *, ptr)
{
struct some_struct kernel_val;
if (in_compat_syscall()) {
get_compat_some_struct(&kernel_val, ptr);
} else {
copy_from_user(&kernel_val, ptr, sizeof(struct some_struct));
}
do the work;
}
then x32 breaks.
And I don't even know how x32 is supposed to support some hypothetical
syscall like this:
long sys_nasty(struct adjtimex *a, struct iovec *b);
where one argument has x32 and x86_64 matching but the other has x32
and x86_32 matching.
This whole thing seems extremely fragile.
^ permalink raw reply
* Re: Can we drop upstream Linux x32 support?
From: H.J. Lu @ 2018-12-11 3:14 UTC (permalink / raw)
To: Andy Lutomirski
Cc: the arch/x86 maintainers, LKML, Linux API, H. Peter Anvin,
Peter Zijlstra, Borislav Petkov, Florian Weimer, Mike Frysinger,
Rich Felker, x32, Arnd Bergmann, Will Deacon, Catalin Marinas,
Linus Torvalds
In-Reply-To: <CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com>
On Mon, Dec 10, 2018 at 5:23 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> Hi all-
>
> I'm seriously considering sending a patch to remove x32 support from
> upstream Linux. Here are some problems with it:
>
> 1. It's not entirely clear that it has users. As far as I know, it's
> supported on Gentoo and Debian, and the Debian popcon graph for x32
> has been falling off dramatically. I don't think that any enterprise
> distro has ever supported x32.
I have been posting x32 GCC results for years:
https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg01358.html
> 2. The way that system calls work is very strange. Most syscalls on
> x32 enter through their *native* (i.e. not COMPAT_SYSCALL_DEFINE)
> entry point, and this is intentional. For example, adjtimex() uses
> the native entry, not the compat entry, because x32's struct timex
> matches the x86_64 layout. But a handful of syscalls have separate
This becomes less an issue with 64-bit time_t.
> entry points -- these are the syscalls starting at 512. These enter
> throuh the COMPAT_SYSCALL_DEFINE entry points.
>
> The x32 syscalls that are *not* in the 512 range violate all semblance
> of kernel syscall convention. In the syscall handlers,
> in_compat_syscall() returns true, but the COMPAT_SYSCALL_DEFINE entry
> is not invoked. This is nutty and risks breaking things when people
> refactor their syscall implementations. And no one tests these
> things. Similarly, if someone calls any of the syscalls below 512 but
> sets bit 31 in RAX, then the native entry will be called with
> in_compat_set().
>
> Conversely, if you call a syscall in the 512 range with bit 31
> *clear*, then the compat entry is set with in_compat_syscall()
> *clear*. This is also nutty.
This is to share syscalls between LP64 and ILP32 (x32) in x86-64 kernel.
> Finally, the kernel has a weird distinction between CONFIG_X86_X32_ABI
> and and CONFIG_X86_X32, which I suspect results in incorrect builds if
> the host doesn't have an x32 toolchain installed.
X86-64 binutils and GCC can be used to build x86-64 kernel with x32 support.
> I propose that we make CONFIG_X86_X32 depend on BROKEN for a release
> or two and then remove all the code if no one complains. If anyone
> wants to re-add it, IMO they're welcome to do so, but they need to do
> it in a way that is maintainable.
--
H.J.
^ 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