[18/25] powerpc: check key protection for user page access

Message ID 1504910713-7094-27-git-send-email-linuxram@us.ibm.com
State Changes Requested
Headers show
Series
  • powerpc: Free up RPAGE_RSV bits
Related show

Commit Message

Ram Pai Sept. 8, 2017, 10:45 p.m.
Make sure that the kernel does not access user pages without
checking their key-protection.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

Comments

Balbir Singh Oct. 18, 2017, 7:57 p.m. | #1
On Fri,  8 Sep 2017 15:45:06 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> Make sure that the kernel does not access user pages without
> checking their key-protection.
>

Why? This makes the routines AMR/thread specific? Looks like
x86 does this as well, but these routines are used by GUP from
the kernel.

Balbir Singh.
Ram Pai Oct. 18, 2017, 9:29 p.m. | #2
On Thu, Oct 19, 2017 at 06:57:32AM +1100, Balbir Singh wrote:
> On Fri,  8 Sep 2017 15:45:06 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > Make sure that the kernel does not access user pages without
> > checking their key-protection.
> >
> 
> Why? This makes the routines AMR/thread specific? Looks like
> x86 does this as well

Yes. the memkey semantics implemented by x86, assumes that the keys and
their access-permission are per thread.  In other words, a key which is
enabled in the context of one thread, will not be enabled in the context
of another thread.

> but these routines are used by GUP from
> the kernel.

See a problem?

RP
Balbir Singh Oct. 18, 2017, 11:08 p.m. | #3
On Wed, 18 Oct 2017 14:29:24 -0700
Ram Pai <linuxram@us.ibm.com> wrote:

> On Thu, Oct 19, 2017 at 06:57:32AM +1100, Balbir Singh wrote:
> > On Fri,  8 Sep 2017 15:45:06 -0700
> > Ram Pai <linuxram@us.ibm.com> wrote:
> >   
> > > Make sure that the kernel does not access user pages without
> > > checking their key-protection.
> > >  
> > 
> > Why? This makes the routines AMR/thread specific? Looks like
> > x86 does this as well  
> 
> Yes. the memkey semantics implemented by x86, assumes that the keys and
> their access-permission are per thread.  In other words, a key which is
> enabled in the context of one thread, will not be enabled in the context
> of another thread.
> 
> > but these routines are used by GUP from
> > the kernel.  
> 
> See a problem?
>

No, I don't understand why gup (called from driver context, probably) should
worry about permissions and keys?

Balbir Singh.
Ram Pai Oct. 19, 2017, 4:46 p.m. | #4
On Thu, Oct 19, 2017 at 10:08:57AM +1100, Balbir Singh wrote:
> On Wed, 18 Oct 2017 14:29:24 -0700
> Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > On Thu, Oct 19, 2017 at 06:57:32AM +1100, Balbir Singh wrote:
> > > On Fri,  8 Sep 2017 15:45:06 -0700
> > > Ram Pai <linuxram@us.ibm.com> wrote:
> > >   
> > > > Make sure that the kernel does not access user pages without
> > > > checking their key-protection.
> > > >  
> > > 
> > > Why? This makes the routines AMR/thread specific? Looks like
> > > x86 does this as well  
> > 
> > Yes. the memkey semantics implemented by x86, assumes that the keys and
> > their access-permission are per thread.  In other words, a key which is
> > enabled in the context of one thread, will not be enabled in the context
> > of another thread.
> > 
> > > but these routines are used by GUP from
> > > the kernel.  
> > 
> > See a problem?
> >
> 
> No, I don't understand why gup (called from driver context, probably) should
> worry about permissions and keys?

There are some user level features; eg: pipe,  where the userspace
donates one of its pages to the kernel, to buffer the pipe stream.

But if the donated page has a non-permissive key associated, the
kernel should reject and return failure. Access to a page
associated with a non-permissive key should fail regardless of who
accesses the page (userspace, or kernel on userspace's behalf).

That is the reason we tap into the GUP routines to validate such
access.

RP

Patch

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index bd244b3..d22bb4d 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -494,6 +494,20 @@  static inline void write_uamor(u64 value)
 
 #ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS
 extern bool arch_pte_access_permitted(u64 pte, bool write, bool execute);
+
+#define pte_access_permitted(pte, write) \
+	(pte_present(pte) && \
+	 ((!(write) || pte_write(pte)) && \
+	  arch_pte_access_permitted(pte_val(pte), !!write, 0)))
+
+/*
+ * We store key in pmd for huge tlb pages. So need
+ * to check for key protection.
+ */
+#define pmd_access_permitted(pmd, write) \
+	(pmd_present(pmd) && \
+	 ((!(write) || pmd_write(pmd)) && \
+	  arch_pte_access_permitted(pmd_val(pmd), !!write, 0)))
 #endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */
 
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR