LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: ying.huang@intel.com, tglx@linutronix.de, mingo@elte.hu,
	linux-kernel@vger.kernel.org
Subject: [PATCH] [4/8] CPA: Fix set_memory_x for ioremap
Date: Mon, 11 Feb 2008 10:34:32 +0100 (CET)	[thread overview]
Message-ID: <20080211093432.DA4331B41CE@basil.firstfloor.org> (raw)
In-Reply-To: <200802111034.764275766@suse.de>


EFI currently calls set_memory_x() on potentially ioremapped addresses.

This is problematic for several reasons: 

- The cpa code internally calls __pa on it which does not work for remapped
addresses and will give some random result.
- cpa will try to change all potential aliases (like the kernel mapping
on x86-64), but that is not needed for NX because the caller does only needs its 
specific virtual address executable. There is no requirement in the x86 architecture 
for nx bits to be coherent between mapping aliases. Also with the
previous problem of __pa returning a wrong address it would likely try to change 
some random other page if you're unlucky and the random result would
match the kernel text range.

There would be several possible ways to fix this:
- Simply don't set the NX bit in the original ioremap and drop
set_memory_x and add a ioremap_exec(). That would be my preferred solution, 
but unfortunately has been dismissed before
- Drop all __pas and always use the physical address derived 
from the looked up PTE. This would need some significant restructuring
and would only fix the first problem above, not the second.
- Special case NX clear to change any aliases. I chose this one
because it happens to fix both problems, so is both a fix
and a optimization.

This implies that it's still not safe calling set_memory_(not x) on
any ioremaped/vmalloced/module addresses. 

I don't have a EFI system so this is untested.

Cc: ying.huang@intel.com

Signed-off-by: Andi Kleen <ak@suse.de>

---
 arch/x86/mm/pageattr.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: linux/arch/x86/mm/pageattr.c
===================================================================
--- linux.orig/arch/x86/mm/pageattr.c
+++ linux/arch/x86/mm/pageattr.c
@@ -28,6 +28,7 @@ struct cpa_data {
 	pgprot_t	mask_clr;
 	int		numpages;
 	int		flushtlb;
+	int		addronly;
 };
 
 static inline int
@@ -602,7 +603,7 @@ static int change_page_attr_addr(struct 
 	 * fixup the low mapping first. __va() returns the virtual
 	 * address in the linear mapping:
 	 */
-	if (within(address, HIGH_MAP_START, HIGH_MAP_END))
+	if (within(address, HIGH_MAP_START, HIGH_MAP_END) && !cpa->addronly)
 		address = (unsigned long) __va(phys_addr);
 #endif
 
@@ -615,7 +616,7 @@ static int change_page_attr_addr(struct 
 	 * If the physical address is inside the kernel map, we need
 	 * to touch the high mapped kernel as well:
 	 */
-	if (within(phys_addr, 0, KERNEL_TEXT_SIZE)) {
+	if (!cpa->addronly && within(phys_addr, 0, KERNEL_TEXT_SIZE)) {
 		/*
 		 * Calc the high mapping address. See __phys_addr()
 		 * for the non obvious details.
@@ -695,6 +696,8 @@ static int change_page_attr_set_clr(unsi
 	cpa.mask_set = mask_set;
 	cpa.mask_clr = mask_clr;
 	cpa.flushtlb = 0;
+	cpa.addronly = !pgprot_val(mask_set) &&
+			pgprot_val(mask_clr) == _PAGE_NX;
 
 	ret = __change_page_attr_set_clr(&cpa);
 

  parent reply	other threads:[~2008-02-11  9:35 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-11  9:34 [PATCH] [0/8] Various kernel mapping bug fixes Andi Kleen
2008-02-11  9:34 ` [PATCH] [1/8] CPA: Fix gbpages support in try_preserve_lage_page Andi Kleen
2008-02-11  9:45   ` Thomas Gleixner
2008-02-11 10:12     ` Ingo Molnar
2008-02-11 11:01       ` Andi Kleen
2008-02-11  9:34 ` [PATCH] [2/8] CPA: Flush the caches when setting pages not present Andi Kleen
2008-02-11 11:00   ` Ingo Molnar
2008-02-11 12:26     ` Andi Kleen
2008-02-11  9:34 ` [PATCH] [3/8] CPA: Test the correct mapping alias on x86-64 Andi Kleen
2008-02-11 11:49   ` Ingo Molnar
2008-02-11  9:34 ` Andi Kleen [this message]
2008-02-11 12:27   ` [PATCH] [4/8] CPA: Fix set_memory_x for ioremap Ingo Molnar
2008-02-11 12:45     ` Andi Kleen
2008-02-11  9:34 ` [PATCH] [5/8] Fix logic error in 64bit memory hotadd Andi Kleen
2008-02-11 12:46   ` Ingo Molnar
2008-02-11 13:05     ` Andi Kleen
2008-02-11 13:33       ` Ingo Molnar
2008-02-11 13:44         ` Andi Kleen
2008-02-12 10:35       ` Yasunori Goto
2008-02-12 11:20         ` Andi Kleen
2008-02-11  9:34 ` [PATCH] [6/8] Account overlapped mappings in end_pfn_map Andi Kleen
2008-02-11 13:08   ` Ingo Molnar
2008-02-11 13:27     ` Andi Kleen
2008-02-11 13:55       ` Ingo Molnar
2008-02-11 14:16       ` Peter Zijlstra
2008-02-11 14:24         ` Andi Kleen
2008-02-11 14:41           ` Sam Ravnborg
2008-02-11 15:12   ` Arjan van de Ven
2008-02-11  9:34 ` [PATCH] [7/8] Implement true end_pfn_mapped for 32bit Andi Kleen
2008-02-12 19:39   ` Thomas Gleixner
2008-02-12 19:49     ` Andi Kleen
2008-02-12 20:25       ` Thomas Gleixner
2008-02-11  9:34 ` [PATCH] [8/8] RFC: Fix some EFI problems Andi Kleen
2008-02-12 20:04   ` Thomas Gleixner
2008-02-12 20:23     ` Andi Kleen
2008-02-12 20:48       ` Thomas Gleixner
2008-02-13 11:05         ` Andi Kleen

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=20080211093432.DA4331B41CE@basil.firstfloor.org \
    --to=ak@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=ying.huang@intel.com \
    --subject='Re: [PATCH] [4/8] CPA: Fix set_memory_x for ioremap' \
    /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

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