diff mbox series

x86/tlb: Flush global mappings when KAISER is disabled

Message ID 20210325200942.GJ31322@zn.tnic (mailing list archive)
State New, archived
Headers show
Series x86/tlb: Flush global mappings when KAISER is disabled | expand

Commit Message

Borislav Petkov March 25, 2021, 8:09 p.m. UTC 
Hi stable folks,

the patch below fixes kernels 4.4 and 4.9 booting on AMD platforms with
PCID support. It doesn't have an upstream counterpart because it patches
the KAISER code which didn't go upstream. It applies fine to both of the
aforementioned kernels - please pick it up.

Thx.

---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 25 Mar 2021 11:02:31 +0100
Subject: [PATCH] x86/tlb: Flush global mappings when KAISER is disabled

Jim Mattson reported that Debian 9 guests using a 4.9-stable kernel
are exploding during alternatives patching:

  kernel BUG at /build/linux-dqnRSc/linux-4.9.228/arch/x86/kernel/alternative.c:709!
  invalid opcode: 0000 [#1] SMP
  Modules linked in:
  CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.9.0-13-amd64 #1 Debian 4.9.228-1
  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
  Call Trace:
   swap_entry_free
   swap_entry_free
   text_poke_bp
   swap_entry_free
   arch_jump_label_transform
   set_debug_rodata
   __jump_label_update
   static_key_slow_inc
   frontswap_register_ops
   init_zswap
   init_frontswap
   do_one_initcall
   set_debug_rodata
   kernel_init_freeable
   rest_init
   kernel_init
   ret_from_fork

triggering the BUG_ON in text_poke() which verifies whether patched
instruction bytes have actually landed at the destination.

Further debugging showed that the TLB flush before that check is
insufficient because there could be global mappings left in the TLB,
leading to a stale mapping getting used.

I say "global mappings" because the hardware configuration is a new one:
machine is an AMD, which means, KAISER/PTI doesn't need to be enabled
there, which also means there's no user/kernel pagetables split and
therefore the TLB can have global mappings.

And the configuration is new one for a second reason: because that AMD
machine supports PCID and INVPCID, which leads the CPU detection code to
set the synthetic X86_FEATURE_INVPCID_SINGLE flag.

Now, __native_flush_tlb_single() does invalidate global mappings when
X86_FEATURE_INVPCID_SINGLE is *not* set and returns.

When X86_FEATURE_INVPCID_SINGLE is set, however, it invalidates the
requested address from both PCIDs in the KAISER-enabled case. But if
KAISER is not enabled and the machine has global mappings in the TLB,
then those global mappings do not get invalidated, which would lead to
the above mismatch from using a stale TLB entry.

So make sure to flush those global mappings in the KAISER disabled case.

Co-debugged by Babu Moger <babu.moger@amd.com>.

Reported-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Hugh Dickins <hughd@google.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Babu Moger <babu.moger@amd.com>
Tested-by: Jim Mattson <jmattson@google.com>
Link: https://lkml.kernel.org/r/CALMp9eRDSW66%2BXvbHVF4ohL7XhThoPoT0BrB0TcS0cgk=dkcBg@mail.gmail.com
---
 arch/x86/include/asm/tlbflush.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Sasha Levin March 25, 2021, 8:36 p.m. UTC | #1 
On Thu, Mar 25, 2021 at 09:09:42PM +0100, Borislav Petkov wrote:
>Hi stable folks,
>
>the patch below fixes kernels 4.4 and 4.9 booting on AMD platforms with
>PCID support. It doesn't have an upstream counterpart because it patches
>the KAISER code which didn't go upstream. It applies fine to both of the
>aforementioned kernels - please pick it up.

Queued up for 4.9 and 4.4, thanks!

>Jim Mattson reported that Debian 9 guests using a 4.9-stable kernel
>are exploding during alternatives patching:

(Cc Ben & Salvatore)

I'm not sure if 4.9 or Debian is still alive or not, but FYI...
Sasha Levin March 25, 2021, 11:19 p.m. UTC | #2 
On Thu, Mar 25, 2021 at 04:36:55PM -0400, Sasha Levin wrote:
>On Thu, Mar 25, 2021 at 09:09:42PM +0100, Borislav Petkov wrote:
>>Hi stable folks,
>>
>>the patch below fixes kernels 4.4 and 4.9 booting on AMD platforms with
>>PCID support. It doesn't have an upstream counterpart because it patches
>>the KAISER code which didn't go upstream. It applies fine to both of the
>>aforementioned kernels - please pick it up.
>
>Queued up for 4.9 and 4.4, thanks!
>
>>Jim Mattson reported that Debian 9 guests using a 4.9-stable kernel
>>are exploding during alternatives patching:
>
>(Cc Ben & Salvatore)
>
>I'm not sure if 4.9 or Debian is still alive or not, but FYI...
		    *on

		    :)
Ben Hutchings March 25, 2021, 11:56 p.m. UTC | #3 
On Thu, 2021-03-25 at 19:19 -0400, Sasha Levin wrote:
> On Thu, Mar 25, 2021 at 04:36:55PM -0400, Sasha Levin wrote:
> > On Thu, Mar 25, 2021 at 09:09:42PM +0100, Borislav Petkov wrote:
> > > Hi stable folks,
> > > 
> > > the patch below fixes kernels 4.4 and 4.9 booting on AMD platforms with
> > > PCID support. It doesn't have an upstream counterpart because it patches
> > > the KAISER code which didn't go upstream. It applies fine to both of the
> > > aforementioned kernels - please pick it up.
> > 
> > Queued up for 4.9 and 4.4, thanks!
> > 
> > > Jim Mattson reported that Debian 9 guests using a 4.9-stable kernel
> > > are exploding during alternatives patching:
> > 
> > (Cc Ben & Salvatore)
> > 
> > I'm not sure if 4.9 or Debian is still alive or not, but FYI...
>                     *on
> 
>                     :)

We're supporting both 4.9 and 4.19 in Debian 9.  The general rule is we
carry on with the same stable kernel branch for the whole 5 year
support period, but add the option of using the kernel version from the
next stable release.

Ben.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index f5ca15622dc9..2bfa4deb8cae 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -245,12 +245,15 @@  static inline void __native_flush_tlb_single(unsigned long addr)
 	 * 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.
+	 * In the KAISER disabled case, do an INVLPG to make sure
+	 * the mapping is flushed in case it is a global one.
 	 */
-	if (kaiser_enabled)
+	if (kaiser_enabled) {
 		invpcid_flush_one(X86_CR3_PCID_ASID_USER, addr);
-	invpcid_flush_one(X86_CR3_PCID_ASID_KERN, addr);
+		invpcid_flush_one(X86_CR3_PCID_ASID_KERN, addr);
+	} else {
+		asm volatile("invlpg (%0)" ::"r" (addr) : "memory");
+	}
 }
 
 static inline void __flush_tlb_all(void)