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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1517229899.18619.9.camel@infradead.org>
Date:   Mon, 29 Jan 2018 12:44:59 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
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, 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

Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (5213 bytes)