Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Michael Ellerman]

Dave Hansen  writes:

> On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
>>> This is a bit nicer than what Ram proposed because it is simpler
>>> and removes special-casing for pkey 0.  On the other hand, it does
>>> allow applciations to pkey_free() pkey-0, but that's just a silly
>>> thing to do, so we are not going to protect against it.
>> What's the consequence of that? Application crashing and burning itself or
>> something more subtle?
>
> You would have to:
>
>   pkey_free(0)
>   ... later
>   new_key = pkey_alloc();
>   // now new_key=0
>   pkey_deny_access(new_key); // or whatever
>
> At which point most apps would probably croak because its stack is
> inaccessible.  The free itself does not make the key inaccessible, *but*
> we could also do that within the existing ABI if we want.  I think I
> called out that behavior as undefined in the manpage.

Allowing key 0 to be freed introduces some pretty weird API IMHO. For
example this part of the manpage:

  An application should not call pkey_free() on any protection key
  which has been assigned to an address range by pkey_mprotect(2)
  and which is still in use. The behavior in this case is undefined
  and may result in an error.

You basically can't avoid hitting undefined behaviour with pkey 0,
because even if you never assigned pkey 0 to an address range, it *is
still in use* - because it's used as the default key for every address
range that doesn't have another key.

So I don't really think it makes sense to allow pkey 0 to be freed. But
I won't die in a ditch over it, I just look forward to a manpage update
that can sensibly describe the semantics.

cheers

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Michael Ellerman]

Dave Hansen  writes:

> On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
>>> This is a bit nicer than what Ram proposed because it is simpler
>>> and removes special-casing for pkey 0.  On the other hand, it does
>>> allow applciations to pkey_free() pkey-0, but that's just a silly
>>> thing to do, so we are not going to protect against it.
>> What's the consequence of that? Application crashing and burning itself or
>> something more subtle?
>
> You would have to:
>
>   pkey_free(0)
>   ... later
>   new_key = pkey_alloc();
>   // now new_key=0
>   pkey_deny_access(new_key); // or whatever
>
> At which point most apps would probably croak because its stack is
> inaccessible.  The free itself does not make the key inaccessible, *but*
> we could also do that within the existing ABI if we want.  I think I
> called out that behavior as undefined in the manpage.

Allowing key 0 to be freed introduces some pretty weird API IMHO. For
example this part of the manpage:

  An application should not call pkey_free() on any protection key
  which has been assigned to an address range by pkey_mprotect(2)
  and which is still in use. The behavior in this case is undefined
  and may result in an error.

You basically can't avoid hitting undefined behaviour with pkey 0,
because even if you never assigned pkey 0 to an address range, it *is
still in use* - because it's used as the default key for every address
range that doesn't have another key.

So I don't really think it makes sense to allow pkey 0 to be freed. But
I won't die in a ditch over it, I just look forward to a manpage update
that can sensibly describe the semantics.

cheers

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Ram Pai]

On Sun, Mar 18, 2018 at 10:30:48AM +0100, Thomas Gleixner wrote:
> On Sat, 17 Mar 2018, Ram Pai wrote:
> > On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> > > 
> > > From: Dave Hansen 
> > > 
> > > mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> > > inconsistent with the manpages, and also inconsistent with
> > > mm->context.pkey_allocation_map.  Stop special casing it and only
> > > disallow values that are actually bad (< 0).
> > > 
> > > The end-user visible effect of this is that you can now use
> > > mprotect_pkey() to set pkey=0.
> > > 
> > > This is a bit nicer than what Ram proposed because it is simpler
> > > and removes special-casing for pkey 0.  On the other hand, it does
> > > allow applciations to pkey_free() pkey-0, but that's just a silly
> > > thing to do, so we are not going to protect against it.
> > 
> > So your proposal 
> > (a) allocates pkey 0 implicitly, 
> > (b) does not stop anyone from freeing pkey-0
> > (c) and allows pkey-0 to be explicitly associated with any address range.
> > correct?
> > 
> > My proposal
> > (a) allocates pkey 0 implicitly, 
> > (b) stops anyone from freeing pkey-0
> > (c) and allows pkey-0 to be explicitly associated with any address range.
> > 
> > So the difference between the two proposals is just the freeing part i.e 
> > (b).
> > Did I get this right?
> 
> Yes, and that's consistent with the other pkeys.
> 

