linux-kernel - Re: [PATCH] x86/mm/cpa: Flush direct map alias during cpa

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200423084114.GS20713@hirez.programming.kicks-ass.net>
Date:   Thu, 23 Apr 2020 10:41:14 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Rick Edgecombe <rick.p.edgecombe@...el.com>
Cc:     dave.hansen@...ux.intel.com, luto@...nel.org, tglx@...utronix.de,
        x86@...nel.org, mingo@...hat.com, bp@...en8.de, hpa@...or.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/mm/cpa: Flush direct map alias during cpa

On Wed, Apr 22, 2020 at 08:13:55PM -0700, Rick Edgecombe wrote:
> 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.

Wouldn't something like so make more sense?

---
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index 59eca6a94ce7..b8c55a2e402d 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -43,7 +43,8 @@ struct cpa_data {
 	unsigned long	pfn;
 	unsigned int	flags;
 	unsigned int	force_split		: 1,
-			force_static_prot	: 1;
+			force_static_prot	: 1,
+			force_flush_all		: 1;
 	struct page	**pages;
 };
 
@@ -355,10 +356,10 @@ 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
+	if (cpa->force_flush_all || cpa->numpages > tlb_single_page_flush_ceiling)
 		flush_tlb_all();
+	else
+		on_each_cpu(__cpa_flush_tlb, cpa, 1);
 
 	if (!cache)
 		return;
@@ -1598,6 +1599,8 @@ static int cpa_process_alias(struct cpa_data *cpa)
 		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY);
 		alias_cpa.curpage = 0;
 
+		cpa->force_flush_all = 1;
+
 		ret = __change_page_attr_set_clr(&alias_cpa, 0);
 		if (ret)
 			return ret;
@@ -1618,6 +1621,7 @@ static int cpa_process_alias(struct cpa_data *cpa)
 		alias_cpa.flags &= ~(CPA_PAGES_ARRAY | CPA_ARRAY);
 		alias_cpa.curpage = 0;
 
+		cpa->force_flush_all = 1;
 		/*
 		 * The high mapping range is imprecise, so ignore the
 		 * return value.