linux-kernel - Re: [RFC PATCH v2 6/6] x86/entry/pti: don't switch PGD on when pti

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:   Wed, 10 Jan 2018 10:11:02 +0100
From:   Willy Tarreau <w@....eu>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Brian Gerst <brgerst@...il.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [RFC PATCH v2 6/6] x86/entry/pti: don't switch PGD on when
 pti_disable is set

On Wed, Jan 10, 2018 at 09:22:07AM +0100, Peter Zijlstra wrote:
> On Tue, Jan 09, 2018 at 01:56:20PM +0100, Willy Tarreau wrote:
> >   - use pti_disable instead of task flag
> > ---
> >  arch/x86/entry/calling.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> > index 2c0d3b5..5361a10 100644
> > --- a/arch/x86/entry/calling.h
> > +++ b/arch/x86/entry/calling.h
> > @@ -229,6 +229,11 @@
> >  
> >  .macro SWITCH_TO_USER_CR3_NOSTACK scratch_reg:req scratch_reg2:req
> >  	ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI
> > +
> > +	/* The "pti_disable" mm attribute is mirrored into this per-cpu var */
> > +	cmpb	$0, PER_CPU_VAR(pti_disable)
> > +	jne	.Lend_\@
> > +
> >  	mov	%cr3, \scratch_reg
> 
> So could you switch back to a task flag for this? That word is already
> cache-hot on the exit path while your new variable is not.

That's a good point. There's already been some demands for a per-thread
setting.

What I can propose then is to partially revert the changes to have this :

  - arch_prctl() adjusts the task flag and not a per-mm variable anymore
    (Linus, are you OK for this ?)

  - arch_prctl() only accepts to perform the action if mm->mm_users == 1
    so that we don't change the setting after having created threads ;
    this way the task flag is replicated to all future threads ;

  - later we may decide to permit re-enabling PTI per thread if it was
    disabled.

If we agree on this, I'd like to propose to have two flags :

  - TIF_DISABLE_PTI_NOW : disable PTI for the current task, reset by execve()
  - TIF_DISABLE_PTI_NEXT : disable PTI after execve(), reset by execve()

execve() would then simply do :

     TIF_DISABLE_PTI_NOW  = TIF_DISABLE_PTI_NEXT;
     TIF_DISABLE_PTI_NEXT = 0;

The former would be used by applications using their own configuration.
The latter would be used by wrappers. This way we seem to cover the various
use cases. And we make this depend on a sysctl that allows the admin to
globally and permanently disable the feature and which is disabled by
default.

Any objection ?

Regards,
Willy