On Mon, Jul 9, 2018 at 6:39 PM Richard Earnshaw
<richard.earns...@arm.com> 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.
>
> 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.
>
> 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.
>
> The second half is that a new intrinsic function is introduced that is
> much simpler than we had before. The basic version of the intrinsic
> is now simply:
>
> T var = __builtin_speculation_safe_value (T unsafe_var);
>
> Full details of the syntax can be found in the documentation patch, in
> patch 1. In summary, when not speculating the intrinsic returns
> unsafe_var; when speculating then if it can be shown that the
> speculative flow has diverged from the intended control flow then zero
> is returned. An optional second argument can be used to return an
> alternative value to zero. The builtin may cause execution to pause
> until the speculation state is resolved.
So a trivial target implementation would be to emit a barrier and then
it would always return unsafe_var and never zero. What I don't understand
fully is what users should do here, thus what the value of ever returning
"unsafe" is. Also I wonder why the API is forcing you to single-out a
special value instead of doing
bool safe = __builtin_speculation_safe_value_p (T unsafe_value);
if (!safe)
/* what now? */
I'm only guessing that the correct way to handle "unsafe" is basically
while (__builtin_speculation_safe_value (val) == 0)
;
use val, it's now safe
that is, the return value is only interesting in sofar as to whether it is equal
to val or the special value?
That said, I wonder why we don't hide that details from the user and
provide a predicate instead.
Richard.
> There are seven patches in this set, as follows.
>
> 1) Introduces the new intrinsic __builtin_sepculation_safe_value.
> 2) Adds a basic hard barrier implementation for AArch32 (arm) state.
> 3) Adds a basic hard barrier implementation for AArch64 state.
> 4) Adds a new command-line option -mtrack-speculation (currently a no-op).
> 5) Disables CB[N]Z and TB[N]Z when -mtrack-speculation.
> 6) Adds the new speculation tracking pass for AArch64
> 7) Uses the new speculation tracking pass to generate CSDB-based barrier
> sequences
>
> I haven't added a speculation-tracking pass for AArch32 at this time.
> It is possible to do this, but would require quite a lot of rework for
> the arm backend due to the limited number of registers that are
> available.
>
> Although patch 6 is AArch64 specific, I'd appreciate a review from
> someone more familiar with the branch edge code than myself. There
> appear to be a number of tricky issues with more complex edges so I'd
> like a second opinion on that code in case I've missed an important
> case.
>
> R.
>
>
>
> Richard Earnshaw (7):
> Add __builtin_speculation_safe_value
> Arm - add speculation_barrier pattern
> AArch64 - add speculation barrier
> AArch64 - Add new option -mtrack-speculation
> AArch64 - disable CB[N]Z TB[N]Z when tracking speculation
> AArch64 - new pass to add conditional-branch speculation tracking
> AArch64 - use CSDB based sequences if speculation tracking is enabled
>
> gcc/builtin-types.def | 6 +
> gcc/builtins.c | 57 ++++
> gcc/builtins.def | 20 ++
> gcc/c-family/c-common.c | 143 +++++++++
> gcc/c-family/c-cppbuiltin.c | 5 +-
> gcc/config.gcc | 2 +-
> gcc/config/aarch64/aarch64-passes.def | 1 +
> gcc/config/aarch64/aarch64-protos.h | 3 +-
> gcc/config/aarch64/aarch64-speculation.cc | 494
> ++++++++++++++++++++++++++++++
> gcc/config/aarch64/aarch64.c | 88 +++++-
> gcc/config/aarch64/aarch64.md | 140 ++++++++-
> gcc/config/aarch64/aarch64.opt | 4 +
> gcc/config/aarch64/iterators.md | 3 +
> gcc/config/aarch64/t-aarch64 | 10 +
> gcc/config/arm/arm.md | 21 ++
> gcc/config/arm/unspecs.md | 1 +
> gcc/doc/cpp.texi | 4 +
> gcc/doc/extend.texi | 29 ++
> gcc/doc/invoke.texi | 10 +-
> gcc/doc/md.texi | 15 +
> gcc/doc/tm.texi | 20 ++
> gcc/doc/tm.texi.in | 2 +
> gcc/target.def | 23 ++
> gcc/targhooks.c | 27 ++
> gcc/targhooks.h | 2 +
> gcc/testsuite/gcc.dg/spec-barrier-1.c | 40 +++
> gcc/testsuite/gcc.dg/spec-barrier-2.c | 19 ++
> gcc/testsuite/gcc.dg/spec-barrier-3.c | 13 +
> 28 files changed, 1191 insertions(+), 11 deletions(-)
> create mode 100644 gcc/config/aarch64/aarch64-speculation.cc
> create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-1.c
> create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-2.c
> create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-3.c