ok.

Yes it makes pkey-0 even more consistent with the other keys, but not
entirely consistent.  pkey-0 still has the priviledge of being
allocated by default.


RP

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Ram Pai]

On Sun, Mar 18, 2018 at 10:30:48AM +0100, Thomas Gleixner wrote:
> On Sat, 17 Mar 2018, Ram Pai wrote:
> > On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> > > 
> > > From: Dave Hansen 
> > > 
> > > mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> > > inconsistent with the manpages, and also inconsistent with
> > > mm->context.pkey_allocation_map.  Stop special casing it and only
> > > disallow values that are actually bad (< 0).
> > > 
> > > The end-user visible effect of this is that you can now use
> > > mprotect_pkey() to set pkey=0.
> > > 
> > > This is a bit nicer than what Ram proposed because it is simpler
> > > and removes special-casing for pkey 0.  On the other hand, it does
> > > allow applciations to pkey_free() pkey-0, but that's just a silly
> > > thing to do, so we are not going to protect against it.
> > 
> > So your proposal 
> > (a) allocates pkey 0 implicitly, 
> > (b) does not stop anyone from freeing pkey-0
> > (c) and allows pkey-0 to be explicitly associated with any address range.
> > correct?
> > 
> > My proposal
> > (a) allocates pkey 0 implicitly, 
> > (b) stops anyone from freeing pkey-0
> > (c) and allows pkey-0 to be explicitly associated with any address range.
> > 
> > So the difference between the two proposals is just the freeing part i.e 
> > (b).
> > Did I get this right?
> 
> Yes, and that's consistent with the other pkeys.
> 

ok.

Yes it makes pkey-0 even more consistent with the other keys, but not
entirely consistent.  pkey-0 still has the priviledge of being
allocated by default.


RP

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Sat, 17 Mar 2018, Ram Pai wrote:
> On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> > 
> > From: Dave Hansen 
> > 
> > mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> > inconsistent with the manpages, and also inconsistent with
> > mm->context.pkey_allocation_map.  Stop special casing it and only
> > disallow values that are actually bad (< 0).
> > 
> > The end-user visible effect of this is that you can now use
> > mprotect_pkey() to set pkey=0.
> > 
> > This is a bit nicer than what Ram proposed because it is simpler
> > and removes special-casing for pkey 0.  On the other hand, it does
> > allow applciations to pkey_free() pkey-0, but that's just a silly
> > thing to do, so we are not going to protect against it.
> 
> So your proposal 
> (a) allocates pkey 0 implicitly, 
> (b) does not stop anyone from freeing pkey-0
> (c) and allows pkey-0 to be explicitly associated with any address range.
> correct?
> 
> My proposal
> (a) allocates pkey 0 implicitly, 
> (b) stops anyone from freeing pkey-0
> (c) and allows pkey-0 to be explicitly associated with any address range.
> 
> So the difference between the two proposals is just the freeing part i.e (b).
> Did I get this right?

Yes, and that's consistent with the other pkeys.

> Its a philosophical debate; allow the user to shoot-in-the-feet or stop
> from not doing so. There is no clear answer either way. I am fine either
> way.

The user can shoot himself already with the other pkeys, so adding another
one does not matter and is again consistent.

Thanks,

tglx

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Sat, 17 Mar 2018, Ram Pai wrote:
> On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> > 
> > From: Dave Hansen 
> > 
> > mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> > inconsistent with the manpages, and also inconsistent with
> > mm->context.pkey_allocation_map.  Stop special casing it and only
> > disallow values that are actually bad (< 0).
> > 
> > The end-user visible effect of this is that you can now use
> > mprotect_pkey() to set pkey=0.
> > 
> > This is a bit nicer than what Ram proposed because it is simpler
> > and removes special-casing for pkey 0.  On the other hand, it does
> > allow applciations to pkey_free() pkey-0, but that's just a silly
> > thing to do, so we are not going to protect against it.
> 
> So your proposal 
> (a) allocates pkey 0 implicitly, 
> (b) does not stop anyone from freeing pkey-0
> (c) and allows pkey-0 to be explicitly associated with any address range.
> correct?
> 
> My proposal
> (a) allocates pkey 0 implicitly, 
> (b) stops anyone from freeing pkey-0
> (c) and allows pkey-0 to be explicitly associated with any address range.
> 
> So the difference between the two proposals is just the freeing part i.e (b).
> Did I get this right?

