* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Rich Felker @ 2018-11-26 17:07 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Florian Weimer, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <1284855405.12857.1543249851299.JavaMail.zimbra@efficios.com>
On Mon, Nov 26, 2018 at 11:30:51AM -0500, Mathieu Desnoyers wrote:
> ----- On Nov 26, 2018, at 10:51 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
>
> > ----- On Nov 26, 2018, at 3:28 AM, Florian Weimer fweimer@redhat.com wrote:
> >
> >> * Mathieu Desnoyers:
> >>
> >>> Using a "weak" symbol in early adopter libraries is important, so they
> >>> can be loaded together into the same process without causing loader
> >>> errors due to many definitions of the same strong symbol.
> >>
> >> This is not how ELF dynamic linking works. If the symbol name is the
> >> same, one definition interposes the others.
> >>
> >> You need to ensure that the symbol has the same size everywhere, though.
> >> There are some tricky interactions with symbol versions, too. (The
> >> interposing libraries must not use symbol versioning.)
> >
> > I was under the impression that loading the same strong symbol into an
> > application multiple times would cause some kind of warning if non-weak. I did
> > some testing to figure out which case I remembered would cause this.
> >
> > When compiling with "-fno-common", dynamic and static linking work fine, but
> > trying to add multiple instances of a given symbol into a single object fails
> > with:
> >
> > /tmp/ccSakXZV.o:(.bss+0x0): multiple definition of `a'
> > /tmp/ccQBJBOo.o:(.bss+0x0): first defined here
> >
> > Even if the symbol has the same size.
> >
> > So considering that we don't care about compiling into a single object here,
> > and only care about static and dynamic linking of libraries, indeed the "weak"
> > symbol is not useful.
> >
> > So let's make __rseq_abi and __rseq_refcount strong symbols then ?
>
> Actually, looking into ld(1) --warn-common, it looks like "weak" would be cleaner
> after all, especially for __rseq_abi which we needs to be initialized to a specific
> value, which is therefore not a common symbol.
>
> " --warn-common
> Warn when a common symbol is combined with another common symbol or with a symbol definition. Unix
> linkers allow this somewhat sloppy practice, but linkers on some other operating systems do not.
> This option allows you to find potential problems from combining global symbols. Unfortunately,
> some C libraries use this practice, so you may get some warnings about symbols in the libraries as
> well as in your programs."
>
> Thoughts ?
AFAIK this has nothing to do with dynamic linking.
Rich
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Rich Felker @ 2018-11-26 17:10 UTC (permalink / raw)
To: Florian Weimer
Cc: Mathieu Desnoyers, carlos, Joseph Myers, Szabolcs Nagy,
libc-alpha, Thomas Gleixner, Ben Maurer, Peter Zijlstra,
Paul E. McKenney, Boqun Feng, Will Deacon, Dave Watson,
Paul Turner, linux-kernel, linux-api
In-Reply-To: <874lc3omh5.fsf@oldenburg.str.redhat.com>
On Mon, Nov 26, 2018 at 05:03:02PM +0100, Florian Weimer wrote:
> * Mathieu Desnoyers:
>
> > So let's make __rseq_abi and __rseq_refcount strong symbols then ?
>
> Yes, please. (But I'm still not sure we need the reference counter.)
The reference counter is needed for out-of-libc implementations
interacting and using the dtor hack. An in-libc implementation doesn't
need to inspect/honor the reference counter, but it does seem to need
to indicate that it has a reference, if you want it to be compatible
with out-of-libc implementations, so that the out-of-libc one will not
unregister the rseq before libc is done with it.
Alternatively another protocol could be chosen for this purpose, but
if has to be something stable and agreed upon, since things would
break badly if libc and the library providing rseq disagreed.
Rich
^ permalink raw reply
* Re: extending wait4(2) or waitid(2) linux syscall
From: Arnd Bergmann @ 2018-11-26 17:15 UTC (permalink / raw)
To: Florian Weimer
Cc: Dmitry V. Levin, Albert ARIBAUD, H. Peter Anvin, GNU C Library,
Linux API
In-Reply-To: <87d0qrooj3.fsf@oldenburg.str.redhat.com>
On Mon, Nov 26, 2018 at 4:18 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Arnd Bergmann:
>
> > On Thu, Nov 15, 2018 at 7:30 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
> >> On Thu, Nov 15, 2018 at 06:39:03AM -0800, Arnd Bergmann wrote:
> >> > On Thu, Nov 15, 2018 at 6:05 AM Dmitry V. Levin wrote:
> >> > > On Thu, Apr 20, 2017 at 03:20:51PM +0200, Albert ARIBAUD wrote:
> >>
> >> 1. strace needs a race-free invocation of wait4(2) or waitid(2)
> >> with a different signal mask, this cannot be achieved without
> >> an extended version of syscall, similar to pselect6(2) extension
> >> over select(2) and ppoll(2) extension over poll(2).
> >>
> >> Signal mask specification in linux requires two parameters:
> >> "const sigset_t *sigmask" and "size_t sigsetsize".
> >> Creating pwait6(2) as an extension of wait4(2) with two arguments
> >> is straightforward.
> >> Creating pwaitid(2) as an extension of waitid(2) that already has 5
> >> arguments would require an indirection similar to pselect6(2).
> >
> > Getting back to this point: you could also do the same thing with
> > the CLONE_FD approach from Josh Triplett[1] or Casey Dahlin's
> > older waitfd() syscall, correct?
>
> A descriptor-based solution would not be useful to glibc because
> applications assume that glibc does not (persistently) open any file
> descriptors behind t heir back.
Right, makes sense. What about a temporary file descriptor as discussed
in the recent procfd() mail thread then? Would that work?
/* for illustration, needs error handling and more features */
int pwait(pid_t id, siginfo_t *infop)
{
char waitfd_file[MAX_PROCFD_LEN];
struct pollfd pfd[1] = { {.events = POLLIN }};
snprintf(waitfd_file, MAX_PROCFD_LEN, "/proc/%d/wait", pid);
pfd.fd = open(waitfd_file, O_RDONLY);
ppoll(&pfd, 1, NULL, sigmask);
read(fd, infop, sizeof(*infop));
close(fd);
return 0;
}
Arnd
^ permalink raw reply
* Re: extending wait4(2) or waitid(2) linux syscall
From: Daniel Colascione @ 2018-11-26 17:27 UTC (permalink / raw)
To: arnd; +Cc: Florian Weimer, ldv, albert.aribaud, hpa, GNU C Library,
Linux API
In-Reply-To: <CAK8P3a3pVS3_70PQ+SeiSZtqXFcb5HON0QGRypWfu=cfLPgN=w@mail.gmail.com>
On Mon, Nov 26, 2018 at 9:15 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Nov 26, 2018 at 4:18 PM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * Arnd Bergmann:
> >
> > > On Thu, Nov 15, 2018 at 7:30 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
> > >> On Thu, Nov 15, 2018 at 06:39:03AM -0800, Arnd Bergmann wrote:
> > >> > On Thu, Nov 15, 2018 at 6:05 AM Dmitry V. Levin wrote:
> > >> > > On Thu, Apr 20, 2017 at 03:20:51PM +0200, Albert ARIBAUD wrote:
> > >>
> > >> 1. strace needs a race-free invocation of wait4(2) or waitid(2)
> > >> with a different signal mask, this cannot be achieved without
> > >> an extended version of syscall, similar to pselect6(2) extension
> > >> over select(2) and ppoll(2) extension over poll(2).
> > >>
> > >> Signal mask specification in linux requires two parameters:
> > >> "const sigset_t *sigmask" and "size_t sigsetsize".
> > >> Creating pwait6(2) as an extension of wait4(2) with two arguments
> > >> is straightforward.
> > >> Creating pwaitid(2) as an extension of waitid(2) that already has 5
> > >> arguments would require an indirection similar to pselect6(2).
> > >
> > > Getting back to this point: you could also do the same thing with
> > > the CLONE_FD approach from Josh Triplett[1] or Casey Dahlin's
> > > older waitfd() syscall, correct?
> >
> > A descriptor-based solution would not be useful to glibc because
> > applications assume that glibc does not (persistently) open any file
> > descriptors behind t heir back.
*My* long-term goal is to expose all new process-related functionality
as operations on file descriptors. I really don't want to further
complicate the wait*(2) series of functions.
Whether the file descriptor is opened "persistently" or not is an
implementation detail. A multi-threaded program can observe the file
descriptor being open regardless. The right approach here is to
abandon this strange requirement that glibc not use file descriptors
for its own purposes. File descriptors are just kernel resource
handles, and it's not reasonable for applications to expect that the
libraries they use do not manage resources for their own purposes.
This self-imposed never-open-file-descriptors behavior on glibc's part
has created API design problems in the past (e.g., with the randomness
functions) and should be abandoned.
^ permalink raw reply
* Re: [RFC PATCH v6 00/26] Control-flow Enforcement: Shadow Stack
From: Yu-cheng Yu @ 2018-11-26 17:38 UTC (permalink / raw)
To: Andy Lutomirski, Florian Weimer, Carlos O'Donell, Rich Felker
Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
Arnd Bergmann, Balbir Singh, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, H. J. Lu, Jann Horn, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov
In-Reply-To: <CALCETrWLtpfkecfUAXJ64Z5xDeHPJxTQSci+T4RCem7vCqorTw@mail.gmail.com>
On Thu, 2018-11-22 at 08:53 -0800, Andy Lutomirski wrote:
> [cc some more libc folks]
>
> I have a general question about this patch set:
>
> If I'm writing a user program, and I write a signal handler, there are
> two things I want to make sure I can still do:
>
> 1. I want to be able to unwind directly from the signal handler
> without involving sigreturn() -- that is, I want to make sure that
> siglongjmp() works. How does this work? Is INCSSP involved? How
Yes, siglongjmp() works by doing INCSSP.
> exactly does the user program know how much to increment SSP by? (And
> why on Earth does INCSSP only consider the low 8 bits of its argument?
> That sounds like a mistake. Can Intel still fix that? On the other
GLIBC calculates how many frames to be unwound and breaks into 255 batches when
necessary.
> hand, what happens if you INCSSP off the end of the shadow stack
> entirely? I assume the next access will fault as long as there's an
> appropriate guard page.)
Yes, that is the case.
>
> 2. I want to be able to modify the signal context from a signal
> handler such that, when the signal handler returns, it will return to
> a frame higher up on the call stack than where the signal started and
> to a different RIP value. How can I do this? I guess I can modify
> the shadow stack with WRSS if WR_SHSTK_EN=1, but how do I tell the
> kernel to kindly skip the frames I want to skip when I do sigreturn()?
>
> The reason I'm asking #2 is that I think it's time to resurrect my old
> vDSO syscall cancellation helper series here:
>
> https://lwn.net/Articles/679434/
If tools/testing/selftests/x86/unwind_vdso.c passes, can we say the kernel does
the right thing? Or do you have other tests that I can run?
>
> and it's not at all clear to me that __vdso_abort_pending_syscall()
> can work without kernel assistance when CET is enabled. I want to
> make sure that it can be done, or I want to come up with some other
> way to allow a signal handler to abort a syscall while CET is on. I
> could probably change __vdso_abort_pending_syscall() to instead point
> RIP to __kernel_vsyscall's epilogue so that we con't change the depth
> of the call stack. But I could imagine that other user programs might
> engage in similar shenanigans and want to have some way to unwind a
> signal's return context without actually jumping there a la
> siglongjmp().
>
> Also, what is the intended setting of WR_SHSTK_EN with this patch set applied?
This bit enables WRSS instruction, which writes to kernel SHSTK. This patch set
uses only WRUSS and WR_SHSTK_EN is not be set.
>
> (I suppose we could just say that 32-bit processes should not use CET,
> but that seems a bit sad.)
They work in compat mode. Should anything break, we can fix it.
Yu-cheng
^ permalink raw reply
* Re: [RFC PATCH v6 00/26] Control-flow Enforcement: Shadow Stack
From: Andy Lutomirski @ 2018-11-26 18:29 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Andrew Lutomirski, Florian Weimer, Carlos O'Donell,
Rich Felker, X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
LKML, open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
Arnd Bergmann, Balbir Singh, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, H. J. Lu, Jann Horn,
Jonathan Corbet <corbe>
In-Reply-To: <35b33f293bc392df71710102f38fa6a40d0bb996.camel@intel.com>
On Mon, Nov 26, 2018 at 9:44 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Thu, 2018-11-22 at 08:53 -0800, Andy Lutomirski wrote:
> > [cc some more libc folks]
>
> >
> > 2. I want to be able to modify the signal context from a signal
> > handler such that, when the signal handler returns, it will return to
> > a frame higher up on the call stack than where the signal started and
> > to a different RIP value. How can I do this? I guess I can modify
> > the shadow stack with WRSS if WR_SHSTK_EN=1, but how do I tell the
> > kernel to kindly skip the frames I want to skip when I do sigreturn()?
> >
> > The reason I'm asking #2 is that I think it's time to resurrect my old
> > vDSO syscall cancellation helper series here:
> >
> > https://lwn.net/Articles/679434/
>
> If tools/testing/selftests/x86/unwind_vdso.c passes, can we say the kernel does
> the right thing? Or do you have other tests that I can run?
I haven't written the relevant test yet. Hopefully soon :)
^ permalink raw reply
* Re: [RFC PATCH v4 1/5] glibc: Perform rseq(2) registration at nptl init and thread creation
From: Mathieu Desnoyers @ 2018-11-26 19:22 UTC (permalink / raw)
To: Rich Felker
Cc: Florian Weimer, carlos, Joseph Myers, Szabolcs Nagy, libc-alpha,
Thomas Gleixner, Ben Maurer, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Will Deacon, Dave Watson, Paul Turner, linux-kernel,
linux-api
In-Reply-To: <20181126171045.GQ23599@brightrain.aerifal.cx>
----- On Nov 26, 2018, at 12:10 PM, Rich Felker dalias@libc.org wrote:
> On Mon, Nov 26, 2018 at 05:03:02PM +0100, Florian Weimer wrote:
>> * Mathieu Desnoyers:
>>
>> > So let's make __rseq_abi and __rseq_refcount strong symbols then ?
>>
>> Yes, please. (But I'm still not sure we need the reference counter.)
>
> The reference counter is needed for out-of-libc implementations
> interacting and using the dtor hack. An in-libc implementation doesn't
> need to inspect/honor the reference counter, but it does seem to need
> to indicate that it has a reference, if you want it to be compatible
> with out-of-libc implementations, so that the out-of-libc one will not
> unregister the rseq before libc is done with it.
Let's consider two use-cases here: one (simpler) is use of rseq TLS
from thread context by out-of-libc implementations. The other is use of
rseq TLS from signal handler by out-of-libc implementations.
If we only care about users of rseq from thread context, then libc
could simply set the refcount value to 1 on thread start,
and should not care about the value on thread exit. The libc can
either directly call rseq unregister, or rely on thread calling exit
to implicitly unregister rseq, which depends on its own TLS life-time
guarantees. For instance, if the IE-model TLS is valid up until call
to exit, just calling the exit system call is fine. However, if a libc
has a window at thread exit during which the kernel can preempt the
thread with the IE-model TLS area being already reclaimed, then it
needs to explicitly call rseq unregister before freeing the TLS.
The second use-case is out-of-libc implementations using rseq from
signal handler. This one is trickier. First, pthread_key setspecific
is unfortunately not async-signal-safe. I can't find a good way to
seamlessly integrate rseq into out-of-libc signal handlers while
performing lazy registration without races on thread exit. If we
figure out a way to do this though, we should increment the refcount
at thread start in libc (rather than just set it to 1) in case a
signal handler gets nested immediately over the start of the thread
and registers rseq as well.
It looks like it's not the only issue I have with calling lttng-ust
instrumentation from signal handlers, here is the list I have so
far:
* glibc global-dynamic TLS variables are not async-signal-safe,
and lttng-ust cannot use IE-model TLS because it is meant to be
dlopen'd,
* pthread_setspecific is not async-signal-safe,
There should be ways to eventually solve those issues, but it would
be nice if for now the way rseq is implemented in libc does not add
yet another limitation for signal handlers.
>
> Alternatively another protocol could be chosen for this purpose, but
> if has to be something stable and agreed upon, since things would
> break badly if libc and the library providing rseq disagreed.
Absolutely. We need to agree on that protocol before user-space
applications/libraries start using rseq.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jarkko Sakkinen @ 2018-11-26 22:06 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jethro Beekman, Andy Lutomirski, Dave Hansen, Florian Weimer,
Linux API, Jann Horn, Linus Torvalds, X86 ML, linux-arch, LKML,
Peter Zijlstra, Rich Felker, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko, Thomas
In-Reply-To: <20181126143534.GA25719@linux.intel.com>
On Mon, Nov 26, 2018 at 06:35:34AM -0800, Sean Christopherson wrote:
> And how would you determine the #UD is related to SGX? Hardware doesn't
> provide any indication that a #UD (or any other fault) is related to SGX
> or occurred in an enclave. The only fault that is special-cased in a
> non-virtualized environment is #PF signaled by the EPCM, which gets the
> PF_SGX bit set in the error code.
Could you not detect #UD from address where it happened? Kernel knows
where enclaves are mapped. BTW, how does Intel run-time emulate opcodes
currently?
Anyway, I've fully discarded the whole idea because implementing single
stepping w/o well defined AEP handler is nasty. I think vDSO's are the
only viable path that at least I'm aware off...
/Jarkko
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: William Kucharski @ 2018-11-27 0:33 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-api, Andrew Morton, Alexey Dobriyan, linux-mm, LKML,
Michal Hocko
In-Reply-To: <20181120103515.25280-4-mhocko@kernel.org>
This determines whether the page can theoretically be THP-mapped , but is the intention to also check for proper alignment and/or preexisting PAGESIZE page cache mappings for the address range?
I'm having to deal with both these issues in the text page THP prototype I've been working on for some time now.
Thanks,
William Kucharski
^ permalink raw reply
* Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Elvira Khabirova @ 2018-11-27 2:53 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
rostedt-nx8X9YLhiw1AfugRpC6u6w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-H+wXaHxf7aLQT0dZR+AlfA,
luto-DgEjT+Ai2ygdnm+yROfE0A, ldv-u2l5PoMzF/Vg9hUCZPvPmw,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181126145643.GD1660-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, 26 Nov 2018 15:56:43 +0100
Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 11/25, Elvira Khabirova wrote:
> >
> > + * These values are stored in task->ptrace_message by tracehook_report_syscall_*
> > + * to describe current syscall-stop.
> > + *
> > + * Values for these constants are chosen so that they do not appear
> > + * in task->ptrace_message by other means.
> > + */
> > +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 0x80000000U
> > +#define PTRACE_EVENTMSG_SYSCALL_EXIT 0x90000000U
>
> Stupid question, why not
>
> #define PTRACE_EVENT_SYSCALL_ENTRY 8
> #define PTRACE_EVENT_SYSCALL_EXIT 9
>
> right after other PTRACE_EVENT_* constants?
I thought about adding new events for syscall {entry,exit}.
For tracers, using new events means setting new options and checking
for new values after waitpid(). They will also have to switch from using
PTRACE_SYSCALL to PTRACE_CONT.
Right now (with this version of the patch) tracers can use
PTRACE_GETEVENTMSG without doing any additional configuration.
More importantly, adding these events would require much more complex
modifications of kernel code than this patch does.
The only benefit I see from adding these events instead of letting
syscall-stops put a value in ptrace_message is an ability to subscribe
to syscall entries, but not to exits, and vice-versa, and I don't think
it is worth it.
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
From: Elvira Khabirova @ 2018-11-27 3:07 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, esyr-H+wXaHxf7aLQT0dZR+AlfA,
rostedt-nx8X9YLhiw1AfugRpC6u6w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-H+wXaHxf7aLQT0dZR+AlfA,
luto-DgEjT+Ai2ygdnm+yROfE0A, ldv-u2l5PoMzF/Vg9hUCZPvPmw,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181126143524.GB1660-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, 26 Nov 2018 15:35:24 +0100
Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 11/25, Elvira Khabirova wrote:
> >
> > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> > The information returned is the same as for syscall-enter-stops.
>
> Oh, this is not nice ;) there must be a better option, I hope... Plus
>
>
> Can't ptrace_get_syscall() check
>
> child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
>
> to detect the PTRACE_EVENT_SECCOMP case?
Nope; looks like exit_code is zeroed after wait().
> Oleg.
>
> > Signed-off-by: Elvira Khabirova <lineprinter-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
> > Signed-off-by: Dmitry V. Levin <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>
> > ---
> > include/linux/ptrace.h | 1 +
> > include/linux/sched.h | 1 +
> > include/linux/tracehook.h | 1 +
> > kernel/ptrace.c | 7 +++++--
> > 4 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> > index 6c2ffed907f5..a993d0fde865 100644
> > --- a/include/linux/ptrace.h
> > +++ b/include/linux/ptrace.h
> > @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long message)
> > {
> > if (unlikely(ptrace_event_enabled(current, event))) {
> > current->ptrace_message = message;
> > + current->ptrace_event = event;
> > ptrace_notify((event << 8) | SIGTRAP);
> > } else if (event == PTRACE_EVENT_EXEC) {
> > /* legacy EXEC report via SIGTRAP */
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index a51c13c2b1a0..86215fb654d6 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -964,6 +964,7 @@ struct task_struct {
> >
> > /* Ptrace state: */
> > unsigned long ptrace_message;
> > + int ptrace_event;
> > kernel_siginfo_t *last_siginfo;
> >
> > struct task_io_accounting ioac;
> > diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> > index 633a83fe7051..5d2e5aa07a5c 100644
> > --- a/include/linux/tracehook.h
> > +++ b/include/linux/tracehook.h
> > @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
> > return 0;
> >
> > current->ptrace_message = message;
> > + current->ptrace_event = 0;
> > ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
> >
> > /*
> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 92c47cd5ad84..74a37e74c7f1 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
> > unsigned long actual_size;
> > unsigned long write_size;
> >
> > - if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
> > + if ((child->ptrace_event == 0 &&
> > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
> > + child->ptrace_event == PTRACE_EVENT_SECCOMP) {
> > int i;
> >
> > info.op = PTRACE_SYSCALL_INFO_ENTRY;
> > @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
> > for (i = 0; i < ARRAY_SIZE(args); i++)
> > info.entry.args[i] = args[i];
> > actual_size = offsetofend(struct ptrace_syscall_info, entry);
> > - } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > + } else if (child->ptrace_event == 0 &&
> > + child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > info.op = PTRACE_SYSCALL_INFO_EXIT;
> > info.arch = syscall_get_arch(child);
> > info.exit.rval = syscall_get_error(child, regs);
> > --
> > 2.19.1
> >
>
--
Strace-devel mailing list
Strace-devel-3+4lAyCyj6AWlMsSdNXQLw@public.gmane.org
https://lists.strace.io/mailman/listinfo/strace-devel
^ permalink raw reply
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
From: Dmitry V. Levin @ 2018-11-27 3:21 UTC (permalink / raw)
To: Elvira Khabirova, Oleg Nesterov
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven Rostedt,
Eugene Syromyatnikov, Ingo Molnar, Andy Lutomirski,
strace-devel-3+4lAyCyj6AWlMsSdNXQLw
In-Reply-To: <20181127040732.1c9f7965@akathisia>
[-- Attachment #1.1: Type: text/plain, Size: 839 bytes --]
On Tue, Nov 27, 2018 at 04:07:32AM +0100, Elvira Khabirova wrote:
> On Mon, 26 Nov 2018 15:35:24 +0100, Oleg Nesterov wrote:
> > On 11/25, Elvira Khabirova wrote:
> > >
> > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> > > The information returned is the same as for syscall-enter-stops.
> >
> > Oh, this is not nice ;) there must be a better option, I hope... Plus
> >
> > Can't ptrace_get_syscall() check
> >
> > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
> >
> > to detect the PTRACE_EVENT_SECCOMP case?
>
> Nope; looks like exit_code is zeroed after wait().
It's explicitly reset to zero in wait_task_stopped() unless WNOWAIT wait
option is set. When strace requests PTRACE_GET_SYSCALL_INFO after
wait4(), child->exit_code is already set to zero.
--
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 RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
From: Dmitry V. Levin @ 2018-11-27 3:48 UTC (permalink / raw)
To: Elvira Khabirova, Oleg Nesterov
Cc: Steven Rostedt, Ingo Molnar, Andy Lutomirski,
Eugene Syromyatnikov, linux-kernel, strace-devel, linux-api
In-Reply-To: <20181127035357.0b381323@akathisia>
[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]
On Tue, Nov 27, 2018 at 03:53:57AM +0100, Elvira Khabirova wrote:
> On Mon, 26 Nov 2018 15:56:43 +0100, Oleg Nesterov wrote:
> > On 11/25, Elvira Khabirova wrote:
> > >
> > > + * These values are stored in task->ptrace_message by tracehook_report_syscall_*
> > > + * to describe current syscall-stop.
> > > + *
> > > + * Values for these constants are chosen so that they do not appear
> > > + * in task->ptrace_message by other means.
> > > + */
> > > +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 0x80000000U
> > > +#define PTRACE_EVENTMSG_SYSCALL_EXIT 0x90000000U
> >
> > Stupid question, why not
> >
> > #define PTRACE_EVENT_SYSCALL_ENTRY 8
> > #define PTRACE_EVENT_SYSCALL_EXIT 9
> >
> > right after other PTRACE_EVENT_* constants?
>
> I thought about adding new events for syscall {entry,exit}.
> For tracers, using new events means setting new options and checking
> for new values after waitpid(). They will also have to switch from using
> PTRACE_SYSCALL to PTRACE_CONT.
> Right now (with this version of the patch) tracers can use
> PTRACE_GETEVENTMSG without doing any additional configuration.
> More importantly, adding these events would require much more complex
> modifications of kernel code than this patch does.
To be honest, we don't see any way of introducing PTRACE_EVENT_* support
in PTRACE_GET_SYSCALL_INFO without adding new fields to struct task_struct.
> The only benefit I see from adding these events instead of letting
> syscall-stops put a value in ptrace_message is an ability to subscribe
> to syscall entries, but not to exits, and vice-versa, and I don't think
> it is worth it.
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Ram Pai @ 2018-11-27 10:23 UTC (permalink / raw)
To: Florian Weimer; +Cc: Dave Hansen, linux-mm, linuxppc-dev, linux-api
In-Reply-To: <87efbqqze4.fsf@oldenburg.str.redhat.com>
On Mon, Nov 12, 2018 at 01:00:19PM +0100, Florian Weimer wrote:
> * Ram Pai:
>
> > On Thu, Nov 08, 2018 at 09:23:35PM +0100, Florian Weimer wrote:
> >> * Ram Pai:
> >>
> >> > Florian,
> >> >
> >> > I can. But I am struggling to understand the requirement. Why is
> >> > this needed? Are we proposing a enhancement to the sys_pkey_alloc(),
> >> > to be able to allocate keys that are initialied to disable-read
> >> > only?
> >>
> >> Yes, I think that would be a natural consequence.
> >>
> >> However, my immediate need comes from the fact that the AMR register can
> >> contain a flag combination that is not possible to represent with the
> >> existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code
> >> could write to AMR directly, so I cannot rule out that certain flag
> >> combinations exist there.
> >>
> >> So I came up with this:
> >>
> >> int
> >> pkey_get (int key)
> >> {
> >> if (key < 0 || key > PKEY_MAX)
> >> {
> >> __set_errno (EINVAL);
> >> return -1;
> >> }
> >> unsigned int index = pkey_index (key);
> >> unsigned long int amr = pkey_read ();
> >> unsigned int bits = (amr >> index) & 3;
> >>
> >> /* Translate from AMR values. PKEY_AMR_READ standing alone is not
> >> currently representable. */
> >> if (bits & PKEY_AMR_READ)
> >
> > this should be
> > if (bits & (PKEY_AMR_READ|PKEY_AMR_WRITE))
>
> This would return zero for PKEY_AMR_READ alone.
>
> >> return PKEY_DISABLE_ACCESS;
> >
> >
> >> else if (bits == PKEY_AMR_WRITE)
> >> return PKEY_DISABLE_WRITE;
> >> return 0;
> >> }
>
> It's hard to tell whether PKEY_DISABLE_ACCESS is better in this case.
> Which is why I want PKEY_DISABLE_READ.
>
> >> And this is not ideal. I would prefer something like this instead:
> >>
> >> switch (bits)
> >> {
> >> case PKEY_AMR_READ | PKEY_AMR_WRITE:
> >> return PKEY_DISABLE_ACCESS;
> >> case PKEY_AMR_READ:
> >> return PKEY_DISABLE_READ;
> >> case PKEY_AMR_WRITE:
> >> return PKEY_DISABLE_WRITE;
> >> case 0:
> >> return 0;
> >> }
> >
> > yes.
> > and on x86 it will be something like:
> > switch (bits)
> > {
> > case PKEY_PKRU_ACCESS :
> > return PKEY_DISABLE_ACCESS;
> > case PKEY_AMR_WRITE:
> > return PKEY_DISABLE_WRITE;
> > case 0:
> > return 0;
> > }
>
> x86 returns the PKRU bits directly, including the nonsensical case
> (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE).
>
> > But for this to work, why do you need to enhance the sys_pkey_alloc()
> > interface? Not that I am against it. Trying to understand if the
> > enhancement is really needed.
>
> sys_pkey_alloc performs an implicit pkey_set for the newly allocated key
> (that is, it updates the PKRU/AMR register). It makes sense to match
> the behavior of the userspace implementation.
Here is a untested patch. Does this meet your needs?
It defines the new flags. Each architecture will than define the set of flags
it supports through PKEY_ACCESS_MASK.
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 92a9962..724ef43 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -21,11 +21,6 @@
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
VM_PKEY_BIT3 | VM_PKEY_BIT4)
-/* Override any generic PKEY permission defines */
-#define PKEY_DISABLE_EXECUTE 0x4
-#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \
- PKEY_DISABLE_WRITE | \
- PKEY_DISABLE_EXECUTE)
static inline u64 pkey_to_vmflag_bits(u16 pkey)
{
diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
index 65065ce..76237b3 100644
--- a/arch/powerpc/include/uapi/asm/mman.h
+++ b/arch/powerpc/include/uapi/asm/mman.h
@@ -31,9 +31,9 @@
#define MAP_HUGETLB 0x40000 /* create a huge page mapping */
/* Override any generic PKEY permission defines */
-#define PKEY_DISABLE_EXECUTE 0x4
#undef PKEY_ACCESS_MASK
#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
PKEY_DISABLE_WRITE |\
+ PKEY_DISABLE_READ |\
PKEY_DISABLE_EXECUTE)
#endif /* _UAPI_ASM_POWERPC_MMAN_H */
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 4860acd..c8b2540 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -62,14 +62,6 @@ int pkey_initialize(void)
int os_reserved, i;
/*
- * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
- * generic defines for PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE.
- * Ensure that the bits a distinct.
- */
- BUILD_BUG_ON(PKEY_DISABLE_EXECUTE &
- (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
-
- /*
* pkey_to_vmflag_bits() assumes that the pkey bits are contiguous
* in the vmaflag. Make sure that is really the case.
*/
@@ -259,6 +251,8 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
new_amr_bits |= AMR_RD_BIT | AMR_WR_BIT;
else if (init_val & PKEY_DISABLE_WRITE)
new_amr_bits |= AMR_WR_BIT;
+ else if (init_val & PKEY_DISABLE_READ)
+ new_amr_bits |= AMR_RD_BIT;
init_amr(pkey, new_amr_bits);
return 0;
diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h
index d4a8d04..e9b121b 100644
--- a/arch/x86/include/uapi/asm/mman.h
+++ b/arch/x86/include/uapi/asm/mman.h
@@ -24,6 +24,11 @@
((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
((key) & 0x8 ? VM_PKEY_BIT3 : 0))
+
+/* Override any generic PKEY permission defines */
+#undef PKEY_ACCESS_MASK
+#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
+ PKEY_DISABLE_WRITE)
#endif
#include <asm-generic/mman.h>
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index e7ee328..61168e4 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -71,7 +71,8 @@
#define PKEY_DISABLE_ACCESS 0x1
#define PKEY_DISABLE_WRITE 0x2
-#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
- PKEY_DISABLE_WRITE)
-
+#define PKEY_DISABLE_EXECUTE 0x4
+#define PKEY_DISABLE_READ 0x8
+#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own
+ mask bits */
#endif /* __ASM_GENERIC_MMAN_COMMON_H */
^ permalink raw reply related
* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Florian Weimer @ 2018-11-27 11:57 UTC (permalink / raw)
To: Ram Pai; +Cc: Dave Hansen, linux-mm, linuxppc-dev, linux-api
In-Reply-To: <20181127102350.GA5795@ram.oc3035372033.ibm.com>
* Ram Pai:
> diff --git a/arch/x86/include/uapi/asm/mman.h b/arch/x86/include/uapi/asm/mman.h
> index d4a8d04..e9b121b 100644
> --- a/arch/x86/include/uapi/asm/mman.h
> +++ b/arch/x86/include/uapi/asm/mman.h
> @@ -24,6 +24,11 @@
> ((key) & 0x2 ? VM_PKEY_BIT1 : 0) | \
> ((key) & 0x4 ? VM_PKEY_BIT2 : 0) | \
> ((key) & 0x8 ? VM_PKEY_BIT3 : 0))
> +
> +/* Override any generic PKEY permission defines */
> +#undef PKEY_ACCESS_MASK
> +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
> + PKEY_DISABLE_WRITE)
> #endif
I would have expected something that translates PKEY_DISABLE_WRITE |
PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts
PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER.
(My understanding is that PKEY_DISABLE_ACCESS does not disable all
access, but produces execute-only memory.)
> diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
> index e7ee328..61168e4 100644
> --- a/include/uapi/asm-generic/mman-common.h
> +++ b/include/uapi/asm-generic/mman-common.h
> @@ -71,7 +71,8 @@
>
> #define PKEY_DISABLE_ACCESS 0x1
> #define PKEY_DISABLE_WRITE 0x2
> -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\
> - PKEY_DISABLE_WRITE)
> -
> +#define PKEY_DISABLE_EXECUTE 0x4
> +#define PKEY_DISABLE_READ 0x8
> +#define PKEY_ACCESS_MASK 0x0 /* arch can override and define its own
> + mask bits */
> #endif /* __ASM_GENERIC_MMAN_COMMON_H */
I think Dave requested a value for PKEY_DISABLE_READ which is further
away from the existing bits.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO
From: Oleg Nesterov @ 2018-11-27 12:31 UTC (permalink / raw)
To: Elvira Khabirova
Cc: rostedt, mingo, linux-kernel, ldv, esyr, luto, strace-devel,
linux-api
In-Reply-To: <20181127040732.1c9f7965@akathisia>
On 11/27, Elvira Khabirova wrote:
>
> On Mon, 26 Nov 2018 15:35:24 +0100
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > On 11/25, Elvira Khabirova wrote:
> > >
> > > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> > > The information returned is the same as for syscall-enter-stops.
> >
> > Oh, this is not nice ;) there must be a better option, I hope... Plus
> >
> >
> > Can't ptrace_get_syscall() check
> >
> > child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
> >
> > to detect the PTRACE_EVENT_SECCOMP case?
>
> Nope; looks like exit_code is zeroed after wait().
Yes, thanks for correcting me,
but we can use child->last_siginfo->si_code. Just like ptrace_request(PTRACE_LISTEN)
does but you can do this lockless (no need to lock_task_sighand()).
And if we require that the user of ptrace_get_syscall() should also use TRACESYSGOOD
then ptrace_get_syscall() can probably do something like
int entry;
if (!child->last_siginfo)
return -EINVAL;
else if (child->last_siginfo->si_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP)
entry = 1;
else if (child->last_siginfo->si_code == SIGTRAP | 0x80)
entry = child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY;
else
return -EINVAL;
and this way PTRACE_EVENTMSG_SYSCALL_ENTRY/EXIT can't confict with seccomp or
anything else.
No?
Of course, debugger can do PTRACE_SETSIGINFO and confuse itself but probably we
do not care?
Oleg.
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: Michal Hocko @ 2018-11-27 13:17 UTC (permalink / raw)
To: William Kucharski
Cc: linux-api, Andrew Morton, Alexey Dobriyan, linux-mm, LKML
In-Reply-To: <0ACDD94B-75AD-4DD0-B2E3-32C0EDFBAA5E@oracle.com>
On Mon 26-11-18 17:33:32, William Kucharski wrote:
>
>
> This determines whether the page can theoretically be THP-mapped , but
> is the intention to also check for proper alignment and/or preexisting
> PAGESIZE page cache mappings for the address range?
This is only about the process wide flag to disable THP. I do not see
how this can be alighnement related. I suspect you wanted to ask in the
smaps patch?
> I'm having to deal with both these issues in the text page THP
> prototype I've been working on for some time now.
Could you be more specific about the issue and how the alignment comes
into the game? The only thing I can think of is to not report VMAs
smaller than the THP as eligible. Is this what you are looking for?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: William Kucharski @ 2018-11-27 14:50 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-api, Andrew Morton, Alexey Dobriyan, linux-mm, LKML
In-Reply-To: <20181127131707.GW12455@dhcp22.suse.cz>
> On Nov 27, 2018, at 6:17 AM, Michal Hocko <mhocko@kernel.org> wrote:
>
> This is only about the process wide flag to disable THP. I do not see
> how this can be alighnement related. I suspect you wanted to ask in the
> smaps patch?
No, answered below.
>
>> I'm having to deal with both these issues in the text page THP
>> prototype I've been working on for some time now.
>
> Could you be more specific about the issue and how the alignment comes
> into the game? The only thing I can think of is to not report VMAs
> smaller than the THP as eligible. Is this what you are looking for?
Basically, if the faulting VA is one that cannot be mapped with a THP
due to alignment or size constraints, it may be "eligible" for THP
mapping but ultimately can't be.
I was just double checking that this was meant to be more of a check done
before code elsewhere performs additional checks and does the actual THP
mapping, not an all-encompassing go/no go check for THP mapping.
Thanks,
William Kucharski
^ permalink raw reply
* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Dave Hansen @ 2018-11-27 15:31 UTC (permalink / raw)
To: Florian Weimer, Ram Pai; +Cc: linux-mm, linuxppc-dev, linux-api
In-Reply-To: <87zhtuhgx0.fsf@oldenburg.str.redhat.com>
On 11/27/18 3:57 AM, Florian Weimer wrote:
> I would have expected something that translates PKEY_DISABLE_WRITE |
> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts
> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER.
>
> (My understanding is that PKEY_DISABLE_ACCESS does not disable all
> access, but produces execute-only memory.)
Correct, it disables all data access, but not execution.
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: Michal Hocko @ 2018-11-27 16:25 UTC (permalink / raw)
To: William Kucharski
Cc: linux-api, Andrew Morton, Alexey Dobriyan, linux-mm, LKML
In-Reply-To: <04647F77-FE93-4A8E-90C1-4245709B88A5@oracle.com>
On Tue 27-11-18 07:50:08, William Kucharski wrote:
>
>
> > On Nov 27, 2018, at 6:17 AM, Michal Hocko <mhocko@kernel.org> wrote:
> >
> > This is only about the process wide flag to disable THP. I do not see
> > how this can be alighnement related. I suspect you wanted to ask in the
> > smaps patch?
>
> No, answered below.
>
> >
> >> I'm having to deal with both these issues in the text page THP
> >> prototype I've been working on for some time now.
> >
> > Could you be more specific about the issue and how the alignment comes
> > into the game? The only thing I can think of is to not report VMAs
> > smaller than the THP as eligible. Is this what you are looking for?
>
> Basically, if the faulting VA is one that cannot be mapped with a THP
> due to alignment or size constraints, it may be "eligible" for THP
> mapping but ultimately can't be.
>
> I was just double checking that this was meant to be more of a check done
> before code elsewhere performs additional checks and does the actual THP
> mapping, not an all-encompassing go/no go check for THP mapping.
I am still not sure I follow you completely here. This just reports
per-task eligibility. The system wide eligibility is reported via sysfs
and the per vma eligibility is reported via /proc/<pid>/smaps.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: Vlastimil Babka @ 2018-11-27 16:50 UTC (permalink / raw)
To: William Kucharski, Michal Hocko
Cc: linux-api, Andrew Morton, Alexey Dobriyan, linux-mm, LKML
In-Reply-To: <04647F77-FE93-4A8E-90C1-4245709B88A5@oracle.com>
On 11/27/18 3:50 PM, William Kucharski wrote:
>
> I was just double checking that this was meant to be more of a check done
> before code elsewhere performs additional checks and does the actual THP
> mapping, not an all-encompassing go/no go check for THP mapping.
Yes, the code doing the actual mapping is still checking also alignment etc.
^ permalink raw reply
* Re: [RFC PATCH 3/3] mm, proc: report PR_SET_THP_DISABLE in proc
From: William Kucharski @ 2018-11-27 17:06 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Michal Hocko, linux-api, Andrew Morton, Alexey Dobriyan, linux-mm,
LKML
In-Reply-To: <283f38d9-1142-60b6-0b84-7129b7f9781e@suse.cz>
> On Nov 27, 2018, at 9:50 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 11/27/18 3:50 PM, William Kucharski wrote:
>>
>> I was just double checking that this was meant to be more of a check done
>> before code elsewhere performs additional checks and does the actual THP
>> mapping, not an all-encompassing go/no go check for THP mapping.
>
> Yes, the code doing the actual mapping is still checking also alignment etc.
Thanks, yes, that is what I was getting at.
^ permalink raw reply
* Re: [RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Kees Cook @ 2018-11-27 22:28 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Andy Lutomirski, Elvira Khabirova, Linux API, Jann Horn,
Oleg Nesterov, Steven Rostedt, Ingo Molnar, LKML,
Eugene Syromiatnikov, strace-devel
In-Reply-To: <20181125041003.GA3258@altlinux.org>
On Sat, Nov 24, 2018 at 8:10 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Fri, Nov 23, 2018 at 07:01:39AM +0300, Dmitry V. Levin wrote:
>> On Thu, Nov 22, 2018 at 04:19:10PM -0800, Andy Lutomirski wrote:
>> > On Thu, Nov 22, 2018 at 11:15 AM Dmitry V. Levin wrote:
>> > > On Thu, Nov 22, 2018 at 06:55:29AM -0800, Andy Lutomirski wrote:
>> > > > On Wed, Nov 21, 2018 at 3:56 PM Dmitry V. Levin wrote:
>> > > > > On Wed, Nov 21, 2018 at 02:56:57PM -0800, Andy Lutomirski wrote:
>> > > > > > Please cc linux-api@vger.kernel.org for future versions.
>> > > > > >
>> > > > > > On Wed, Nov 21, 2018 at 7:58 AM Elvira Khabirova wrote:
>> > > > > > >
>> > > > > > > struct ptrace_syscall_info {
>> > > > > > > __u8 op; /* 0 for entry, 1 for exit */
>> > > > > >
>> > > > > > Can you add proper defines, like:
>> > > > > >
>> > > > > > #define PTRACE_SYSCALL_ENTRY 0
>> > > > > > #define PTRACE_SYSCALL_EXIT 1
>> > > > > > #define PTRACE_SYSCALL_SECCOMP 2
>> > > > > >
>> > > > > > and make seccomp work from the start? I'd rather we don't merge an
>> > > > > > implementation that doesn't work for seccomp and then have to rework
>> > > > > > it later.
Yes, please.
>> > > > >
>> > > > > What's the difference between PTRACE_EVENT_SECCOMP and syscall-entry-stop
>> > > > > with regards to PTRACE_GET_SYSCALL_INFO request? At least they have the
>> > > > > same entry_info to return.
>> > > >
>> > > > I'm not sure there's any material difference.
>> > >
>> > > In that case we don't really need PTRACE_SYSCALL_SECCOMP: op field
>> > > describes the structure inside the union to use, not the ptrace stop.
>> >
>> > Unless we think the structures might diverge in the future.
Yes, I want to make sure we have a way to expand this, especially for
seccomp: we've come close a few times to adding new fields to struct
seccomp_data, for example.
>>
>> If these structures ever diverge, then a seccomp structure will be added
>> to the union, and a portable userspace code will likely look this way:
>>
>> #include <linux/ptrace.h>
>> ...
>> struct ptrace_syscall_info info;
>> long rc = ptrace(PTRACE_GET_SYSCALL_INFO, pid, (void *) sizeof(info), &info);
>> ...
>> switch (info.op) {
>> case PTRACE_SYSCALL_INFO_ENTRY:
>> /* handle info.entry */
>> case PTRACE_SYSCALL_INFO_EXIT:
>> /* handle info.exit */
>> #ifdef PTRACE_SYSCALL_INFO_SECCOMP
>> case PTRACE_SYSCALL_INFO_SECCOMP:
>> /* handle info.seccomp */
>> #endif
>> default:
>> /* handle unknown info.op */
>> }
>>
>> In other words, it would be better if PTRACE_SYSCALL_INFO_* selector
>> constants were introduced along with corresponding structures in the
>> union.
>
> However, the approach I suggested doesn't provide forward compatibility:
> if userspace is compiled with kernel headers that don't define
> PTRACE_SYSCALL_INFO_SECCOMP, it will break when the kernel
> starts to use PTRACE_SYSCALL_INFO_SECCOMP instead of
> PTRACE_SYSCALL_INFO_ENTRY for PTRACE_EVENT_SECCOMP support
> in PTRACE_GET_SYSCALL_INFO.
>
> The solution is to introduce PTRACE_SYSCALL_INFO_SECCOMP and struct
> ptrace_syscall_info.seccomp along with PTRACE_EVENT_SECCOMP support
> in PTRACE_GET_SYSCALL_INFO. The initial revision of the seccomp
> structure could be made the same as the entry structure, or it can
> diverge from the beginning, e.g., by adding ret_data field containing
> SECCOMP_RET_DATA return value stored in ptrace_message, this would save
> ptracers an extra PTRACE_GETEVENTMSG call currently required to obtain it.
Yup, that'd be a nice addition.
--
Kees Cook
^ permalink raw reply
* [PATCH 0/1] Add prctl to kill descendants on exit
From: Jürg Billeter @ 2018-11-27 22:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Thomas Gleixner, Eric Biederman, Kees Cook,
Andy Lutomirski, linux-api, linux-kernel, Jürg Billeter
This patch adds a new prctl to kill all descendant processes on exit.
See commit message for details of the prctl.
This is a replacement of PR_SET_PDEATHSIG_PROC I proposed last year [1].
In the following discussion, Oleg suggested this approach.
The motivation for this is to provide a lightweight mechanism to prevent
stray processes. There is also a related Bugzilla entry [2].
PID namespaces can also be used to prevent stray processes, of course.
However, they are not quite as lightweight as they typically also
require a new mount namespace to be able to mount a new /proc. And they
require CAP_SYS_ADMIN. User namespaces can help to gain CAP_SYS_ADMIN,
however, that further increases the overhead and the other effects of
the user namespace may not be desired.
PID 1 in PID namespaces also exhibits non-standard signal behavior
(SIGNAL_UNKILLABLE) [3].
[1] https://lkml.kernel.org/lkml/20170929123058.48924-1-j@bitron.ch/
[2] https://bugzilla.kernel.org/show_bug.cgi?id=43300
[3] https://lkml.kernel.org/lkml/20180803144021.56920-2-j@bitron.ch/
Jürg Billeter (1):
prctl: add PR_{GET,SET}_KILL_DESCENDANTS_ON_EXIT
fs/exec.c | 6 ++++++
include/linux/sched/signal.h | 3 +++
include/uapi/linux/prctl.h | 4 ++++
kernel/exit.c | 12 ++++++++++++
kernel/sys.c | 11 +++++++++++
security/apparmor/lsm.c | 1 +
security/selinux/hooks.c | 3 +++
7 files changed, 40 insertions(+)
--
2.19.2
^ permalink raw reply
* [PATCH] prctl: add PR_{GET,SET}_KILL_DESCENDANTS_ON_EXIT
From: Jürg Billeter @ 2018-11-27 22:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Thomas Gleixner, Eric Biederman, Kees Cook,
Andy Lutomirski, linux-api, linux-kernel, Jürg Billeter
In-Reply-To: <20181127225408.7553-1-j@bitron.ch>
This introduces a new thread group flag that can be set by calling
prctl(PR_SET_KILL_DESCENDANTS_ON_EXIT, 1, 0, 0, 0)
When a thread group exits with this flag set, it will send SIGKILL to
all descendant processes. This can be used to prevent stray child
processes.
This flag is cleared on privilege gaining execve(2) to ensure an
unprivileged process cannot get a privileged process to send SIGKILL.
Descendants that are orphaned and reparented to an ancestor of the
current process before the current process exits, will not be killed.
PR_SET_CHILD_SUBREAPER can be used to contain orphaned processes.
If a descendant gained privileges, the current process may not be
allowed to kill it, and the descendant process will survive.
PR_SET_NO_NEW_PRIVS can be used to prevent descendant processes from
gaining privileges.
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Jürg Billeter <j@bitron.ch>
---
fs/exec.c | 6 ++++++
include/linux/sched/signal.h | 3 +++
include/uapi/linux/prctl.h | 4 ++++
kernel/exit.c | 12 ++++++++++++
kernel/sys.c | 11 +++++++++++
security/apparmor/lsm.c | 1 +
security/selinux/hooks.c | 3 +++
7 files changed, 40 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index 1ebf6e5a521d..f48ff4333393 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1339,6 +1339,12 @@ void setup_new_exec(struct linux_binprm * bprm)
/* Make sure parent cannot signal privileged process. */
current->pdeath_signal = 0;
+ /*
+ * Do not send SIGKILL from privileged process as it may
+ * have been requested by an unprivileged process.
+ */
+ current->signal->kill_descendants_on_exit = 0;
+
/*
* For secureexec, reset the stack limit to sane default to
* avoid bad behavior from the prior rlimits. This has to
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 1be35729c2c5..3bfb71701488 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -124,6 +124,9 @@ struct signal_struct {
unsigned int is_child_subreaper:1;
unsigned int has_child_subreaper:1;
+ /* Send SIGKILL to descendant processes on exit */
+ unsigned int kill_descendants_on_exit:1;
+
#ifdef CONFIG_POSIX_TIMERS
/* POSIX.1b Interval Timers */
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index c0d7ea0bf5b6..2ac4da1f282b 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -198,6 +198,10 @@ struct prctl_mm_map {
# define PR_CAP_AMBIENT_LOWER 3
# define PR_CAP_AMBIENT_CLEAR_ALL 4
+/* Send SIGKILL to descendant processes on exit */
+#define PR_SET_KILL_DESCENDANTS_ON_EXIT 48
+#define PR_GET_KILL_DESCENDANTS_ON_EXIT 49
+
/* arm64 Scalable Vector Extension controls */
/* Flag values must be kept in sync with ptrace NT_ARM_SVE interface */
#define PR_SVE_SET_VL 50 /* set task vector length */
diff --git a/kernel/exit.c b/kernel/exit.c
index 0e21e6d21f35..7fe0c694685a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -694,6 +694,15 @@ static void forget_original_parent(struct task_struct *father,
list_splice_tail_init(&father->children, &reaper->children);
}
+static int kill_descendant_visitor(struct task_struct *p, void *data)
+{
+ /* This may fail, e.g., when a descendant process gained privileges. */
+ group_send_sig_info(SIGKILL, SEND_SIG_NOINFO, p, PIDTYPE_TGID);
+
+ /* Always continue walking the process tree. */
+ return 1;
+}
+
/*
* Send signals to all our closest relatives so that they know
* to properly mourn us..
@@ -704,6 +713,9 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
struct task_struct *p, *n;
LIST_HEAD(dead);
+ if (group_dead && tsk->signal->kill_descendants_on_exit)
+ walk_process_tree(tsk, kill_descendant_visitor, NULL);
+
write_lock_irq(&tasklist_lock);
forget_original_parent(tsk, &dead);
diff --git a/kernel/sys.c b/kernel/sys.c
index 123bd73046ec..8d9af81da093 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2476,6 +2476,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
return -EINVAL;
error = arch_prctl_spec_ctrl_set(me, arg2, arg3);
break;
+ case PR_SET_KILL_DESCENDANTS_ON_EXIT:
+ if (arg3 || arg4 || arg5)
+ return -EINVAL;
+ me->signal->kill_descendants_on_exit = !!arg2;
+ break;
+ case PR_GET_KILL_DESCENDANTS_ON_EXIT:
+ if (arg3 || arg4 || arg5)
+ return -EINVAL;
+ error = put_user(me->signal->kill_descendants_on_exit,
+ (int __user *)arg2);
+ break;
default:
error = -EINVAL;
break;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8b8b70620bbe..d0d8f88130fb 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -695,6 +695,7 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
aa_inherit_files(bprm->cred, current->files);
current->pdeath_signal = 0;
+ current->signal->kill_descendants_on_exit = 0;
/* reset soft limits and set hard limits for the new label */
__aa_transition_rlimits(label, new_label);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..313a7be43a98 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2653,6 +2653,9 @@ static void selinux_bprm_committing_creds(struct linux_binprm *bprm)
/* Always clear parent death signal on SID transitions. */
current->pdeath_signal = 0;
+ /* Disable SIGKILL requested from before the SID transition. */
+ current->signal->kill_descendants_on_exit = 0;
+
/* Check whether the new SID can inherit resource limits from the old
* SID. If not, reset all soft limits to the lower of the current
* task's hard limit and the init task's soft limit.
--
2.19.2
^ permalink raw reply related
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