[00/11] (v2) Mitigation against unsafe data speculation (CVE-2017-5753)

Message ID 1532684275-13041-1-git-send-email-Richard.Earnshaw@arm.com
Headers show
Series
  • (v2) Mitigation against unsafe data speculation (CVE-2017-5753)
Related show

Message

Richard Earnshaw (lists) July 27, 2018, 9:37 a.m.
Port Maintainers: You need to decide what action is required for your
port to handle speculative execution, even if that action is to use
the trivial no-speculation on this architecture.  You must also
consider whether or not a furture implementation of your architecture
might need to deal with this in making that decision.

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.

There are eleven 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
8) Provides an alternative hook implementation for use on targets that never
   speculatively execute
9) Provides an trivial example of using that hook in the pdp11 backend.
10) Provides a possible implementation of the hard barrier for x86
11) Updates the PowerPC backend which already had a suitable barrier under
    a different name.

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 (11):
  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
  targhooks - provide an alternative hook for targets that never execute
    speculatively
  pdp11 - example of a port not needing a speculation barrier
  x86 - add speculation_barrier pattern
  rs6000 - add speculation_barrier pattern

 gcc/builtin-attrs.def                       |   2 +
 gcc/builtin-types.def                       |   6 +
 gcc/builtins.c                              |  60 ++++
 gcc/builtins.def                            |  22 ++
 gcc/c-family/c-common.c                     | 164 +++++++++
 gcc/c-family/c-cppbuiltin.c                 |   7 +-
 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                |  94 +++++-
 gcc/config/aarch64/aarch64.md               | 141 +++++++-
 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/config/i386/i386.md                     |  10 +
 gcc/config/pdp11/pdp11.c                    |   3 +
 gcc/config/rs6000/rs6000.c                  |   2 +-
 gcc/config/rs6000/rs6000.md                 |   2 +-
 gcc/doc/cpp.texi                            |   4 +
 gcc/doc/extend.texi                         |  91 +++++
 gcc/doc/invoke.texi                         |  10 +-
 gcc/doc/md.texi                             |  15 +
 gcc/doc/tm.texi                             |  36 ++
 gcc/doc/tm.texi.in                          |   4 +
 gcc/target.def                              |  40 +++
 gcc/targhooks.c                             |  39 +++
 gcc/targhooks.h                             |   4 +
 gcc/testsuite/c-c++-common/spec-barrier-1.c |  38 +++
 gcc/testsuite/c-c++-common/spec-barrier-2.c |  17 +
 gcc/testsuite/gcc.dg/spec-barrier-3.c       |  13 +
 33 files changed, 1350 insertions(+), 13 deletions(-)
 create mode 100644 gcc/config/aarch64/aarch64-speculation.cc
 create mode 100644 gcc/testsuite/c-c++-common/spec-barrier-1.c
 create mode 100644 gcc/testsuite/c-c++-common/spec-barrier-2.c
 create mode 100644 gcc/testsuite/gcc.dg/spec-barrier-3.c

-- 
2.7.4

Comments

John David Anglin July 27, 2018, 7:48 p.m. | #1
On 2018-07-27 5:37 AM, Richard Earnshaw wrote:
> Port Maintainers: You need to decide what action is required for your

> port to handle speculative execution, even if that action is to use

> the trivial no-speculation on this architecture.  You must also

> consider whether or not a furture implementation of your architecture

> might need to deal with this in making that decision.

On hppa, I think we should go with the hook that assumes there is no 
speculative execution.

Nominally, there is branch prediction and speculative execution; but the 
spectre test program
was not able to successfully access memory on my rp3440.

As far as I know, the details of speculative execution on PA-RISC are 
not public.  Jeff would know
best.

Dave

-- 
John David Anglin  dave.anglin@bell.net
Jeff Law Aug. 2, 2018, 6:40 p.m. | #2
On 07/27/2018 01:48 PM, John David Anglin wrote:
> On 2018-07-27 5:37 AM, Richard Earnshaw wrote:

>> Port Maintainers: You need to decide what action is required for your

>> port to handle speculative execution, even if that action is to use

>> the trivial no-speculation on this architecture.  You must also

>> consider whether or not a furture implementation of your architecture

>> might need to deal with this in making that decision.

> On hppa, I think we should go with the hook that assumes there is no

> speculative execution.

> 

> Nominally, there is branch prediction and speculative execution; but the

> spectre test program

> was not able to successfully access memory on my rp3440.

> 

> As far as I know, the details of speculative execution on PA-RISC are

> not public.  Jeff would know

