[PATCH v2 1/6] powerpc/pkeys: Enable all user-allocatable pkeys at init.

[Michael Ellerman]

Ram Pai <linuxram at us.ibm.com> writes:
> On Tue, Jun 19, 2018 at 10:39:52PM +1000, Michael Ellerman wrote:
>> Ram Pai <linuxram at us.ibm.com> writes:
>> 
>> > In a multithreaded application, a key allocated by one thread must
>> > be activate and usable on all threads.
>> >
>> > Currently this is not the case, because the UAMOR bits for all keys are
>> > disabled by default. When a new key is allocated in one thread, though
>> > the corresponding UAMOR bits for that thread get enabled, the UAMOR bits
>> > for all other existing threads continue to have their bits disabled.
>> > Other threads have no way to set permissions on the key, effectively
>> > making the key useless.
>> 
>> This all seems a bit strongly worded to me. It's arguable whether a key
>> should be usable by the thread that allocated it or all threads.
>> 
>> You could conceivably have a design where threads are blocked from using
>> a key until they're given permission to do so by the thread that
>> allocated the key.
>> 
>> But we're changing the behaviour to match x86 and because we don't have
>> an API to grant another thread access to a key. Right?
>
> correct. The other threads have no way to access or change the
> permissions on the key.

OK.

Though prior to patch 6 all threads have read/write permissions for all
keys, so they don't necessarily need to change permissions on a key
allocated by another thread.

>> > Enable the UAMOR bits for all keys, at process creation. Since the
>> > contents of UAMOR are inherited at fork, all threads are capable of
>> > modifying the permissions on any key.
>> >
>> > BTW: changing the permission on unallocated keys has no effect, till
>> > those keys are not associated with any PTEs. The kernel will anyway
>> > disallow to association of unallocated keys with PTEs.
>> 
>> This is an ABI change, which is bad, but I guess we call it a bug fix
>> because things didn't really work previously?
>
> Yes its a behaviorial change for the better. There is no downside
> to the change because no applications should break. Single threaded
> apps will continue to just work fine. Multithreaded applications,
> which were unable to consume the API/ABI, will now be able to do so.

Multi-threaded applications were able to use the API, as long as they
were satisfied with the semantics it provided, ie. that restrictions on
a key were only possible on the thread that allocated the key.

I'm not trying to argue for the sake of it, it's important that we
understand the subtleties of what we're changing and how it affects
existing software - even if we think there is essentially no existing
software.

I'll try and massage the change log to capture it.

I ended up with what's below.

cheers

  powerpc/pkeys: Give all threads control of their key permissions
  
  Currently in a multithreaded application, a key allocated by one
  thread is not usable by other threads. By "not usable" we mean that
  other threads are unable to change the access permissions for that
  key for themselves.
  
  When a new key is allocated in one thread, the corresponding UAMOR
  bits for that thread get enabled, however the UAMOR bits for that key
  for all other threads remain disabled.
  
  Other threads have no way to set permissions on the key, and the
  current default permissions are that read/write is enabled for all
  keys, which means the key has no effect for other threads. Although
  that may be the desired behaviour in some circumstances, having all
  threads able to control their permissions for the key is more
  flexible.
  
  The current behaviour also differs from the x86 behaviour, which is
  problematic for users.
  
  To fix this, enable the UAMOR bits for all keys, at process
  creation (in start_thread(), ie exec time). Since the contents of
  UAMOR are inherited at fork, all threads are capable of modifying the
  permissions on any key.
  
  This is technically an ABI break on powerpc, but pkey support is
  fairly new on powerpc and not widely used, and this brings us into
  line with x86.
  
  Fixes: cf43d3b26452 ("powerpc: Enable pkey subsystem")
  Cc: stable at vger.kernel.org # v4.16+
  Tested-by: Florian Weimer <fweimer at redhat.com>
  Signed-off-by: Ram Pai <linuxram at us.ibm.com>
  [mpe: Reword some of the changelog]
  Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>