linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] x86/mm/pkeys: fix user-visible pkey state destruction at fork()
@ 2019-01-02 21:56 Dave Hansen
  2019-01-02 21:56 ` [PATCH 1/2] x86/pkeys: properly copy pkey state " Dave Hansen
  2019-01-02 21:56 ` [PATCH 2/2] x86/selftests/pkeys: fork() to check for state being preserved Dave Hansen
  0 siblings, 2 replies; 9+ messages in thread
From: Dave Hansen @ 2019-01-02 21:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dave Hansen, x86, tglx, mingo, bp, hpa, peterz, mpe, will.deacon,
	luto, jroedel, stable

Hi x86 maintainers,

This is an important fix that I believe needs to be merged for 4.21.
Without it, applications calling fork() can potentially double-allocate
a protection key, causing lots of strange problems.

Thomas's Reviewed-by is on the the actual fix, but not the selftest.

I would also be happy to send this as a pull request if you would
prefer.

Cc: x86@kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: stable@vger.kernel.org

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 1/2] x86/pkeys: copy pkey state at fork()
@ 2018-10-26 19:51 Dave Hansen
  2018-10-26 19:51 ` [PATCH 2/2] x86/selftests/pkeys: fork() to check for state being preserved Dave Hansen
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Hansen @ 2018-10-26 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dave Hansen, tglx, mingo, bp, hpa, x86, peterz, mpe, will.deacon,
	luto, jroedel


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

Our 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, we need to preserve two bits of state across a fork:
'execute_only_pkey' and 'pkey_allocation_map'.  Those are preserved by
the memcpy(), which I thought did the right thing.  But, mm_init()
calls init_new_context(), which I thought was *only* for execve()-time
and overwrites 'execute_only_pkey' and 'pkey_allocation_map' with
"new" values.

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.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Joerg Roedel <jroedel@suse.de>
---

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

diff -puN arch/x86/include/asm/mmu_context.h~x86-pkeys-no-init-at-fork arch/x86/include/asm/mmu_context.h
--- a/arch/x86/include/asm/mmu_context.h~x86-pkeys-no-init-at-fork	2018-10-26 09:24:09.444102622 -0700
+++ b/arch/x86/include/asm/mmu_context.h	2018-10-26 09:24:09.451102622 -0700
@@ -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);
 }
_

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-15  9:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-02 21:56 [PATCH 0/2] x86/mm/pkeys: fix user-visible pkey state destruction at fork() Dave Hansen
2019-01-02 21:56 ` [PATCH 1/2] x86/pkeys: properly copy pkey state " Dave Hansen
     [not found]   ` <20190103135224.1BC7A21479@mail.kernel.org>
2019-01-03 18:54     ` Dave Hansen
2019-01-15  9:36   ` [tip:x86/urgent] x86/pkeys: Properly " tip-bot for Dave Hansen
2019-01-02 21:56 ` [PATCH 2/2] x86/selftests/pkeys: fork() to check for state being preserved Dave Hansen
     [not found]   ` <20190103135225.4929F217D9@mail.kernel.org>
2019-01-03 18:57     ` Dave Hansen
2019-01-10 19:16       ` Greg KH
2019-01-15  9:37   ` [tip:x86/urgent] x86/selftests/pkeys: Fork() " tip-bot for Dave Hansen
  -- strict thread matches above, loose matches on Subject: below --
2018-10-26 19:51 [PATCH 1/2] x86/pkeys: copy pkey state at fork() Dave Hansen
2018-10-26 19:51 ` [PATCH 2/2] x86/selftests/pkeys: fork() to check for state being preserved Dave Hansen

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