[PATCH] x86/mm: Set NX bit when making pages present

* [PATCH] x86/mm: Set NX bit when making pages present
@ 2022-09-09 15:27 Dave Hansen
  2022-09-16  3:59 ` Yujie Liu
  2022-09-19 18:14 ` Edgecombe, Rick P
  0 siblings, 2 replies; 5+ messages in thread
From: Dave Hansen @ 2022-09-09 15:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dave Hansen, kernel test robot, Peter Zijlstra, Andy Lutomirski,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin

The x86 mm code now actively refuses to create writable, executable
mappings and warns when there is an attempt to create one.

0day ran across a case triggered by module unloading, but that looks
to be a generic problem.  It presumably goes like this:

	1. Load module with direct map, P=1,W=1,NX=1
	2. Map module executable, set P=1,W=0,NX=0
	3. Free module, land in vfree()->vm_remove_mappings()
	4. Set P=0 during alias processing, P=0,W=0,NX=0
	5. Restore kernel mapping via set_direct_map_default_noflush(),
	   set P=1,W=1, resulting in P=1,W=1,NX=0

That's clearly a writable, executable mapping which is a no-no.  The
new W^X code is clearly doing its job.

Fix it by actively setting _PAGE_NX when creating writable mappings.

One concern: I haven't been able to actually reproduce this, even by
loading and unloading the module that 0day hit it with.  I'd like to
be able to reproduce this before committing a fix.

Reported-by: kernel test robot <yujie.liu@intel.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Link: https://lore.kernel.org/all/fcf89147-440b-e478-40c9-228c9fe56691@intel.com/

--

0day folks, please do share these as they come up.  We want to keep
fixing them.
---
 arch/x86/mm/pat/set_memory.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 1a2d6376251c..5fb5874ea2c6 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2247,6 +2247,12 @@ static int __set_pages_p(struct page *page, int numpages)
 				.mask_clr = __pgprot(0),
 				.flags = 0};
 
+	/*
+	 * Avoid W^X mappings that occur if the old
+	 * mapping was !_PAGE_RW and !_PAGE_NX.
+	 */
+	pgprot_val(cpa.mask_set) |= __supported_pte_mask & _PAGE_NX;
+
 	/*
 	 * No alias checking needed for setting present flag. otherwise,
 	 * we may need to break large pages for 64-bit kernel text
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread