Re: [PATCH v3] powerpc, pkey: make protection key 0 less special

On Mon, May 07, 2018 at 01:21:49PM +0200, Michal Suchánek wrote:

On Sun, 6 May 2018 13:10:43 -0700
Ram Pai [off-list ref] wrote:

quoted

On Sat, May 05, 2018 at 02:39:56PM +0200, Michal Suchánek wrote:

quoted

On Fri, 4 May 2018 14:45:07 -0700
Ram Pai [off-list ref] wrote:
  

quoted

On Fri, May 04, 2018 at 02:31:10PM -0700, Dave Hansen wrote:  

quoted

On 05/04/2018 02:26 PM, Michal Suchánek wrote:    

quoted

If it is not ok to change permissions of pkey 0 is it ok to
free it?    

It's pretty much never OK to free it on x86 or ppc.  But, we're
not going to put code in to keep userspace from shooting itself
in the foot, at least on x86.    

and on powerpc aswell.  

But once it's free it can be re-allocated. So you are moving the
special-casing from free code to code dealing with allocation.  

Actually if an application frees key-0, it has potentially opened up a
can-of-worms. It could step on anything that explodes. 

Its choice between imposing policies on an application v/s freeing it
up to choose its own policy. I think the kernel should impose some
form of mild-policy. But others think there should be none.

It is a problem of the application when it frees a key it still needs.
I am not for or against allowing that. The problem is with the
interface change once the key is freed.

quoted

quoted

If you want something like allocate_exec_only_pkey then the function
(either in kernel or in userspace) needs to make sure it is not
getting/requesting key 0 on powerpc.  

Yes. makes sense. I will put in some checks towards that.

Suppose an application is adapted to take advantage of freeing key
0, perhaps to revoke access to any code and data used at runtime
initialization which is not longer needed. 

As I understand it with the proposed change any address range not
associated with non-default key becomes inaccessible and key 0 becomes
available for allocation again.

The above sentence is difficult to parse. I think what you are saying is --
	Any address range associated with key-0 become inaccessible if key-0
	is freed.  Is that a correct simplification of the above
	statement?

Assuming I got it right --  

	Yes. key-0 when freed becomes available for allocation again.
	No. When key-0 is freed any address range
associated with key-0 will continue to be accessible.  It was accessible
to begin with and will continue to be accessible.  It was accessible,
   because kernel; during task-creation, never disables any permissions on
   key-0, and also never lets userspace disable any permissions on
   key-0.

The problem arises, when key-0 gets reallocated at a later time. If the
key-0 at that point(during reallocation) is treated like any other key;
allowing userspace to change its permissions, it can explode on
access to any page associated with key-0. (BTW: almost everything is
associated with key-0 by default).

This is fine on x86 where key 0 is fully functional. 

However, on powerpc the application now has key 0 available and it is
not fully a fully functional key. So the application now needs to check
that the key it is allocating is not key 0. Otherwise further key
operations may unexpectedly fail.

To prevent this I would suggest

a) do not allow allocating key 0. If it is freed it becomes reserved

Certainly yes. We never promised any particular key to the user space. So
not returning key-0 during allocation should break no semantics.

b) never expose key 0 to applications. Allocate key 2 as the default
key and present the key numbers with an offset to userspace so key 2
appears as key 0 to user applications.

this is complicated. offseting the number is not easy. if kernel
has allocated key-2 and tells the user space to use key-0, userspace
will use bit-0 and bit-1 of the AMR register to program the permissions
on the key, and cpu will start applying those changes on key-0.
Basically kernel, userspace and cpu will be out-of-sync. This is
possible if the userspace uses a shift-aware library function to program
the AMR. BTW: A library function integrated into glibc is yet to be
defined AFAICK.

Thanks

Michal

-- 
Ram Pai