[ARM] Fix PR85434: spilling of stack protector guard's address on ARM

In case of high register pressure in PIC mode, address of the stack
protector's guard can be spilled on ARM targets as shown in PR85434,
thus allowing an attacker to control what the canary would be compared
against. This is also known as CVE-2018-12886. ARM does lack
stack_protect_set and stack_protect_test insn patterns, defining them
does not help as the address is expanded regularly and the patterns
only deal with the copy and test of the guard with the canary.

This problem does not occur for x86 targets because the PIC access and
the test can be done in the same instruction. Aarch64 is exempt too
because PIC access insn pattern are mov of UNSPEC which prevents it from
the second access in the epilogue being CSEd in cse_local pass with the
first access in the prologue.

The approach followed here is to create new "combined" set and test
standard pattern names that take the unexpanded guard and do the set or
test. This allows the target to use an opaque pattern (eg. using UNSPEC)
to hide the individual instructions being generated to the compiler and
split the pattern into generic load, compare and branch instruction
after register allocator, therefore avoiding any spilling. This is here
implemented for the ARM targets. For targets not implementing these new
standard pattern names, the existing stack_protect_set and
stack_protect_test pattern names are used.

To be able to split PIC access after register allocation, the functions
had to be augmented to force a new PIC register load and to control
which register it loads into. This is because sharing the PIC register
between prologue and epilogue could lead to spilling due to CSE again
which an attacker could use to control what the canary gets compared
against.

2018-08-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    gcc/
    PR target/85434
    * target-insns.def (stack_protect_combined_set): Define new standard
    pattern name.
    (stack_protect_combined_test): Likewise.
    * cfgexpand.c (stack_protect_prologue): Try new
    stack_protect_combined_set pattern first.
    * function.c (stack_protect_epilogue): Try new
    stack_protect_combined_test pattern first.
    * config/arm/arm.c (require_pic_register): Add pic_reg and compute_now
    parameters to control which register to use as PIC register and force
    reloading PIC register respectively.  Insert in the stream of insns if
    possible.
    (legitimize_pic_address): Expose above new parameters in prototype and
    adapt recursive calls accordingly.
    (arm_legitimize_address): Adapt to new legitimize_pic_address
    prototype.
    (thumb_legitimize_address): Likewise.
    (arm_emit_call_insn): Adapt to new require_pic_register prototype.
    * config/arm/arm-protos.h (legitimize_pic_address): Adapt to prototype
    change.
    * config/arm/arm.md (movsi expander): Adapt to legitimize_pic_address
    prototype change.
    (stack_protect_combined_set): New insn_and_split pattern.
    (stack_protect_set): New insn pattern.
    (stack_protect_combined_test): New insn_and_split pattern.
    (stack_protect_test): New insn pattern.
    * config/arm/unspecs.md (UNSPEC_SP_SET): New unspec.
    (UNSPEC_SP_TEST): Likewise.
    * doc/md.texi (stack_protect_combined_set): Document new standard
    pattern name.
    (stack_protect_set): Clarify that the operand for guard's address is
    legal.
    (stack_protect_combined_test): Document new standard pattern name.
    (stack_protect_test): Clarify that the operand for guard's address is
    legal.

    gcc/testsuite/
    PR target/85434
    * gcc.target/arm/pr85434.c: New test.

From-SVN: r263245

1 files changed, 47 insertions, 8 deletions

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 00c1239..e793525 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -7377,22 +7377,61 @@ builtins.
 The get/set patterns have a single output/input operand respectively,
 with @var{mode} intended to be @code{Pmode}.
 
+@cindex @code{stack_protect_combined_set} instruction pattern
+@item @samp{stack_protect_combined_set}
+This pattern, if defined, moves a @code{ptr_mode} value from an address
+whose declaration RTX is given in operand 1 to the memory in operand 0
+without leaving the value in a register afterward.  If several
+instructions are needed by the target to perform the operation (eg. to
+load the address from a GOT entry then load the @code{ptr_mode} value
+and finally store it), it is the backend's responsibility to ensure no
+intermediate result gets spilled.  This is to avoid leaking the value
+some place that an attacker might use to rewrite the stack guard slot
+after having clobbered it.
+
+If this pattern is not defined, then the address declaration is
+expanded first in the standard way and a @code{stack_protect_set}
+pattern is then generated to move the value from that address to the
+address in operand 0.
+
 @cindex @code{stack_protect_set} instruction pattern
 @item @samp{stack_protect_set}
-This pattern, if defined, moves a @code{ptr_mode} value from the memory
-in operand 1 to the memory in operand 0 without leaving the value in
-a register afterward.  This is to avoid leaking the value some place
-that an attacker might use to rewrite the stack guard slot after
-having clobbered it.
+This pattern, if defined, moves a @code{ptr_mode} value from the valid
+memory location in operand 1 to the memory in operand 0 without leaving
+the value in a register afterward.  This is to avoid leaking the value
+some place that an attacker might use to rewrite the stack guard slot
+after having clobbered it.
+
+Note: on targets where the addressing modes do not allow to load
+directly from stack guard address, the address is expanded in a standard
+way first which could cause some spills.
 
 If this pattern is not defined, then a plain move pattern is generated.
 
+@cindex @code{stack_protect_combined_test} instruction pattern
+@item @samp{stack_protect_combined_test}
+This pattern, if defined, compares a @code{ptr_mode} value from an
+address whose declaration RTX is given in operand 1 with the memory in
+operand 0 without leaving the value in a register afterward and
+branches to operand 2 if the values were equal.  If several
+instructions are needed by the target to perform the operation (eg. to
+load the address from a GOT entry then load the @code{ptr_mode} value
+and finally store it), it is the backend's responsibility to ensure no
+intermediate result gets spilled.  This is to avoid leaking the value
+some place that an attacker might use to rewrite the stack guard slot
+after having clobbered it.
+
+If this pattern is not defined, then the address declaration is
+expanded first in the standard way and a @code{stack_protect_test}
+pattern is then generated to compare the value from that address to the
+value at the memory in operand 0.
+
 @cindex @code{stack_protect_test} instruction pattern
 @item @samp{stack_protect_test}
 This pattern, if defined, compares a @code{ptr_mode} value from the
-memory in operand 1 with the memory in operand 0 without leaving the
-value in a register afterward and branches to operand 2 if the values
-were equal.
+valid memory location in operand 1 with the memory in operand 0 without
+leaving the value in a register afterward and branches to operand 2 if
+the values were equal.
 
 If this pattern is not defined, then a plain compare pattern and
 conditional branch pattern is used.