linux-kernel - Re: [PATCH v6 5/6] Optionally flush L1D on context switch

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]
Date:   Thu, 14 May 2020 21:28:38 +0000
From:   "Singh, Balbir" <sblbir@...zon.com>
To:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "keescook@...omium.org" <keescook@...omium.org>,
        "thomas.lendacky@....com" <thomas.lendacky@....com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>,
        "jpoimboe@...hat.com" <jpoimboe@...hat.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "dave.hansen@...el.com" <dave.hansen@...el.com>
Subject: Re: [PATCH v6 5/6] Optionally flush L1D on context switch

On Wed, 2020-05-13 at 17:27 +0200, Thomas Gleixner wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> Balbir Singh <sblbir@...zon.com> writes:
> 
> > Implement a mechanism to selectively flush the L1D cache. The goal
> > is to
> > allow tasks that are paranoid due to the recent snoop assisted data
> > sampling
> > vulnerabilites, to flush their L1D on being switched out.  This
> > protects
> > their data from being snooped or leaked via side channels after the
> > task
> > has context switched out.
> > 
> > There are two scenarios we might want to protect against, a task
> > leaving
> > the CPU with data still in L1D (which is the main concern of this
> > patch),
> > the second scenario is a malicious task coming in (not so well
> > trusted)
> > for which we want to clean up the cache before it starts. Only the
> > case
> > for the former is addressed.
> > 
> > A new thread_info flag TIF_SPEC_FLUSH_L1D is added to track tasks
> > which
> > opt-into L1D flushing. cpu_tlbstate.last_user_mm_spec is used to
> > convert
> > the TIF flags into mm state (per cpu via last_user_mm_spec) in
> > cond_mitigation(), which then used to do decide when to call
> > flush_l1d().
> > 
> > Add prctl()'s to opt-in to the L1D cache on context switch out, the
> > existing mechanisms of tracking prev_mm via cpu_tlbstate is
> > reused to track state of the tasks and to flush the L1D cache.
> > The prctl interface is generic and can be ported over to other
> > architectures.
> > 
> > Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> > Signed-off-by: Balbir Singh <sblbir@...zon.com>
> > Reviewed-by: Kees Cook <keescook@...omium.org>
> > ---
> >  arch/x86/include/asm/thread_info.h |  7 ++++-
> >  arch/x86/mm/tlb.c                  | 44
> > ++++++++++++++++++++++++++++--
> >  include/uapi/linux/prctl.h         |  4 +++
> >  kernel/sys.c                       | 20 ++++++++++++++
> >  4 files changed, 72 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/thread_info.h
> > b/arch/x86/include/asm/thread_info.h
> > index 8de8ceccb8bc..67de693d9ba1 100644
> > --- a/arch/x86/include/asm/thread_info.h
> > +++ b/arch/x86/include/asm/thread_info.h
> > @@ -84,7 +84,7 @@ struct thread_info {
> >  #define TIF_SYSCALL_AUDIT    7       /* syscall auditing active */
> >  #define TIF_SECCOMP          8       /* secure computing */
> >  #define TIF_SPEC_IB          9       /* Indirect branch speculation
> > mitigation */
> > -#define TIF_SPEC_FORCE_UPDATE        10      /* Force speculation
> > MSR update in context switch */
> > +#define TIF_SPEC_FLUSH_L1D   10      /* Flush L1D on mm switches
> > (processes) */
> >  #define TIF_USER_RETURN_NOTIFY       11      /* notify kernel of
> > userspace return */
> >  #define TIF_UPROBE           12      /* breakpointed or
> > singlestepping */
> >  #define TIF_PATCH_PENDING    13      /* pending live patching
> > update */
> > @@ -96,6 +96,7 @@ struct thread_info {
> >  #define TIF_MEMDIE           20      /* is terminating due to OOM
> > killer */
> >  #define TIF_POLLING_NRFLAG   21      /* idle is polling for
> > TIF_NEED_RESCHED */
> >  #define TIF_IO_BITMAP                22      /* uses I/O bitmap */
> > +#define TIF_SPEC_FORCE_UPDATE        23      /* Force speculation
> > MSR update in context switch */
> >  #define TIF_FORCED_TF                24      /* true if TF in
> > eflags artificially */
> >  #define TIF_BLOCKSTEP                25      /* set when we want
> > DEBUGCTLMSR_BTF */
> >  #define TIF_LAZY_MMU_UPDATES 27      /* task is updating the mmu
> > lazily */
> > @@ -132,6 +133,7 @@ struct thread_info {
> >  #define _TIF_ADDR32          (1 << TIF_ADDR32)
> >  #define _TIF_X32             (1 << TIF_X32)
> >  #define _TIF_FSCHECK         (1 << TIF_FSCHECK)
> > +#define _TIF_SPEC_FLUSH_L1D  (1 << TIF_SPEC_FLUSH_L1D)
> 
> Bah. These defines are ordered in the same way as the TIF defines....
> 
> >  /*
> > - * Bits to mangle the TIF_SPEC_IB state into the mm pointer which
> > is
> > + * Bits to mangle the TIF_SPEC_* state into the mm pointer which is
> >   * stored in cpu_tlb_state.last_user_mm_spec.
> >   */
> >  #define LAST_USER_MM_IBPB    0x1UL
> > -#define LAST_USER_MM_SPEC_MASK       (LAST_USER_MM_IBPB)
> > +#define LAST_USER_MM_L1D_FLUSH       0x2UL
> > +#define LAST_USER_MM_SPEC_MASK       (LAST_USER_MM_IBPB |
> > LAST_USER_MM_L1D_FLUSH)
> 
> You lost
> 
> +       BUILD_BUG_ON(TIF_SPEC_FLUSH_L1D != TIF_SPEC_IB + 1);
> 
> from patch I gave you.


Oops.. I'll fix up both and redo patch 5/6, by splitting it up, into
interface vs flush bits

Thanks,
Balbir Singh.