linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
To: dave.hansen@linux.intel.com, luto@kernel.org,
	peterz@infradead.org, tglx@linutronix.de, x86@kernel.org,
	mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
	linux-kernel@vger.kernel.org
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Subject: [PATCH] x86/mm/cpa: Flush direct map alias during cpa
Date: Wed, 22 Apr 2020 20:13:55 -0700	[thread overview]
Message-ID: <20200423031355.23955-1-rick.p.edgecombe@intel.com> (raw)

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


             reply	other threads:[~2020-04-23  3:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23  3:13 Rick Edgecombe [this message]
2020-04-23  8:41 ` [PATCH] x86/mm/cpa: Flush direct map alias during cpa Peter Zijlstra
2020-04-23 19:02   ` Edgecombe, Rick P
2020-04-24 10:53     ` Peter Zijlstra
2020-05-01 18:22       ` [tip: x86/urgent] " tip-bot2 for Rick Edgecombe

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=20200423031355.23955-1-rick.p.edgecombe@intel.com \
    --to=rick.p.edgecombe@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).