linux-kernel - Re: [PATCH] x86/mm/kaiser: Flush the correct ASID in __native_flush_tlb

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 29 Nov 2017 15:35:26 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        tglx@...utronix.de, richard.fellner@...dent.tugraz.at,
        moritz.lipp@...k.tugraz.at, daniel.gruss@...k.tugraz.at,
        michael.schwarz@...k.tugraz.at, luto@...nel.org,
        torvalds@...ux-foundation.org, keescook@...gle.com,
        hughd@...gle.com, bp@...en8.de, x86@...nel.org
Subject: Re: [PATCH] x86/mm/kaiser: Flush the correct ASID in
 __native_flush_tlb_single()

On Tue, Nov 28, 2017 at 01:55:31AM -0800, Dave Hansen wrote:
>  static inline void __native_flush_tlb_single(unsigned long addr)
>  {
> +	u16 loaded_mm_asid = this_cpu_read(cpu_tlbstate.loaded_mm_asid);
>  
>  	/*
> +	 * Handle systems that do not support PCIDs.  This will also
> +	 * get used in cases where this is called before PCID detection
> +	 * is done.
>  	 */
>  	if (!this_cpu_has(X86_FEATURE_INVPCID_SINGLE)) {
> +		__invlpg(addr);
>  		return;
>  	}
> +
> +	/*
> +	 * An "invalid" loaded_mm_asid means that we have not
> +	 * initialized 'cpu_tlbstate' and are not using PCIDs.
> +	 * Just flush the TLB as if PCIDs were not present.
> +	 */
> +	if (loaded_mm_asid == INVALID_HW_ASID) {
> +		__invlpg(addr);
> +		return;
> +	}
> +
>  	/* Flush the address out of both PCIDs. */
>  	/*
>  	 * An optimization here might be to determine addresses
> @@ -451,6 +474,9 @@ static inline void __native_flush_tlb_si
>  	if (kern_asid(loaded_mm_asid) != user_asid(loaded_mm_asid))
>  		invpcid_flush_one(user_asid(loaded_mm_asid), addr);
>  	invpcid_flush_one(kern_asid(loaded_mm_asid), addr);
> +
> +	/* Check that we are flushing the active ASID: */
> +	VM_WARN_ON_ONCE(kern_asid(loaded_mm_asid) != cr3_asid());
>  }

Can't we do this differently (after my recent patches)? It appears to me
we can unconditionally do INVLPG to shoot down the kernel mapping, and
then, depending on INVPCID support we can either use that to shoot down
a single page or simply invalidate the entire user mapping.

---
 arch/x86/include/asm/tlbflush.h | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 481d5094559e..9587722162ee 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -438,29 +438,20 @@ static inline void __native_flush_tlb_single(unsigned long addr)
 {
 	u32 loaded_mm_asid = this_cpu_read(cpu_tlbstate.loaded_mm_asid);
 
+	asm volatile("invlpg (%0)" ::"r" (addr) : "memory");
+
+	if (!kaiser_enabled)
+		return;
+
 	/*
 	 * Some platforms #GP if we call invpcid(type=1/2) before
 	 * CR4.PCIDE=1.  Just call invpcid in the case we are called
 	 * early.
 	 */
-	if (!this_cpu_has(X86_FEATURE_INVPCID_SINGLE)) {
+	if (!this_cpu_has(X86_FEATURE_INVPCID_SINGLE))
 		flush_user_asid(loaded_mm_asid);
-		asm volatile("invlpg (%0)" ::"r" (addr) : "memory");
-		return;
-	}
-	/* Flush the address out of both PCIDs. */
-	/*
-	 * An optimization here might be to determine addresses
-	 * that are only kernel-mapped and only flush the kernel
-	 * ASID.  But, userspace flushes are probably much more
-	 * important performance-wise.
-	 *
-	 * Make sure to do only a single invpcid when KAISER is
-	 * disabled and we have only a single ASID.
-	 */
-	if (kern_asid(loaded_mm_asid) != user_asid(loaded_mm_asid))
+	else
 		invpcid_flush_one(user_asid(loaded_mm_asid), addr);
-	invpcid_flush_one(kern_asid(loaded_mm_asid), addr);
 }
 
 static inline void __flush_tlb_all(void)