Yes, and that's consistent with the other pkeys.

> Its a philosophical debate; allow the user to shoot-in-the-feet or stop
> from not doing so. There is no clear answer either way. I am fine either
> way.

The user can shoot himself already with the other pkeys, so adding another
one does not matter and is again consistent.

Thanks,

tglx

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

On 03/17/2018 04:24 PM, Ram Pai wrote:
> So the difference between the two proposals is just the freeing part i.e (b).
> Did I get this right?

Yeah, I think that's the only difference.

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

On 03/17/2018 04:24 PM, Ram Pai wrote:
> So the difference between the two proposals is just the freeing part i.e (b).
> Did I get this right?

Yeah, I think that's the only difference.

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Ram Pai]

On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> inconsistent with the manpages, and also inconsistent with
> mm->context.pkey_allocation_map.  Stop special casing it and only
> disallow values that are actually bad (< 0).
> 
> The end-user visible effect of this is that you can now use
> mprotect_pkey() to set pkey=0.
> 
> This is a bit nicer than what Ram proposed because it is simpler
> and removes special-casing for pkey 0.  On the other hand, it does
> allow applciations to pkey_free() pkey-0, but that's just a silly
> thing to do, so we are not going to protect against it.

So your proposal 
(a) allocates pkey 0 implicitly, 
(b) does not stop anyone from freeing pkey-0
(c) and allows pkey-0 to be explicitly associated with any address range.
correct?

My proposal
(a) allocates pkey 0 implicitly, 
(b) stops anyone from freeing pkey-0
(c) and allows pkey-0 to be explicitly associated with any address range.

So the difference between the two proposals is just the freeing part i.e (b).
Did I get this right?

Its a philosophical debate; allow the user
to shoot-in-the-feet or stop from not doing so. There is no 
clear answer either way. I am fine either way.

So here is my

Reviewed-by: Ram Pai 

I will write a corresponding patch for powerpc.