It's been eons.   I think there's enough building blocks on the PA to
mount a spectre v1 attack.  They've got branch prediction with varying
degress of speculative execution, caches and user accessable cycle timers.

There's varying degrees of out of order execution all the way back in
the PA7xxx processors (hit-under-miss) to full o-o-o execution in the
PA8xxx series (including the PA8900 that's in the rp3440).

I suspect that given enough time we could figure out why the test didn't
indicate spectre v1 vulnerability on your system and twiddle it, but
given it's a dead processor, I doubt it's worth the effort.

jeff
John David Anglin Aug. 2, 2018, 8:19 p.m. | #3
On 2018-08-02 2:40 PM, Jeff Law wrote:
> It's been eons.   I think there's enough building blocks on the PA to

> mount a spectre v1 attack.  They've got branch prediction with varying

> degress of speculative execution, caches and user accessable cycle timers.

Yes.
>

> There's varying degrees of out of order execution all the way back in

> the PA7xxx processors (hit-under-miss) to full o-o-o execution in the

> PA8xxx series (including the PA8900 that's in the rp3440).

However, as far as I know, loads and stores are always ordered.
>

> I suspect that given enough time we could figure out why the test didn't

> indicate spectre v1 vulnerability on your system and twiddle it, but

> given it's a dead processor, I doubt it's worth the effort.

Spectre output looks like this:
dave@mx3210:~/meltdown$ ./spectre
Reading 40 bytes:
Reading at malicious_x = 0xffffef10... Unclear: 0xFE='?' score=999    
(second best: 0xFC score=999)
Reading at malicious_x = 0xffffef11... Unclear: 0xFC='?' score=999    
(second best: 0xFB score=999)
Reading at malicious_x = 0xffffef12... Unclear: 0xFE='?' score=999    
(second best: 0xFC score=999)

I don't think there's a suitable barrier.  The sync instruction seems 
like overkill.

So, I'm going to install attached change after testing is complete.

Dave

-- 
John David Anglin  dave.anglin@bell.net
Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c	(revision 263228)
+++ config/pa/pa.c	(working copy)
@@ -428,6 +428,9 @@
 #undef TARGET_STARTING_FRAME_OFFSET
 #define TARGET_STARTING_FRAME_OFFSET pa_starting_frame_offset
 
+#undef TARGET_HAVE_SPECULATION_SAFE_VALUE
+#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 /* Parse the -mfixed-range= option string.  */
Richard Earnshaw (lists) Aug. 3, 2018, 9:06 a.m. | #4
On 02/08/18 21:19, John David Anglin wrote:
> On 2018-08-02 2:40 PM, Jeff Law wrote:

>> It's been eons.   I think there's enough building blocks on the PA to

>> mount a spectre v1 attack.  They've got branch prediction with varying

>> degress of speculative execution, caches and user accessable cycle

>> timers.

> Yes.

>>

>> There's varying degrees of out of order execution all the way back in

>> the PA7xxx processors (hit-under-miss) to full o-o-o execution in the

>> PA8xxx series (including the PA8900 that's in the rp3440).

> However, as far as I know, loads and stores are always ordered.

>>

>> I suspect that given enough time we could figure out why the test didn't

>> indicate spectre v1 vulnerability on your system and twiddle it, but

>> given it's a dead processor, I doubt it's worth the effort.

> Spectre output looks like this:

> dave@mx3210:~/meltdown$ ./spectre

> Reading 40 bytes:

> Reading at malicious_x = 0xffffef10... Unclear: 0xFE='?' score=999   

> (second best: 0xFC score=999)

> Reading at malicious_x = 0xffffef11... Unclear: 0xFC='?' score=999   

> (second best: 0xFB score=999)

> Reading at malicious_x = 0xffffef12... Unclear: 0xFE='?' score=999   

> (second best: 0xFC score=999)

> 

> I don't think there's a suitable barrier.  The sync instruction seems

> like overkill.

> 

> So, I'm going to install attached change after testing is complete.

> 


It's your call as port maintainers.

I've created a PR for each unfixed architecture.  Please can you commit
the patch against that so that I can track things for back-porting.

Thanks,

R.

> Dave

> 

> 

> pa-spectre.d

> 

> 

> Index: config/pa/pa.c

> ===================================================================

> --- config/pa/pa.c	(revision 263228)

> +++ config/pa/pa.c	(working copy)

> @@ -428,6 +428,9 @@

>  #undef TARGET_STARTING_FRAME_OFFSET

>  #define TARGET_STARTING_FRAME_OFFSET pa_starting_frame_offset

>  

> +#undef TARGET_HAVE_SPECULATION_SAFE_VALUE

> +#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed

> +

>  struct gcc_target targetm = TARGET_INITIALIZER;

>  

>  /* Parse the -mfixed-range= option string.  */

>
Jeff Law Aug. 3, 2018, 5:26 p.m. | #5
On 08/02/2018 02:19 PM, John David Anglin wrote:
> On 2018-08-02 2:40 PM, Jeff Law wrote:

>> It's been eons.   I think there's enough building blocks on the PA to

>> mount a spectre v1 attack.  They've got branch prediction with varying

>> degress of speculative execution, caches and user accessable cycle

>> timers.

> Yes.

>>

>> There's varying degrees of out of order execution all the way back in

>> the PA7xxx processors (hit-under-miss) to full o-o-o execution in the

>> PA8xxx series (including the PA8900 that's in the rp3440).

> However, as far as I know, loads and stores are always ordered.

I'm pretty sure that's not true on PA8000 class machines:

You can get the details here:

http://web.archive.org/web/20040214092531/http://www.cpus.hp.com/technical_references/advperf.shtml

It describes in reasonable detail how the load/store reorder buffer and
the address reorder buffer works as well as the tag checking to detect
when a speculative load was executed and its results had to be thrown
away due to a store-to-load dependency check in the ARB.

But again, given the state of the target, I'm not at all concerned about
mitigating spectre v1.

Jeff
John David Anglin Aug. 6, 2018, 9:52 p.m. | #6
On 2018-08-03 5:06 AM, Richard Earnshaw (lists) wrote:
>> I don't think there's a suitable barrier.  The sync instruction seems

>> like overkill.

>>

>> So, I'm going to install attached change after testing is complete.

>>

> It's your call as port maintainers.

I committed the attached change after testing on hppa-unknown-linux-gnu.

Dave

-- 
John David Anglin  dave.anglin@bell.net
2018-08-06  John David Anglin  <danglin@gcc.gnu.org>

	PR target/86807
	* config/pa/pa.c (TARGET_HAVE_SPECULATION_SAFE_VALUE):
	Define to speculation_safe_value_not_needed.

Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c	(revision 263228)
+++ config/pa/pa.c	(working copy)
@@ -428,6 +428,9 @@
 #undef TARGET_STARTING_FRAME_OFFSET
 #define TARGET_STARTING_FRAME_OFFSET pa_starting_frame_offset
 
+#undef TARGET_HAVE_SPECULATION_SAFE_VALUE
+#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 /* Parse the -mfixed-range= option string.  */
Richard Earnshaw (lists) Aug. 7, 2018, 2:05 p.m. | #7
On 06/08/18 22:52, John David Anglin wrote:
> On 2018-08-03 5:06 AM, Richard Earnshaw (lists) wrote:

>>> I don't think there's a suitable barrier.  The sync instruction seems

>>> like overkill.

>>>

>>> So, I'm going to install attached change after testing is complete.

>>>

>> It's your call as port maintainers.

> I committed the attached change after testing on hppa-unknown-linux-gnu.

> 


Thanks.  Wrong PR, though: that was for the SPU port.  The hppa PR is 86785.

R.

> Dave

> 

> 

> pa-spectre.d

> 

> 

> 2018-08-06  John David Anglin  <danglin@gcc.gnu.org>

> 

> 	PR target/86807

> 	* config/pa/pa.c (TARGET_HAVE_SPECULATION_SAFE_VALUE):

> 	Define to speculation_safe_value_not_needed.

> 

> Index: config/pa/pa.c

> ===================================================================

> --- config/pa/pa.c	(revision 263228)

> +++ config/pa/pa.c	(working copy)

> @@ -428,6 +428,9 @@

>  #undef TARGET_STARTING_FRAME_OFFSET

>  #define TARGET_STARTING_FRAME_OFFSET pa_starting_frame_offset

>  

> +#undef TARGET_HAVE_SPECULATION_SAFE_VALUE

> +#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed

> +

>  struct gcc_target targetm = TARGET_INITIALIZER;

>  

>  /* Parse the -mfixed-range= option string.  */

>
John David Anglin Aug. 7, 2018, 2:56 p.m. | #8
On 2018-08-07 10:05 AM, Richard Earnshaw (lists) wrote:
> Thanks.  Wrong PR, though: that was for the SPU port.  The hppa PR is 86785.

Oops, sorry for extra work.

Dave

-- 
John David Anglin  dave.anglin@bell.net