linux-kernel - Re: [PATCH] x86/speculation: Use Indirect Branch Prediction Barrier in 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180129135643.GA31322@isilmar-4.linta.de>
Date:   Mon, 29 Jan 2018 14:56:43 +0100
From:   Dominik Brodowski <linux@...inikbrodowski.net>
To:     David Woodhouse <dwmw2@...radead.org>
Cc:     arjan@...ux.intel.com, tglx@...utronix.de, karahmed@...zon.de,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        tim.c.chen@...ux.intel.com, bp@...en8.de, peterz@...radead.org
Subject: Re: [PATCH] x86/speculation: Use Indirect Branch Prediction Barrier
 in context switch

On Mon, Jan 29, 2018 at 12:44:59PM +0000, David Woodhouse wrote:
> On Mon, 2018-01-29 at 13:28 +0100, Dominik Brodowski wrote:
> 
> > The commit message is much more about the A->idle-> improvement than
> > on the basic design decisions to limit this to non-dumpable processes.
> 
> Yeah, I collapsed the commit messages from the three commits into one.
> But the resulting commit message does start with the basic non-dumpable 
> concept, and explain the trade-off (sensitivity vs. overhead) using the
> comment from what was the second path in the series.
> 
> >  And
> > that still seems to be under discussion (see, for example, Jon Masters
> > message of today, https://lkml.org/lkml/2018/1/29/34 ). So this design
> > choice should, at least, be more explicit (if not tunable...).
> 
> That isn't stunningly relevant here. Yes, we might build userspace with
> retpoline support and there'll be an additional case here so it becomes
> !dumpable && !retpoline-userspace. But that's all way off in the
> future.
> 
> > > @@ -219,6 +220,25 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
> > >  	} else {
> > >  		u16 new_asid;
> > >  		bool need_flush;
> > > +		u64 last_ctx_id = this_cpu_read(cpu_tlbstate.last_ctx_id);
> > > +
> > > +		/*
> > > +		 * Avoid user/user BTB poisoning by flushing the branch
> > > +		 * predictor when switching between processes. This stops
> > > +		 * one process from doing Spectre-v2 attacks on another.
> > > +		 *
> > > +                 * As an optimization, flush indirect branches only when
> > > +                 * switching into processes that disable dumping.
> > > +                 *
> > > +                 * This will not flush branches when switching into kernel
> > > +		 * threads. It will also not flush if we switch to idle
> > Whitespace damage. And maybe add ", as the kernel depends on retpoline
> > protection instead" after "threads" here -- I think that was the reason why
> > you think kernel threads are safe; or did I misunderstand you?
> 
> I'll fix up the whitespace; thanks. For the other comment... no, if
> kernel threads needed *any* kind of protection from this IBPB then we'd
> be hosed by the time we got here anyway. Let's not imply that an IBPB
> here would be at all useful for the kernel under any circumstances.
> 
> 
> > > 
> > > +		 * thread and back to the same process. It will flush if we
> > > +		 * switch to a different non-dumpable process.
> > "process, as that gives additional protection to high value processes like
> > gpg. Other processes are left unprotected here to reduce the overhead of the
> > barrier [... maybe add some rationale here ...]"
> 
> The rationale is to reduce the overhead of the barrier. I've added that
> to the comment, based on what's in the commit message already. Thanks.
> 
> http://git.infradead.org/users/dwmw2/linux-retpoline.git/commitdiff/8431c5a87520cd14f8769745589443e603682b23

That reads much better now, thanks. All other issues (e.g. adding an
IPBP also for dumpable tasks) can easily be discussed on top of that commit.

Thanks,
	Dominik