> 
> Signed-off-by: Dave Hansen 
> Cc: Ram Pai 
> Cc: Thomas Gleixner 
> Cc: Dave Hansen 
> Cc: Michael Ellermen 
> Cc: Ingo Molnar 
> Cc: Andrew Morton p
> Cc: Shuah Khan 
> ---
> 
>  b/arch/x86/include/asm/mmu_context.h |2 +-
>  b/arch/x86/include/asm/pkeys.h   |6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff -puN arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
> arch/x86/include/asm/mmu_context.h
> --- a/arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
> 2018-03-16 14:46:39.023285476 -0700
> +++ b/arch/x86/include/asm/mmu_context.h  2018-03-16 14:46:39.028285476 
> -0700
> @@ -191,7 +191,7 @@ static inline int init_new_context(struc
> 
>  #ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
>   if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
> - /* pkey 0 is the default and always allocated */
> + /* pkey 0 is the default and allocated implicitly */
>   mm->context.pkey_allocation_map = 0x1;
>   /* -1 means unallocated or invalid */
>   mm->context.execute_only_pkey = -1;
> diff -puN arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 
> arch/x86/include/asm/pkeys.h
> --- a/arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated   
> 2018-03-16 14:46:39.025285476 -0700
> +++ b/arch/x86/include/asm/pkeys.h2018-03-16 14:46:39.028285476 -0700
> @@ -49,10 +49,10 @@ bool mm_pkey_is_allocated(struct mm_stru
>  {
>   /*
>* "Allocated" pkeys are those that have been returned
> -  * from pkey_alloc().  pkey 0 is special, and never
> -  * returned from pkey_alloc().
> +  * from pkey_alloc() or pkey 0 which is allocated
> +  * implicitly when the mm is created.
>*/
> - if (pkey <= 0)
> + if (pkey < 0)
>   return false;
>   if (pkey >= arch_max_pkey())
>   return false;
> _

-- 
Ram Pai

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Ram Pai]

On Fri, Mar 16, 2018 at 02:46:56PM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen 
> 
> mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> inconsistent with the manpages, and also inconsistent with
> mm->context.pkey_allocation_map.  Stop special casing it and only
> disallow values that are actually bad (< 0).
> 
> The end-user visible effect of this is that you can now use
> mprotect_pkey() to set pkey=0.
> 
> This is a bit nicer than what Ram proposed because it is simpler
> and removes special-casing for pkey 0.  On the other hand, it does
> allow applciations to pkey_free() pkey-0, but that's just a silly
> thing to do, so we are not going to protect against it.

So your proposal 
(a) allocates pkey 0 implicitly, 
(b) does not stop anyone from freeing pkey-0
(c) and allows pkey-0 to be explicitly associated with any address range.
correct?

My proposal
(a) allocates pkey 0 implicitly, 
(b) stops anyone from freeing pkey-0
(c) and allows pkey-0 to be explicitly associated with any address range.

So the difference between the two proposals is just the freeing part i.e (b).
Did I get this right?

Its a philosophical debate; allow the user
to shoot-in-the-feet or stop from not doing so. There is no 
clear answer either way. I am fine either way.

So here is my

Reviewed-by: Ram Pai 

I will write a corresponding patch for powerpc.

> 
> Signed-off-by: Dave Hansen 
> Cc: Ram Pai 
> Cc: Thomas Gleixner 
> Cc: Dave Hansen 
> Cc: Michael Ellermen 
> Cc: Ingo Molnar 
> Cc: Andrew Morton p
> Cc: Shuah Khan 
> ---
> 
>  b/arch/x86/include/asm/mmu_context.h |2 +-
>  b/arch/x86/include/asm/pkeys.h   |6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff -puN arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
> arch/x86/include/asm/mmu_context.h
> --- a/arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
> 2018-03-16 14:46:39.023285476 -0700
> +++ b/arch/x86/include/asm/mmu_context.h  2018-03-16 14:46:39.028285476 
> -0700
> @@ -191,7 +191,7 @@ static inline int init_new_context(struc
> 
>  #ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
>   if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
> - /* pkey 0 is the default and always allocated */
> + /* pkey 0 is the default and allocated implicitly */
>   mm->context.pkey_allocation_map = 0x1;
>   /* -1 means unallocated or invalid */
>   mm->context.execute_only_pkey = -1;
> diff -puN arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 
> arch/x86/include/asm/pkeys.h
> --- a/arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated   
> 2018-03-16 14:46:39.025285476 -0700
> +++ b/arch/x86/include/asm/pkeys.h2018-03-16 14:46:39.028285476 -0700
> @@ -49,10 +49,10 @@ bool mm_pkey_is_allocated(struct mm_stru
>  {
>   /*
>* "Allocated" pkeys are those that have been returned
> -  * from pkey_alloc().  pkey 0 is special, and never
> -  * returned from pkey_alloc().
> +  * from pkey_alloc() or pkey 0 which is allocated
> +  * implicitly when the mm is created.
>*/
> - if (pkey <= 0)
> + if (pkey < 0)
>   return false;
>   if (pkey >= arch_max_pkey())
>   return false;
> _

-- 
Ram Pai

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Sat, 17 Mar 2018, Dave Hansen wrote:
> On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
> >> This is a bit nicer than what Ram proposed because it is simpler
> >> and removes special-casing for pkey 0.  On the other hand, it does
> >> allow applciations to pkey_free() pkey-0, but that's just a silly
> >> thing to do, so we are not going to protect against it.
> > What's the consequence of that? Application crashing and burning itself or
> > something more subtle?
> 
> You would have to:
> 
>   pkey_free(0)
>   ... later
>   new_key = pkey_alloc();
>   // now new_key=0
>   pkey_deny_access(new_key); // or whatever
> 
> At which point most apps would probably croak because its stack is
> inaccessible.  The free itself does not make the key inaccessible, *but*
> we could also do that within the existing ABI if we want.  I think I
> called out that behavior as undefined in the manpage.

As long as its documented and the change only allows people to shoot them
more in the foot, we're all good.

Thanks,

tglx

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Sat, 17 Mar 2018, Dave Hansen wrote:
> On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
> >> This is a bit nicer than what Ram proposed because it is simpler
> >> and removes special-casing for pkey 0.  On the other hand, it does
> >> allow applciations to pkey_free() pkey-0, but that's just a silly
> >> thing to do, so we are not going to protect against it.
> > What's the consequence of that? Application crashing and burning itself or
> > something more subtle?
> 
> You would have to:
> 
>   pkey_free(0)
>   ... later
>   new_key = pkey_alloc();
>   // now new_key=0
>   pkey_deny_access(new_key); // or whatever
> 
> At which point most apps would probably croak because its stack is
> inaccessible.  The free itself does not make the key inaccessible, *but*
> we could also do that within the existing ABI if we want.  I think I
> called out that behavior as undefined in the manpage.

As long as its documented and the change only allows people to shoot them
more in the foot, we're all good.

Thanks,

tglx

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
>> This is a bit nicer than what Ram proposed because it is simpler
>> and removes special-casing for pkey 0.  On the other hand, it does
>> allow applciations to pkey_free() pkey-0, but that's just a silly
>> thing to do, so we are not going to protect against it.
> What's the consequence of that? Application crashing and burning itself or
> something more subtle?

You would have to:

pkey_free(0)
... later
new_key = pkey_alloc();
// now new_key=0
pkey_deny_access(new_key); // or whatever

At which point most apps would probably croak because its stack is
inaccessible.  The free itself does not make the key inaccessible, *but*
we could also do that within the existing ABI if we want.  I think I
called out that behavior as undefined in the manpage.

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

On 03/17/2018 02:12 AM, Thomas Gleixner wrote:
>> This is a bit nicer than what Ram proposed because it is simpler
>> and removes special-casing for pkey 0.  On the other hand, it does
>> allow applciations to pkey_free() pkey-0, but that's just a silly
>> thing to do, so we are not going to protect against it.
> What's the consequence of that? Application crashing and burning itself or
> something more subtle?

You would have to:

pkey_free(0)
... later
new_key = pkey_alloc();
// now new_key=0
pkey_deny_access(new_key); // or whatever

At which point most apps would probably croak because its stack is
inaccessible.  The free itself does not make the key inaccessible, *but*
we could also do that within the existing ABI if we want.  I think I
called out that behavior as undefined in the manpage.

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Fri, 16 Mar 2018, Dave Hansen wrote:

> 
> From: Dave Hansen 
> 
> mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> inconsistent with the manpages, and also inconsistent with
> mm->context.pkey_allocation_map.  Stop special casing it and only
> disallow values that are actually bad (< 0).
> 
> The end-user visible effect of this is that you can now use
> mprotect_pkey() to set pkey=0.
> 
> This is a bit nicer than what Ram proposed because it is simpler
> and removes special-casing for pkey 0.  On the other hand, it does
> allow applciations to pkey_free() pkey-0, but that's just a silly
> thing to do, so we are not going to protect against it.

What's the consequence of that? Application crashing and burning itself or
something more subtle?

Thanks,

tglx

Re: [PATCH 1/3] x86, pkeys: do not special case protection key 0

[Thomas Gleixner]

On Fri, 16 Mar 2018, Dave Hansen wrote:

> 
> From: Dave Hansen 
> 
> mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
> inconsistent with the manpages, and also inconsistent with
> mm->context.pkey_allocation_map.  Stop special casing it and only
> disallow values that are actually bad (< 0).
> 
> The end-user visible effect of this is that you can now use
> mprotect_pkey() to set pkey=0.
> 
> This is a bit nicer than what Ram proposed because it is simpler
> and removes special-casing for pkey 0.  On the other hand, it does
> allow applciations to pkey_free() pkey-0, but that's just a silly
> thing to do, so we are not going to protect against it.

What's the consequence of that? Application crashing and burning itself or
something more subtle?

Thanks,

tglx

[PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

From: Dave Hansen 

mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
inconsistent with the manpages, and also inconsistent with
mm->context.pkey_allocation_map.  Stop special casing it and only
disallow values that are actually bad (< 0).

The end-user visible effect of this is that you can now use
mprotect_pkey() to set pkey=0.

This is a bit nicer than what Ram proposed because it is simpler
and removes special-casing for pkey 0.  On the other hand, it does
allow applciations to pkey_free() pkey-0, but that's just a silly
thing to do, so we are not going to protect against it.

Signed-off-by: Dave Hansen 
Cc: Ram Pai 
Cc: Thomas Gleixner 
Cc: Dave Hansen 
Cc: Michael Ellermen 
Cc: Ingo Molnar 
Cc: Andrew Morton p
Cc: Shuah Khan 
---

 b/arch/x86/include/asm/mmu_context.h |2 +-
 b/arch/x86/include/asm/pkeys.h   |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff -puN arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
arch/x86/include/asm/mmu_context.h
--- a/arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated   
2018-03-16 14:46:39.023285476 -0700
+++ b/arch/x86/include/asm/mmu_context.h2018-03-16 14:46:39.028285476 
-0700
@@ -191,7 +191,7 @@ static inline int init_new_context(struc
 
 #ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
-   /* pkey 0 is the default and always allocated */
+   /* pkey 0 is the default and allocated implicitly */
mm->context.pkey_allocation_map = 0x1;
/* -1 means unallocated or invalid */
mm->context.execute_only_pkey = -1;
diff -puN arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 
arch/x86/include/asm/pkeys.h
--- a/arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 2018-03-16 
14:46:39.025285476 -0700
+++ b/arch/x86/include/asm/pkeys.h  2018-03-16 14:46:39.028285476 -0700
@@ -49,10 +49,10 @@ bool mm_pkey_is_allocated(struct mm_stru
 {
/*
 * "Allocated" pkeys are those that have been returned
-* from pkey_alloc().  pkey 0 is special, and never
-* returned from pkey_alloc().
+* from pkey_alloc() or pkey 0 which is allocated
+* implicitly when the mm is created.
 */
-   if (pkey <= 0)
+   if (pkey < 0)
return false;
if (pkey >= arch_max_pkey())
return false;
_

[PATCH 1/3] x86, pkeys: do not special case protection key 0

[Dave Hansen]

From: Dave Hansen 

mm_pkey_is_allocated() treats pkey 0 as unallocated.  That is
inconsistent with the manpages, and also inconsistent with
mm->context.pkey_allocation_map.  Stop special casing it and only
disallow values that are actually bad (< 0).

The end-user visible effect of this is that you can now use
mprotect_pkey() to set pkey=0.

This is a bit nicer than what Ram proposed because it is simpler
and removes special-casing for pkey 0.  On the other hand, it does
allow applciations to pkey_free() pkey-0, but that's just a silly
thing to do, so we are not going to protect against it.

Signed-off-by: Dave Hansen 
Cc: Ram Pai 
Cc: Thomas Gleixner 
Cc: Dave Hansen 
Cc: Michael Ellermen 
Cc: Ingo Molnar 
Cc: Andrew Morton p
Cc: Shuah Khan 
---

 b/arch/x86/include/asm/mmu_context.h |2 +-
 b/arch/x86/include/asm/pkeys.h   |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff -puN arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated 
arch/x86/include/asm/mmu_context.h
--- a/arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated   
2018-03-16 14:46:39.023285476 -0700
+++ b/arch/x86/include/asm/mmu_context.h2018-03-16 14:46:39.028285476 
-0700
@@ -191,7 +191,7 @@ static inline int init_new_context(struc
 
 #ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
-   /* pkey 0 is the default and always allocated */
+   /* pkey 0 is the default and allocated implicitly */
mm->context.pkey_allocation_map = 0x1;
/* -1 means unallocated or invalid */
mm->context.execute_only_pkey = -1;
diff -puN arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 
arch/x86/include/asm/pkeys.h
--- a/arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated 2018-03-16 
14:46:39.025285476 -0700
+++ b/arch/x86/include/asm/pkeys.h  2018-03-16 14:46:39.028285476 -0700
@@ -49,10 +49,10 @@ bool mm_pkey_is_allocated(struct mm_stru
 {
/*
 * "Allocated" pkeys are those that have been returned
-* from pkey_alloc().  pkey 0 is special, and never
-* returned from pkey_alloc().
+* from pkey_alloc() or pkey 0 which is allocated
+* implicitly when the mm is created.
 */
-   if (pkey <= 0)
+   if (pkey < 0)
return false;
if (pkey >= arch_max_pkey())
return false;
_