lkml.org 
[lkml]   [2016]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 6/9] x86, pkeys: add pkey set/get syscalls
    On Jul 9, 2016 1:37 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
    >
    >
    > * Dave Hansen <dave@sr71.net> wrote:
    >
    > > On 07/08/2016 12:18 AM, Ingo Molnar wrote:
    > >
    > > > So the question is, what is user-space going to do? Do any glibc patches
    > > > exist? How are the user-space library side APIs going to look like?
    > >
    > > My goal at the moment is to get folks enabled to the point that they can start
    > > modifying apps to use pkeys without having to patch their kernels.
    > > I don't have confidence that we can design good high-level userspace interfaces
    > > without seeing some real apps try to use the low-level ones and seeing how they
    > > struggle.
    > >
    > > I had some glibc code to do the pkey alloc/free operations, but those aren't
    > > necessary if we're doing it in the kernel. Other than getting the syscall
    > > wrappers in place, I don't have any immediate plans to do anything in glibc.
    > >
    > > Was there something you were expecting to see?
    >
    > Yeah, so (as you probably guessed!) I'm starting to have second thoughts about the
    > complexity of the alloc/free/set/get interface I suggested, and Mel's review
    > certainly strengthened that feeling.
    >
    > I have two worries:
    >
    > 1)
    >
    > A technical worry I have is that the 'pkey allocation interface' does not seem to
    > be taking the per thread property of pkeys into account - while that property
    > would be useful for apps. That is a limitation that seems unjustified.
    >
    > The reason for this is that we are storing the key allocation bitmap in struct_mm,
    > in mm->context.pkey_allocation_map - while we should be storing it in task_struct
    > or thread_info.

    Huh? Doesn't this have to be per mm? Sure, PKRU is per thread, but
    the page tables are shared.

    > 2)
    >
    > My main worry is that it appears at this stage that we are still pretty far away
    > from completely shadowing the hardware pkey state in the kernel - and without that
    > we cannot really force user-space to use the 'proper' APIs. They can just use the
    > raw instructions, condition them on a CPUID and be done with it: everything can be
    > organized in user-space.
    >

    My vote would be to keep the allocation mechanism but get rid of pkey_set.

    Also, I think the debug poisoning feature is overcomplicated. Let's
    just forbid mprotect_key with an unallocated key.

    There are still two issues that I think we need to address, though:

    1. Signal delivery shouldn't unconditionally clear PKRU. That's what
    the current patches do, and it's unsafe. I'd rather set PKRU to the
    maximally locked down state on signal delivery (except for the
    PROT_EXEC key), although that might cause its own set of problems.

    2. When thread A allocates a pkey, how does it lock down thread B?

    #2 could be addressed by using fully-locked-down as the initial state
    post-exec() and copying the state on clone(). Dave, are there any
    cases in practice where one thread would allocate a pkey and want
    other threads to immediately have access to the memory with that key?

    I find myself wondering whether we should stop using XSAVE for PKRU
    sooner rather than later. If we do anything like the above, we
    completely lose the init optimization, and the code would be a good
    deal simpler if we switched PKRU directly in switch_to and could
    therefore treat it like a normal register everywhere else.

    --Andy

    \
     
     \ /
      Last update: 2016-07-11 07:21    [W:2.544 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site