Re: [PATCH 0/7] Mitigation against unsafe data speculation (CVE-2017-5753)

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 10/07/18 14:48, Bill Schmidt wrote:
> 
>> On Jul 10, 2018, at 3:49 AM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>>
>> On 10/07/18 00:13, Jeff Law wrote:
>>> On 07/09/2018 10:38 AM, Richard Earnshaw wrote:
>>>>
>>>> The patches I posted earlier this year for mitigating against
>>>> CVE-2017-5753 (Spectre variant 1) attracted some useful feedback, from
>>>> which it became obvious that a rethink was needed.  This mail, and the
>>>> following patches attempt to address that feedback and present a new
>>>> approach to mitigating against this form of attack surface.
>>>>
>>>> There were two major issues with the original approach:
>>>>
>>>> - The speculation bounds were too tightly constrained - essentially
>>>>  they had to represent and upper and lower bound on a pointer, or a
>>>>  pointer offset.
>>>> - The speculation constraints could only cover the immediately preceding
>>>>  branch, which often did not fit well with the structure of the existing
>>>>  code.
>>>>
>>>> An additional criticism was that the shape of the intrinsic did not
>>>> fit particularly well with systems that used a single speculation
>>>> barrier that essentially had to wait until all preceding speculation
>>>> had to be resolved.
>>> Right.  I suggest the Intel and IBM reps chime in on the updated semantics.
>>>
>>
>> Yes, logically, this is a boolean tracker value.  In practice we use ~0
>> for true and 0 for false, so that we can simply use it as a mask
>> operation later.
>>
>> I hope this intrinsic will be even more acceptable than the one that
>> Bill Schmidt acked previously, it's even simpler than the version we had
>> last time.
> 
> Yes, I think this looks quite good.  Thanks!
> 
> Thanks also for digging into the speculation tracking algorithm.  This
> has good potential as a conservative opt-in approach.  The obvious
> concern is whether performance will be acceptable even for apps
> that really want the protection.
> 
> We took a look at Chandler's WIP LLVM patch and ran some SPEC2006 
> numbers on a Skylake box.  We saw geomean degradations of about
> 42% (int) and 33% (fp).  (This was just one test, so caveat emptor.)
> This isn't terrible given the number of potential false positives and the
> early state of the algorithm, but it's still a lot from a customer perspective.
> I'll be interested in whether your interprocedural improvements are
> able to reduce the conservatism a bit.
> 

So I don't have any numbers for SPEC2006.  I have some initial numbers
for SPEC2000 when just adding the tracking code (so not applying the
second part of the mitigation).  In that case INT2000 is down by ~13%
and FP2000 was by comparison almost in the noise (~2.4%).

Applying the tracker value to all memory loads would push those numbers
up significantly, I suspect.  That's part of the reason for preferring
the intrinsic rather than automatic mitigation: the intrinsic is much
more targeted.

R.


> Thanks,
> Bill
>>
>>>>
>>>> To address all of the above, these patches adopt a new approach, based
>>>> in part on a posting by Chandler Carruth to the LLVM developers list
>>>> (https://lists.llvm.org/pipermail/llvm-dev/2018-March/122085.html),
>>>> but which we have extended to deal with inter-function speculation.
>>>> The patches divide the problem into two halves.
>>> We're essentially turning the control dependency into a value that we
>>> can then use to munge the pointer or the resultant data.
>>>
>>>>
>>>> The first half is some target-specific code to track the speculation
>>>> condition through the generated code to provide an internal variable
>>>> which can tell us whether or not the CPU's control flow speculation
>>>> matches the data flow calculations.  The idea is that the internal
>>>> variable starts with the value TRUE and if the CPU's control flow
>>>> speculation ever causes a jump to the wrong block of code the variable
>>>> becomes false until such time as the incorrect control flow
>>>> speculation gets unwound.
>>> Right.
>>>
>>> So one of the things that comes immediately to mind is you have to run
>>> this early enough that you can still get to all the control flow and
>>> build your predicates.  Otherwise you have do undo stuff like
>>> conditional move generation.
>>
>> No, the opposite, in fact.  We want to run this very late, at least on
>> Arm systems (AArch64 or AArch32).  Conditional move instructions are
>> fine - they're data-flow operations, not control flow (in fact, that's
>> exactly what the control flow tracker instructions are).  By running it
>> late we avoid disrupting any of the earlier optimization passes as well.
>>
>>>
>>> On the flip side, the earlier you do this mitigation, the more you have
>>> to worry about what the optimizers are going to do to the code later in
>>> the pipeline.  It's almost guaranteed a naive implementation is going to
>>> muck this up since we can propagate the state of the condition into the
>>> arms which will make the predicate state a compile time constant.
>>>
>>> In fact this seems to be running into the area of pointer providence and
>>> some discussions we had around atomic a few years back.
>>>
>>> I also wonder if this could be combined with taint analysis to produce a
>>> much lower overhead solution in cases were developers have done analysis
>>> and know what objects are potentially under attacker control.  So
>>> instead of analyzing everything, we can have a much narrower focus.
>>
>> Automatic application of the tracker to vulnerable variables would be
>> nice, but I haven't attempted to go there yet: at present I still rely
>> on the user to annotate code with the new intrinsic.
>>
>> That doesn't mean that we couldn't extend the overall approach later to
>> include automatic tracking.
>>
>>>
>>> The pointer munging could well run afoul of alias analysis engines that
>>> don't expect to be seeing those kind of operations.
>>
>> I think the pass runs late enough that it isn't a problem.
>>
>>>
>>> Anyway, just some initial high level thoughts.  I'm sure there'll be
>>> more as I read the implementation.
>>>
>>
>> Thanks for starting to look at this so quickly.
>>
>> R.
>>
>>>
>>> Jeff
>