linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Ram Pai <linuxram@us.ibm.com>
To: mpe@ellerman.id.au
Cc: linuxppc-dev@lists.ozlabs.org, hbabu@us.ibm.com,
	mhocko@kernel.org, bauerman@linux.vnet.ibm.com,
	linuxram@us.ibm.com, Ulrich.Weigand@de.ibm.com,
	fweimer@redhat.com, msuchanek@suse.de
Subject: [PATCH 2/2] powerpc/pkeys: key allocation/deallocation must not change pkey registers
Date: Mon, 25 Jun 2018 19:16:14 -0700	[thread overview]
Message-ID: <1529979376-7292-2-git-send-email-linuxram@us.ibm.com> (raw)
In-Reply-To: <1529979376-7292-1-git-send-email-linuxram@us.ibm.com>

Key allocation and deallocation has the side effect of programming the
UAMOR/AMR/IAMR registers. This is wrong, since its the responsibility of
the application and not that of the kernel, to modify the permission on
the key.

Do not modify the pkey registers at key allocation/deallocation.

This patch also fixes a bug where a sys_pkey_free() resets the UAMOR
bits of the key, thus making its permissions unmodifiable from user
space.  Latter if the same key gets reallocated from a different thread
this thread will no longer be able to change the permissions on the key.

Problem noticed/reported by Michael Ellermen while running
selftests/core-pkeys

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/pkeys.h |   11 -----------
 arch/powerpc/mm/pkeys.c          |   27 ---------------------------
 2 files changed, 0 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index c824528..92a9962 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -101,8 +101,6 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
 	return __mm_pkey_is_allocated(mm, pkey);
 }
 
-extern void __arch_activate_pkey(int pkey);
-extern void __arch_deactivate_pkey(int pkey);
 /*
  * Returns a positive, 5-bit key on success, or -1 on failure.
  * Relies on the mmap_sem to protect against concurrency in mm_pkey_alloc() and
@@ -131,11 +129,6 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
 	ret = ffz((u32)mm_pkey_allocation_map(mm));
 	__mm_pkey_allocated(mm, ret);
 
-	/*
-	 * Enable the key in the hardware
-	 */
-	if (ret > 0)
-		__arch_activate_pkey(ret);
 	return ret;
 }
 
@@ -147,10 +140,6 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
 	if (!mm_pkey_is_allocated(mm, pkey))
 		return -EINVAL;
 
-	/*
-	 * Disable the key in the hardware
-	 */
-	__arch_deactivate_pkey(pkey);
 	__mm_pkey_free(mm, pkey);
 
 	return 0;
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index 0b03914..27ac7f0 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -224,33 +224,6 @@ static inline void init_iamr(int pkey, u8 init_bits)
 	write_iamr(old_iamr | new_iamr_bits);
 }
 
-static void pkey_status_change(int pkey, bool enable)
-{
-	u64 old_uamor;
-
-	/* Reset the AMR and IAMR bits for this key */
-	init_amr(pkey, 0x0);
-	init_iamr(pkey, 0x0);
-
-	/* Enable/disable key */
-	old_uamor = read_uamor();
-	if (enable)
-		old_uamor |= (0x3ul << pkeyshift(pkey));
-	else
-		old_uamor &= ~(0x3ul << pkeyshift(pkey));
-	write_uamor(old_uamor);
-}
-
-void __arch_activate_pkey(int pkey)
-{
-	pkey_status_change(pkey, true);
-}
-
-void __arch_deactivate_pkey(int pkey)
-{
-	pkey_status_change(pkey, false);
-}
-
 /*
  * Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
  * specified in @init_val.
-- 
1.7.1

  reply	other threads:[~2018-06-26  2:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-26  2:16 [PATCH 1/2] powerpc/pkeys: preallocate execute_only key only if the key is available Ram Pai
2018-06-26  2:16 ` Ram Pai [this message]
2018-07-03  1:35   ` [PATCH 2/2] powerpc/pkeys: key allocation/deallocation must not change pkey registers Thiago Jung Bauermann
2018-06-26  2:16 ` [PATCH 1/2] powerpc/core-pkeys: execute-permission on keys are disabled by default Ram Pai
2018-07-03  3:30   ` Thiago Jung Bauermann
2018-06-26  2:16 ` [PATCH 2/2] powerpc/ptrace-pkeys: " Ram Pai
2018-07-03  3:30   ` Thiago Jung Bauermann
2018-06-29  2:56 ` [PATCH 1/2] powerpc/pkeys: preallocate execute_only key only if the key is available Thiago Jung Bauermann
2018-06-29  6:07   ` Gabriel Paubert
2018-06-30  0:58     ` Thiago Jung Bauermann
2018-06-30  1:40       ` Ram Pai
2018-06-30 16:56       ` Gabriel Paubert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1529979376-7292-2-git-send-email-linuxram@us.ibm.com \
    --to=linuxram@us.ibm.com \
    --cc=Ulrich.Weigand@de.ibm.com \
    --cc=bauerman@linux.vnet.ibm.com \
    --cc=fweimer@redhat.com \
    --cc=hbabu@us.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mhocko@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=msuchanek@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).