linux-kernel - Re: [PATCH 15/24] x86/mm: Allow flushing for future ASID switches

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:   Tue, 28 Nov 2017 20:05:05 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     Andy Lutomirski <luto@...capital.net>,
        Ingo Molnar <mingo@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 15/24] x86/mm: Allow flushing for future ASID switches

On Tue, Nov 28, 2017 at 10:13:30AM -0800, Dave Hansen wrote:
> Thanks for looking at this, Peter.  I've been resisting doing this for a
> bit and it's an embarrassingly small amount of code.

Right, well, its not complete yet, and it might be complete crap :-)

> On 11/28/2017 08:39 AM, Peter Zijlstra wrote:
> > @@ -220,7 +221,21 @@ For 32-bit we have the following conventions - kernel is built with
> >  .macro SWITCH_TO_USER_CR3 scratch_reg:req
> >  	STATIC_JUMP_IF_FALSE .Lend_\@, kaiser_enabled_key, def=1
> >  	mov	%cr3, \scratch_reg
> > -	ADJUST_USER_CR3 \scratch_reg
> > +	push	\scratch_reg
> 
> Do we have a good stack in all the spots that we need to do this?  It
> may have changed with the trampoline stack, but I'm 100% sure that it
> wasn't so in the recent past.

Dunno really. I figured I'd give it a go and see what happens. So far
the machine still works. But I was hoping Andy would have an opinion on
this.

> Let me see if I'm reading the assembly right.

Yep, seems you can read asm :-)


> > +DECLARE_PER_CPU(unsigned long, __asid_flush);
> 
> Could we spare enough space to make this something like
> user_asid_flush_pending_mask?

Yeah, if I can get it all working we'll bikeshed on a name ;-)

> It took me a minute to realize that it was a mask.  Also, since we only
> have 6 asids, should we bit a bit more stingy with the type?

I picked unsigned long because our bitops (__set_bit in this case, use
it), and I know we're LE and could simply use a shorter type, but meh.

> It took me a minute to realize that mixing these is still OK, even if
> the mm associated with the ASID changes.  It's because once the ASID is
> stale, it doesn't matter *why* it is stale.  Just that the next guy who
> *uses* it needs to do the flush.  You can do 1,000 tlb flushes, a
> context switch, a tlb flush and another context switch, but if you only
> go out to userspace once, you only need 1 ASID flush.  That fits
> perfectly with this bit that gets set a bunch of times and only cleared
> once at exit to userspace.

Just so.

I'm now staring at the RESTORE_CR3 stuff, and that appears to be called
in the NMI handling where the stack is not to be used (if I read it
right), so that's going to be a little more tricky.

Let me prod at that..