diff mbox

pkeys on POWER: Access rights not reset on execve

Message ID 20180611200807.GA5773@ram.oc3035372033.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ram Pai June 11, 2018, 8:08 p.m. UTC 
On Mon, Jun 11, 2018 at 07:29:33PM +0200, Florian Weimer wrote:
> On 06/11/2018 07:23 PM, Ram Pai wrote:
> >On Fri, Jun 08, 2018 at 07:53:51AM +0200, Florian Weimer wrote:
> >>On 06/08/2018 04:34 AM, Ram Pai wrote:
> >>>>
> >>>>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'
> 
> >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 <linuxram@us.ibm.com>
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 <linuxram@us.ibm.com>

Comments

Florian Weimer June 12, 2018, 12:17 p.m. UTC | #1 
On 06/11/2018 10:08 PM, Ram Pai wrote:
> 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<linuxram@us.ibm.com>
> Date:   Mon Jun 11 14:57:34 2018 -0500
> 
>      powerpc/pkeys: Deny read/write/execute by default

With this patch, my existing misc/tst-pkey test in glibc passes.  The 
in-tree version still has some incorrect assumptions on implementation 
behavior, but those are test bugs.  The kernel behavior with your patch 
look good to me.  Thanks.

Florian
diff mbox

Patch

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));