Re: [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

["Jan Beulich" <JBeulich@suse.com>]

>>> On 31.01.19 at 23:39, <andrew.cooper3@citrix.com> wrote:
> On 25/01/2019 10:14, Jan Beulich wrote:
>>>>> On 24.01.19 at 22:29, <andrew.cooper3@citrix.com> wrote:
>>> Worse is the "evaluate condition, stash result, fence, use variable"
>>> option, which is almost completely useless.  If you work out the
>>> resulting instruction stream, you'll have a conditional expression
>>> calculated down into a register, then a fence, then a test register and
>>> conditional jump into one of two basic blocks.  This takes the perf hit,
>>> and doesn't protect either of the basic blocks for speculative
>>> mis-execution.
>> How does it not protect anything? It shrinks the speculation window
>> to just the register test and conditional branch,
> 
> A speculation window starts at a number of arbitrary points, and persist
> until the processor has confirmed the speculation precondition was true
> or false.  There can be multiple overlapping speculative windows open at
> a single time.
> 
>> which ought to be
>> far smaller than that behind a memory access which fails to hit any
>> of the caches (and perhaps even any of the TLBs). This is the more
>> that LFENCE does specifically not prevent insn fetching from
>> continuing.
> 
> I'm afraid that isn't relevant.
> 
> For the attack described in this series, the speculation window which
> matters starts with a conditional jump.  In this scenario, the fact that
> you have stashed the value and issued a fence doesn't stop an attacker
> from controlling the conditional jump.
> 
> The lfence doesn't interact with the branch predictor.  Any poisoned
> predictions will survive.
> 
> As a result, the only safe course of action is to let the processor
> follow the prediction, *then* wait for speculation to catch up with
> reality and see whether the prediction was correct.  As such, code is
> only safe when the fence is at the head of both basic blocks.
> 
>> That said I agree that the LFENCE would better sit between the
>> register test and the conditional branch, but as we've said so many
>> times before - this can't be achieved without compiler support.
> 
> It also doesn't fix the problem.
> 
> Both of these examples do narrow the speculation to just having each
> basic block entered with each others legitimate entry condition, but the
> following code sample is still vulnerable to leakage under these two
> related strategies.
> 
> int foo(int a, int b, int c)
> {
>     if ( eval_nospec(a) )
>         return array_b[b];
>     else
>         return array_c[c];
> }

All fine, but as you say, speculation starts at the conditional
branch. If the LFENCE sits immediately ahead of it, how far can
speculation actually make it before it gets canceled? I'm not
putting under question that the best we can do is adding one
fence on each side, but as we're anyway debating how to
balance added security vs lost performance, I remain not fully
convinced that this isn't an option someone may want to pick.

>> Then again, following an earlier reply of mine on another sub-
>> thread, nothing really prevents the compiler from moving ahead
>> and folding the two LFENCEs of the "both branches" model into
>> one. It just so happens that apparently right now this never
>> occurs (assuming Norbert has done full generated code analysis
>> to confirm the intended placement).
> 
> Following on from that other thread, eval_nospec() is only useful if we
> can guarantee that it places fences at the head of each basic block,
> rather than elsewhere.

We clearly agree on this aspect. The questionable point though
isn't what is useful, but what the compiler might possibly do with
the constructs we add.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel