From: Andy Lutomirski <luto@amacapital.net> To: Dave Hansen <dave.hansen@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, Dave Hansen <dave.hansen@linux.intel.com>, Al Viro <viro@zeniv.linux.org.uk>, X86 ML <x86@kernel.org>, Hugh Dickins <hughd@google.com>, Andrew Morton <akpm@linux-foundation.org>, Linux API <linux-api@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>, Mel Gorman <mgorman@techsingularity.net>, Linus Torvalds <torvalds@linux-foundation.org>, linux-arch <linux-arch@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, Arnd Bergmann <arnd@arndb.de>, Peter Zijlstra <a.p.zijlstra@chello.nl>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "H. Peter Anvin" <hpa@zytor.com> Subject: Re: [PATCH 6/9] x86, pkeys: add pkey set/get syscalls Date: Tue, 12 Jul 2016 15:55:45 -0700 [thread overview] Message-ID: <CALCETrVfmYm5jzM=JWCS0NjBA4VFouren2X22w7M+gLBQF-W4w@mail.gmail.com> (raw) In-Reply-To: <578524E0.6080401@intel.com> On Tue, Jul 12, 2016 at 10:12 AM, Dave Hansen <dave.hansen@intel.com> wrote: > On 07/12/2016 09:32 AM, Andy Lutomirski wrote: >> I think it's more or less impossible to get sensible behavior passing >> pkey != 0 data to legacy functions. If you call: >> >> void frob(struct foo *p); >> >> If frob in turn passes p to a thread, what PKRU is it supposed to use? > > The thread inheritance of PKRU can be nice. It actually gives things a > good chance of working if you can control PKRU before clone(). I'd > describe the semantics like this: > > PKRU values are inherited at the time of a clone() system > call. Threads unaware of protection keys may work on > protection-key-protected data as long as PKRU is set up in > advance of the clone() and never needs to be changed inside the > thread. > > If a thread is created before PKRU is set appropriately, the > thread may not be able to act on protection-key-protected data. Given the apparent need for seccomp's TSYNC, I'm a bit nervous that this will be restrictive to a problematic degree. > > Otherwise, the semantics are simpler, but they basically give threads no > chance of ever working: > > Threads unaware of protection keys and which can not manage > PKRU may not operate on data where a non-zero key has been > passed to pkey_mprotect(). > > It isn't clear to me that one of these is substantially better than the > other. It's fairly easy in either case for an app that cares to get the > behavior of the other. > > But, one is clearly easier to implement in the kernel. :) > >>>> So how is user code supposed lock down all of its threads? >>>> >>>> seccomp has TSYNC for this, but I don't think that PKRU allows >>>> something like that. >>> >>> I'm not sure this is possible for PKRU. Think of a simple PKRU >>> manipulation in userspace: >>> >>> pkru = rdpkru(); >>> pkru |= PKEY_DENY_ACCESS<<key*2; >>> wrpkru(pkru); >>> >>> If we push a PKRU value into a thread between the rdpkru() and wrpkru(), >>> we'll lose the content of that "push". I'm not sure there's any way to >>> guarantee this with a user-controlled register. >> >> We could try to insist that user code uses some vsyscall helper that >> tracks which bits are as-yet-unassigned. That's quite messy, though. > > Yeah, doable, but not without some new data going out to userspace, plus > the vsyscall code itself. > >> We could also arbitrarily partition the key space into >> initially-wide-open, initially-read-only, and initially-no-access and >> let pkey_alloc say which kind it wants. > > The point is still that wrpkru destroyed the 'push' operation. You > always end up with a PKRU that (at least temporarily) ignored the 'push'. > Not with my partitioning proposal. We'd never asynchronously modify another thread's state -- we'd start start with a mask that gives us a good chance of having the initial state always be useful. To be completely precise, the initial state would be something like: 0 = all access, 1 (PROT_EXEC) = deny read and write, 2-11: deny read and write, 12-21: deny write, 22-31: all access Then pkru_alloc would take a parameter giving the requested initial state, and it would only work if a key with that initial state is available. If we went with the vdso approach, the API could look like: pkru_state_t prev = pkru_push(mask, value); ... pkru_pop(prev); // or pkru_pop(mask, prev)? This doesn't fundamentally require the vdso, except that implementing bitwise operations on PKRU can't be done atomically with RDPKRU / WRPKRU. Grr. This also falls apart pretty badly when sigreturn happens, so I don't think I like this approach. --Andy
WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@amacapital.net> To: Dave Hansen <dave.hansen@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, Dave Hansen <dave.hansen@linux.intel.com>, Al Viro <viro@zeniv.linux.org.uk>, X86 ML <x86@kernel.org>, Hugh Dickins <hughd@google.com>, Andrew Morton <akpm@linux-foundation.org>, Linux API <linux-api@vger.kernel.org>, Ingo Molnar <mingo@kernel.org>, Mel Gorman <mgorman@techsingularity.net>, Linus Torvalds <torvalds@linux-foundation.org>, linux-arch <linux-arch@vger.kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, Arnd Bergmann <arnd@arndb.de>, Peter Zijlstra <a.p.zijlstra@chello.nl>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "H. Peter Anvin" <hpa@zytor.com> Subject: Re: [PATCH 6/9] x86, pkeys: add pkey set/get syscalls Date: Tue, 12 Jul 2016 15:55:45 -0700 [thread overview] Message-ID: <CALCETrVfmYm5jzM=JWCS0NjBA4VFouren2X22w7M+gLBQF-W4w@mail.gmail.com> (raw) In-Reply-To: <578524E0.6080401@intel.com> On Tue, Jul 12, 2016 at 10:12 AM, Dave Hansen <dave.hansen@intel.com> wrote: > On 07/12/2016 09:32 AM, Andy Lutomirski wrote: >> I think it's more or less impossible to get sensible behavior passing >> pkey != 0 data to legacy functions. If you call: >> >> void frob(struct foo *p); >> >> If frob in turn passes p to a thread, what PKRU is it supposed to use? > > The thread inheritance of PKRU can be nice. It actually gives things a > good chance of working if you can control PKRU before clone(). I'd > describe the semantics like this: > > PKRU values are inherited at the time of a clone() system > call. Threads unaware of protection keys may work on > protection-key-protected data as long as PKRU is set up in > advance of the clone() and never needs to be changed inside the > thread. > > If a thread is created before PKRU is set appropriately, the > thread may not be able to act on protection-key-protected data. Given the apparent need for seccomp's TSYNC, I'm a bit nervous that this will be restrictive to a problematic degree. > > Otherwise, the semantics are simpler, but they basically give threads no > chance of ever working: > > Threads unaware of protection keys and which can not manage > PKRU may not operate on data where a non-zero key has been > passed to pkey_mprotect(). > > It isn't clear to me that one of these is substantially better than the > other. It's fairly easy in either case for an app that cares to get the > behavior of the other. > > But, one is clearly easier to implement in the kernel. :) > >>>> So how is user code supposed lock down all of its threads? >>>> >>>> seccomp has TSYNC for this, but I don't think that PKRU allows >>>> something like that. >>> >>> I'm not sure this is possible for PKRU. Think of a simple PKRU >>> manipulation in userspace: >>> >>> pkru = rdpkru(); >>> pkru |= PKEY_DENY_ACCESS<<key*2; >>> wrpkru(pkru); >>> >>> If we push a PKRU value into a thread between the rdpkru() and wrpkru(), >>> we'll lose the content of that "push". I'm not sure there's any way to >>> guarantee this with a user-controlled register. >> >> We could try to insist that user code uses some vsyscall helper that >> tracks which bits are as-yet-unassigned. That's quite messy, though. > > Yeah, doable, but not without some new data going out to userspace, plus > the vsyscall code itself. > >> We could also arbitrarily partition the key space into >> initially-wide-open, initially-read-only, and initially-no-access and >> let pkey_alloc say which kind it wants. > > The point is still that wrpkru destroyed the 'push' operation. You > always end up with a PKRU that (at least temporarily) ignored the 'push'. > Not with my partitioning proposal. We'd never asynchronously modify another thread's state -- we'd start start with a mask that gives us a good chance of having the initial state always be useful. To be completely precise, the initial state would be something like: 0 = all access, 1 (PROT_EXEC) = deny read and write, 2-11: deny read and write, 12-21: deny write, 22-31: all access Then pkru_alloc would take a parameter giving the requested initial state, and it would only work if a key with that initial state is available. If we went with the vdso approach, the API could look like: pkru_state_t prev = pkru_push(mask, value); ... pkru_pop(prev); // or pkru_pop(mask, prev)? This doesn't fundamentally require the vdso, except that implementing bitwise operations on PKRU can't be done atomically with RDPKRU / WRPKRU. Grr. This also falls apart pretty badly when sigreturn happens, so I don't think I like this approach. --Andy -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-07-12 22:56 UTC|newest] Thread overview: 103+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-07 12:47 [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault bit Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 14:40 ` Mel Gorman 2016-07-07 14:40 ` Mel Gorman 2016-07-07 15:42 ` Dave Hansen 2016-07-07 15:42 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 2/9] mm: implement new pkey_mprotect() system call Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 14:40 ` Mel Gorman 2016-07-07 14:40 ` Mel Gorman 2016-07-07 16:51 ` Dave Hansen 2016-07-07 16:51 ` Dave Hansen 2016-07-08 10:15 ` Mel Gorman 2016-07-08 10:15 ` Mel Gorman 2016-07-07 12:47 ` [PATCH 3/9] x86, pkeys: make mprotect_key() mask off additional vm_flags Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 4/9] x86: wire up mprotect_key() system call Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 5/9] x86, pkeys: allocation/free syscalls Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 14:40 ` Mel Gorman 2016-07-07 14:40 ` Mel Gorman 2016-07-07 15:38 ` Dave Hansen 2016-07-07 15:38 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 6/9] x86, pkeys: add pkey set/get syscalls Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 14:45 ` Mel Gorman 2016-07-07 14:45 ` Mel Gorman 2016-07-07 17:33 ` Dave Hansen 2016-07-07 17:33 ` Dave Hansen 2016-07-08 7:18 ` Ingo Molnar 2016-07-08 7:18 ` Ingo Molnar 2016-07-08 16:32 ` Dave Hansen 2016-07-08 16:32 ` Dave Hansen 2016-07-08 16:32 ` Dave Hansen 2016-07-09 8:37 ` Ingo Molnar 2016-07-09 8:37 ` Ingo Molnar 2016-07-09 8:37 ` Ingo Molnar 2016-07-11 4:25 ` Andy Lutomirski 2016-07-11 4:25 ` Andy Lutomirski 2016-07-11 7:35 ` Ingo Molnar 2016-07-11 7:35 ` Ingo Molnar 2016-07-11 7:35 ` Ingo Molnar 2016-07-11 14:28 ` Dave Hansen 2016-07-11 14:28 ` Dave Hansen 2016-07-12 7:13 ` Ingo Molnar 2016-07-12 7:13 ` Ingo Molnar 2016-07-12 15:39 ` Dave Hansen 2016-07-12 15:39 ` Dave Hansen 2016-07-11 14:50 ` Andy Lutomirski 2016-07-11 14:50 ` Andy Lutomirski 2016-07-11 14:34 ` Dave Hansen 2016-07-11 14:34 ` Dave Hansen 2016-07-11 14:34 ` Dave Hansen 2016-07-11 14:45 ` Andy Lutomirski 2016-07-11 14:45 ` Andy Lutomirski 2016-07-11 15:48 ` Dave Hansen 2016-07-11 15:48 ` Dave Hansen 2016-07-12 16:32 ` Andy Lutomirski 2016-07-12 16:32 ` Andy Lutomirski 2016-07-12 17:12 ` Dave Hansen 2016-07-12 17:12 ` Dave Hansen 2016-07-12 22:55 ` Andy Lutomirski [this message] 2016-07-12 22:55 ` Andy Lutomirski 2016-07-13 7:56 ` Ingo Molnar 2016-07-13 7:56 ` Ingo Molnar 2016-07-13 18:43 ` Andy Lutomirski 2016-07-13 18:43 ` Andy Lutomirski 2016-07-14 8:07 ` Ingo Molnar 2016-07-14 8:07 ` Ingo Molnar 2016-07-18 4:43 ` Andy Lutomirski 2016-07-18 4:43 ` Andy Lutomirski 2016-07-18 9:56 ` Ingo Molnar 2016-07-18 9:56 ` Ingo Molnar 2016-07-18 18:02 ` Dave Hansen 2016-07-18 18:02 ` Dave Hansen 2016-07-18 18:02 ` Dave Hansen 2016-07-18 20:12 ` Dave Hansen 2016-07-18 20:12 ` Dave Hansen 2016-07-18 20:12 ` Dave Hansen 2016-07-08 19:26 ` Dave Hansen 2016-07-08 19:26 ` Dave Hansen 2016-07-08 10:22 ` Mel Gorman 2016-07-08 10:22 ` Mel Gorman 2016-07-08 10:22 ` Mel Gorman 2016-07-07 12:47 ` [PATCH 7/9] generic syscalls: wire up memory protection keys syscalls Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 8/9] pkeys: add details of system call use to Documentation/ Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 12:47 ` [PATCH 9/9] x86, pkeys: add self-tests Dave Hansen 2016-07-07 12:47 ` Dave Hansen 2016-07-07 14:47 ` [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys Mel Gorman 2016-07-07 14:47 ` Mel Gorman 2016-07-07 14:47 ` Mel Gorman 2016-07-08 18:38 ` Hugh Dickins 2016-07-08 18:38 ` Hugh Dickins 2016-07-08 18:38 ` Hugh Dickins -- strict thread matches above, loose matches on Subject: below -- 2016-06-09 0:01 [PATCH 0/9] [v3] " Dave Hansen 2016-06-09 0:01 ` [PATCH 6/9] x86, pkeys: add pkey set/get syscalls Dave Hansen 2016-06-09 0:01 ` Dave Hansen 2016-06-07 20:47 [PATCH 0/9] [v2] System Calls for Memory Protection Keys Dave Hansen 2016-06-07 20:47 ` [PATCH 6/9] x86, pkeys: add pkey set/get syscalls Dave Hansen 2016-06-07 20:47 ` Dave Hansen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CALCETrVfmYm5jzM=JWCS0NjBA4VFouren2X22w7M+gLBQF-W4w@mail.gmail.com' \ --to=luto@amacapital.net \ --cc=a.p.zijlstra@chello.nl \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=dave.hansen@intel.com \ --cc=dave.hansen@linux.intel.com \ --cc=hpa@zytor.com \ --cc=hughd@google.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@techsingularity.net \ --cc=mingo@kernel.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=viro@zeniv.linux.org.uk \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.