[PATCH] x86/mm/cpa: Flush direct map alias during cpa

* [PATCH] x86/mm/cpa: Flush direct map alias during cpa
@ 2020-04-23  3:13 Rick Edgecombe
  2020-04-23  8:41 ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Rick Edgecombe @ 2020-04-23  3:13 UTC (permalink / raw)
  To: dave.hansen, luto, peterz, tglx, x86, mingo, bp, hpa, linux-kernel
  Cc: Rick Edgecombe

Change cpa_flush() to always flush_tlb_all(), as it previously did. As an
optimization, cpa_flush() was changed to optionally only flush the range
in "struct cpa_data *data" if it was small enough. However, this range
does not include any direct map aliases changed in cpa_process_alias().
So small set_memory_() calls that touch that alias don't get the direct
map changes flushed. This situation can happen when the virtual address
taking variants are passed an address in vmalloc or modules space.

The issue was verified by creating a sequence like:
/* Allocate something in vmalloc */
void *ptr = vmalloc();
/* Set vmalloc addr and direct map alias as not present */
set_memory_np((unsigned long)ptr, num_pages);
/* Try to read from direct map alias */
printk("%d\n", *(int*)page_address(vmalloc_to_page(ptr));

Which successfully read from the direct mapped page now set as not
present.

There is usually some flushing that happens before the PTE is set in this
operation, which covers up that the alias doesn't actually get flushed
after it's changed. So to reproduce, set_memory_np() was also tweaked to
touch the address right before it clears its PTE. This loads the old value
in the TLB to simulate a state that could happen in real life for a number
of reasons.

Note this issue does not extend to cases where the set_memory_() calls are
passed a direct map address, or page array, etc, as the primary target. In
those cases the direct map would be flushed.

Fixes: 935f583 ("x86/mm/cpa: Optimize cpa_flush_array() TLB invalidation")
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---

Besides the flushing that happens in most cases before the PTE is changed,
the other thing that covers this up is that stale TLB hits around RO->RW
CPA's would be silently fixed by the spurious fault fixer. It looks like
some of the set_memory_uc() calls can operate on vmapped memory addresses
though. So this would miss the flush of the UC attribute on the direct map.

So there isn't any confirmed bug, but it seems like we should be flushing
these, and there possibly is one around cache attributes.

 arch/x86/mm/pat/set_memory.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index c4aedd00c1ba..9b6d2854b842 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -331,15 +331,6 @@ static void cpa_flush_all(unsigned long cache)
 	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
 }
 
-static void __cpa_flush_tlb(void *data)
-{
-	struct cpa_data *cpa = data;
-	unsigned int i;
-
-	for (i = 0; i < cpa->numpages; i++)
-		__flush_tlb_one_kernel(fix_addr(__cpa_addr(cpa, i)));
-}
-
 static void cpa_flush(struct cpa_data *data, int cache)
 {
 	struct cpa_data *cpa = data;
@@ -352,10 +343,7 @@ static void cpa_flush(struct cpa_data *data, int cache)
 		return;
 	}
 
-	if (cpa->numpages <= tlb_single_page_flush_ceiling)
-		on_each_cpu(__cpa_flush_tlb, cpa, 1);
-	else
-		flush_tlb_all();
+	flush_tlb_all();
 
 	if (!cache)
 		return;
-- 
2.20.1


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