Re: pkeys on POWER: Access rights not reset on execve

On Mon, Jun 11, 2018 at 07:29:33PM +0200, Florian Weimer wrote:

On 06/11/2018 07:23 PM, Ram Pai wrote:

quoted

On Fri, Jun 08, 2018 at 07:53:51AM +0200, Florian Weimer wrote:

quoted

On 06/08/2018 04:34 AM, Ram Pai wrote:

quoted

quoted

So the remaining question at this point is whether the Intel
behavior (default-deny instead of default-allow) is preferable.

Florian, remind me what behavior needs to fixed?

See the other thread.  The Intel register equivalent to the AMR by
default disallows access to yet-unallocated keys, so that threads
which are created before key allocation do not magically gain access
to a key allocated by another thread.

Are you referring to the thread
'[PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics'

quoted

Otherwise please point me to the URL of that thread. Sorry and thankx. :)

No, it's this issue:

  ...

Ok. try this patch. This patch is on top of the 5 patches that I had
sent last week i.e  "[PATCH  0/5] powerpc/pkeys: fixes to pkeys"

The following is a draft patch though to check if it meets your
expectations.

commit fe53b5fe2dcb3139ea27ade3ae7cbbe43c4af3be
Author: Ram Pai [off-list ref]
Date:   Mon Jun 11 14:57:34 2018 -0500

    powerpc/pkeys: Deny read/write/execute by default
    
    Deny everything for all keys; with some exceptions. Do not do this for
    pkey-0, or else everything will come to a screaching halt.  Also by
    default, do not deny execute for execute-only key.
    
    This is a draft-patch for now.
    
    Signed-off-by: Ram Pai [off-list ref]

diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 8225263..289aafd 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c

@@ -128,13 +128,13 @@ int pkey_initialize(void)
 
 	/* register mask is in BE format */
 	pkey_amr_mask = ~0x0ul;
-	pkey_iamr_mask = ~0x0ul;
+	pkey_amr_mask &= ~(0x3ul << pkeyshift(PKEY_0));
+	pkey_amr_mask &= ~(0x3ul << pkeyshift(1));
 
-	for (i = 0; i < (pkeys_total - os_reserved); i++) {
-		pkey_amr_mask &= ~(0x3ul << pkeyshift(i));
-		pkey_iamr_mask &= ~(0x1ul << pkeyshift(i));
-	}
-	pkey_amr_mask |= (AMR_RD_BIT|AMR_WR_BIT) << pkeyshift(EXECUTE_ONLY_KEY);
+	pkey_iamr_mask = ~0x0ul;
+	pkey_iamr_mask &= ~(0x3ul << pkeyshift(PKEY_0));
+	pkey_iamr_mask &= ~(0x3ul << pkeyshift(1));
+	pkey_iamr_mask &= ~(0x3ul << pkeyshift(EXECUTE_ONLY_KEY));
 
 	pkey_uamor_mask = ~0x0ul;
 	pkey_uamor_mask &= ~(0x3ul << pkeyshift(PKEY_0));

-- 
Ram Pai