4.20-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Dave Hansen <dave.han...@linux.intel.com>

commit a31e184e4f69965c99c04cc5eb8a4920e0c63737 upstream.

Memory protection key behavior should be the same in a child as it was
in the parent before a fork.  But, there is a bug that resets the
state in the child at fork instead of preserving it.

The creation of new mm's is a bit convoluted.  At fork(), the code
does:

  1. memcpy() the parent mm to initialize child
  2. mm_init() to initalize some select stuff stuff
  3. dup_mmap() to create true copies that memcpy() did not do right

For pkeys two bits of state need to be preserved across a fork:
'execute_only_pkey' and 'pkey_allocation_map'.

Those are preserved by the memcpy(), but mm_init() invokes
init_new_context() which overwrites 'execute_only_pkey' and
'pkey_allocation_map' with "new" values.

The author of the code erroneously believed that init_new_context is *only*
called at execve()-time.  But, alas, init_new_context() is used at execve()
and fork().

The result is that, after a fork(), the child's pkey state ends up looking
like it does after an execve(), which is totally wrong.  pkeys that are
already allocated can be allocated again, for instance.

To fix this, add code called by dup_mmap() to copy the pkey state from
parent to child explicitly.  Also add a comment above init_new_context() to
make it more clear to the next poor sod what this code is used for.

Fixes: e8c24d3a23a ("x86/pkeys: Allocation/free syscalls")
Signed-off-by: Dave Hansen <dave.han...@linux.intel.com>
Signed-off-by: Thomas Gleixner <t...@linutronix.de>
Reviewed-by: Thomas Gleixner <t...@linutronix.de>
Cc: b...@alien8.de
Cc: h...@zytor.com
Cc: pet...@infradead.org
Cc: m...@ellerman.id.au
Cc: will.dea...@arm.com
Cc: l...@kernel.org
Cc: jroe...@suse.de
Cc: sta...@vger.kernel.org
Cc: Borislav Petkov <b...@alien8.de>
Cc: "H. Peter Anvin" <h...@zytor.com>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Will Deacon <will.dea...@arm.com>
Cc: Andy Lutomirski <l...@kernel.org>
Cc: Joerg Roedel <jroe...@suse.de>
Link: https://lkml.kernel.org/r/20190102215655.7a695...@viggo.jf.intel.com
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
 arch/x86/include/asm/mmu_context.h |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -178,6 +178,10 @@ static inline void switch_ldt(struct mm_
 
 void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk);
 
+/*
+ * Init a new mm.  Used on mm copies, like at fork()
+ * and on mm's that are brand-new, like at execve().
+ */
 static inline int init_new_context(struct task_struct *tsk,
                                   struct mm_struct *mm)
 {
@@ -228,8 +232,22 @@ do {                                               \
 } while (0)
 #endif
 
+static inline void arch_dup_pkeys(struct mm_struct *oldmm,
+                                 struct mm_struct *mm)
+{
+#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
+       if (!cpu_feature_enabled(X86_FEATURE_OSPKE))
+               return;
+
+       /* Duplicate the oldmm pkey state in mm: */
+       mm->context.pkey_allocation_map = oldmm->context.pkey_allocation_map;
+       mm->context.execute_only_pkey   = oldmm->context.execute_only_pkey;
+#endif
+}
+
 static inline int arch_dup_mmap(struct mm_struct *oldmm, struct mm_struct *mm)
 {
+       arch_dup_pkeys(oldmm, mm);
        paravirt_arch_dup_mmap(oldmm, mm);
        return ldt_dup_context(oldmm, mm);
 }


Reply via email to