linux-kernel - Re: [RFC][PATCH v2 11/11] context_tracking,x86: Fix text_poke_sync() vs NOHZ

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: <20211021192543.GV174703@worktop.programming.kicks-ass.net>
Date:   Thu, 21 Oct 2021 21:25:43 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Marcelo Tosatti <mtosatti@...hat.com>
Cc:     gor@...ux.ibm.com, jpoimboe@...hat.com, jikos@...nel.org,
        mbenes@...e.cz, pmladek@...e.com, mingo@...nel.org,
        linux-kernel@...r.kernel.org, joe.lawrence@...hat.com,
        fweisbec@...il.com, tglx@...utronix.de, hca@...ux.ibm.com,
        svens@...ux.ibm.com, sumanthk@...ux.ibm.com,
        live-patching@...r.kernel.org, paulmck@...nel.org,
        rostedt@...dmis.org, x86@...nel.org
Subject: Re: [RFC][PATCH v2 11/11] context_tracking,x86: Fix text_poke_sync()
 vs NOHZ_FULL

On Thu, Oct 21, 2021 at 03:39:35PM -0300, Marcelo Tosatti wrote:
> Peter,
> 
> static __always_inline void arch_exit_to_user_mode(void)
> {
>         mds_user_clear_cpu_buffers();
> }
> 
> /**
>  * mds_user_clear_cpu_buffers - Mitigation for MDS and TAA vulnerability
>  *
>  * Clear CPU buffers if the corresponding static key is enabled
>  */
> static __always_inline void mds_user_clear_cpu_buffers(void)
> {
>         if (static_branch_likely(&mds_user_clear))
>                 mds_clear_cpu_buffers();
> }
> 
> We were discussing how to perform objtool style validation 
> that no code after the check for 

I'm not sure what the point of the above is... Were you trying to ask
for validation that nothing runs after the mds_user_clear_cpu_buffer()?

That isn't strictly true today, there's lockdep code after it. I can't
recall why that order is as it is though.

Pretty much everything in noinstr is magical, we just have to think
harder there (and possibly start writing more comments there).

> > +             /* NMI happens here and must still do/finish CT_WORK_n */
> > +             sync_core();
> 
> But after the discussion with you, it seems doing the TLB checking 
> and (also sync_core) checking very late/very early on exit/entry 
> makes things easier to review.

I don't know about late, it must happen *very* early in entry. The
sync_core() must happen before any self-modifying code gets called
(static_branch, static_call, etc..) with possible exception of the
context_tracking static_branch.

The TLBi must also happen super early, possibly while still on the
entry stack (since the task stack is vmap'ed). We currently don't run C
code on the entry stack, that needs quite a bit of careful work to make
happen.

> Can then use a single atomic variable with USER/KERNEL state and cmpxchg
> loops.

We're not going to add an atomic to context tracking. There is one, we
just got to extract/share it with RCU.