From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 26 Feb 2019 15:20:29 -0000 Received: from mx1.redhat.com ([209.132.183.28]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1gyeWx-00030f-VV for speck@linutronix.de; Tue, 26 Feb 2019 16:20:28 +0100 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9011AC0740F0 for ; Tue, 26 Feb 2019 15:20:20 +0000 (UTC) Received: from treble (ovpn-120-232.rdu2.redhat.com [10.10.120.232]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3FAEA600C0 for ; Tue, 26 Feb 2019 15:20:20 +0000 (UTC) Date: Tue, 26 Feb 2019 09:20:18 -0600 From: Josh Poimboeuf Subject: [MODERATED] Re: [patch V4 05/11] x86/speculation/mds: Clear CPU buffers on exit to user Message-ID: <20190226152018.aatzvnkq7vy3xdcs@treble> References: <20190222222418.405369026@linutronix.de> <20190222224149.527959429@linutronix.de> MIME-Version: 1.0 In-Reply-To: <20190222224149.527959429@linutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Fri, Feb 22, 2019 at 11:24:23PM +0100, speck for Thomas Gleixner wrote: > From: Thomas Gleixner > > Add a static key which controls the invocation of the CPU buffer clear > mechanism on exit to user space and add the call into > prepare_exit_to_usermode() and do_nmi() right before actually returning. > > Add documentation which kernel to user space transition this covers and > explain why some corner cases are not mitigated. > > Signed-off-by: Thomas Gleixner > --- > V3 --> V4: Add #DS mitigation and document that the #MC corner case > is really not interesting. > > V3: Add NMI conditional on user regs and update documentation accordingly. > Use the static branch scheme suggested by Peter. Fix typos ... > --- > Documentation/x86/mds.rst | 41 +++++++++++++++++++++++++++++++++++ > arch/x86/entry/common.c | 10 ++++++++ > arch/x86/include/asm/nospec-branch.h | 2 + > arch/x86/kernel/cpu/bugs.c | 4 ++- > arch/x86/kernel/nmi.c | 6 +++++ > arch/x86/kernel/traps.c | 9 +++++++ > 6 files changed, 71 insertions(+), 1 deletion(-) > > --- a/Documentation/x86/mds.rst > +++ b/Documentation/x86/mds.rst > @@ -94,3 +94,44 @@ According to current knowledge additiona > itself are not required because the necessary gadgets to expose the leaked > data cannot be controlled in a way which allows exploitation from malicious > user space or VM guests. > + > +Mitigation points > +----------------- > + > +1. Return to user space > +^^^^^^^^^^^^^^^^^^^^^^^ > + When transitioning from kernel to user space the CPU buffers are flushed > + on affected CPUs: > + > + - always when the mitigation mode is full. The migitation is enabled Currently the mitigation is always full. > + through the static key mds_user_clear. > + > + This covers transitions from kernel to user space through a return to > + user space from a syscall and from an interrupt or a regular exception. > + > + There are other kernel to user space transitions which are not covered > + by this: NMIs and all non maskable exceptions which go through the > + paranoid exit, which means that they are not invoking the regular Actually, NMI *is* mitigated. What is a non maskable exception? The statement about all paranoid exits being covered isn't correct, because #DF is mitigated. > + prepare_exit_to_usermode() which handles the CPU buffer clearing. > + > + Access to sensible data like keys, credentials in the NMI context is > + mostly theoretical: The CPU can do prefetching or execute a > + misspeculated code path and thereby fetching data which might end up > + leaking through a buffer. This paragraph can be removed, since NMI is mitigated. > + > + But for mounting other attacks the kernel stack address of the task is > + already valuable information. So in full mitigation mode, the NMI is > + mitigated on the return from do_nmi() to provide almost complete > + coverage. This one is correct. > + > + There is one non maskable exception which returns through paranoid exit Again the phrase "non maskable exception". Maybe I'm missing something but I have no idea what that means. > + and is to some extent controllable from user space through > + modify_ldt(2): #DF. So mitigation is required in the double fault > + handler as well. > + > + Another corner case is a #MC which hits between the buffer clear and the > + actual return to user. As this still is in kernel space it takes the > + paranoid exit path which does not clear the CPU buffers. So the #MC > + handler repopulates the buffers to some extent. Machine checks are not > + reliably controllable and the window is extremly small so mitigation > + would just tick a checkbox that this theoretical corner case is covered. There is no mention of #DB anywhere, shouldn't it also be mitigated? > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > > #define CREATE_TRACE_POINTS > #include > @@ -180,6 +181,13 @@ static void exit_to_usermode_loop(struct > } > } > > +static inline void mds_user_clear_cpu_buffers(void) > +{ > + if (!static_branch_likely(&mds_user_clear)) > + return; > + mds_clear_cpu_buffers(); > +} > + > /* Called with IRQs disabled. */ > __visible inline void prepare_exit_to_usermode(struct pt_regs *regs) > { > @@ -212,6 +220,8 @@ static void exit_to_usermode_loop(struct > #endif > > user_enter_irqoff(); > + > + mds_user_clear_cpu_buffers(); > } > > #define SYSCALL_EXIT_WORK_FLAGS \ > --- a/arch/x86/include/asm/nospec-branch.h > +++ b/arch/x86/include/asm/nospec-branch.h > @@ -318,6 +318,8 @@ DECLARE_STATIC_KEY_FALSE(switch_to_cond_ > DECLARE_STATIC_KEY_FALSE(switch_mm_cond_ibpb); > DECLARE_STATIC_KEY_FALSE(switch_mm_always_ibpb); > > +DECLARE_STATIC_KEY_FALSE(mds_user_clear); > + > #include > > /** > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -63,10 +63,12 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_i > /* Control unconditional IBPB in switch_mm() */ > DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb); > > +/* Control MDS CPU buffer clear before returning to user space */ > +DEFINE_STATIC_KEY_FALSE(mds_user_clear); > + > void __init check_bugs(void) > { > identify_boot_cpu(); > - > /* > * identify_boot_cpu() initialized SMT support information, let the > * core code know. > --- a/arch/x86/kernel/nmi.c > +++ b/arch/x86/kernel/nmi.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > #define CREATE_TRACE_POINTS > #include > @@ -533,6 +534,11 @@ do_nmi(struct pt_regs *regs, long error_ > write_cr2(this_cpu_read(nmi_cr2)); > if (this_cpu_dec_return(nmi_state)) > goto nmi_restart; > + > + if (!static_branch_likely(&mds_user_clear)) > + return; > + if (user_mode(regs)) > + mds_clear_cpu_buffers(); This could be simplied: if (user_mode(regs)) mds_user_clear_cpu_buffers(); > } > NOKPROBE_SYMBOL(do_nmi); > > --- a/arch/x86/kernel/traps.c > +++ b/arch/x86/kernel/traps.c > @@ -366,6 +366,15 @@ dotraplinkage void do_double_fault(struc > regs->ip = (unsigned long)general_protection; > regs->sp = (unsigned long)&gpregs->orig_ax; > > + /* > + * This situation can be triggered by userspace via > + * modify_ldt(2) and the return does not take the regular > + * user space exit, so a CPU buffer clear is required when > + * MDS mitigation is enabled. > + */ > + if (static_branch_unlikely(&mds_user_clear)) > + mds_clear_cpu_buffers(); Shouldn't it be likely? Anyway this can just use mds_user_clear_cpu_buffers(). -- Josh