aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Sandiford <richard.sandiford@arm.com>2020-08-05 15:18:36 +0100
committerRichard Sandiford <richard.sandiford@arm.com>2020-08-05 15:18:36 +0100
commitfe1a26429038d7cd17abc53f96a6f3e2639b605f (patch)
treee37f7d8a433b1c8f5113745d5098845be27d223d
parent2c272091c09298eea02b6bb3b3ffd95db9ea505b (diff)
downloadgcc-fe1a26429038d7cd17abc53f96a6f3e2639b605f.zip
gcc-fe1a26429038d7cd17abc53f96a6f3e2639b605f.tar.gz
gcc-fe1a26429038d7cd17abc53f96a6f3e2639b605f.tar.bz2
aarch64: Clear canary value after stack_protect_test [PR96191]
The stack_protect_test patterns were leaving the canary value in the temporary register, meaning that it was often still in registers on return from the function. An attacker might therefore have been able to use it to defeat stack-smash protection for a later function. gcc/ PR target/96191 * config/aarch64/aarch64.md (stack_protect_test_<mode>): Set the CC register directly, instead of a GPR. Replace the original GPR destination with an extra scratch register. Zero out operand 3 after use. (stack_protect_test): Update accordingly. gcc/testsuite/ PR target/96191 * gcc.target/aarch64/stack-protector-1.c: New test. * gcc.target/aarch64/stack-protector-2.c: Likewise.
-rw-r--r--gcc/config/aarch64/aarch64.md34
-rw-r--r--gcc/testsuite/gcc.target/aarch64/stack-protector-1.c89
-rw-r--r--gcc/testsuite/gcc.target/aarch64/stack-protector-2.c6
3 files changed, 110 insertions, 19 deletions
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 25d7725..9b20dd0 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -7217,10 +7217,8 @@
(match_operand 2)]
""
{
- rtx result;
machine_mode mode = GET_MODE (operands[0]);
- result = gen_reg_rtx(mode);
if (aarch64_stack_protector_guard != SSP_GLOBAL)
{
/* Generate access through the system register. The
@@ -7245,29 +7243,27 @@
operands[1] = gen_rtx_MEM (mode, tmp_reg);
}
emit_insn ((mode == DImode
- ? gen_stack_protect_test_di
- : gen_stack_protect_test_si) (result,
- operands[0],
- operands[1]));
-
- if (mode == DImode)
- emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
- result, const0_rtx, operands[2]));
- else
- emit_jump_insn (gen_cbranchsi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
- result, const0_rtx, operands[2]));
+ ? gen_stack_protect_test_di
+ : gen_stack_protect_test_si) (operands[0], operands[1]));
+
+ rtx cc_reg = gen_rtx_REG (CCmode, CC_REGNUM);
+ emit_jump_insn (gen_condjump (gen_rtx_EQ (VOIDmode, cc_reg, const0_rtx),
+ cc_reg, operands[2]));
DONE;
})
+;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the
+;; canary value does not live beyond the end of this sequence.
(define_insn "stack_protect_test_<mode>"
- [(set (match_operand:PTR 0 "register_operand" "=r")
- (unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")
- (match_operand:PTR 2 "memory_operand" "m")]
- UNSPEC_SP_TEST))
+ [(set (reg:CC CC_REGNUM)
+ (unspec:CC [(match_operand:PTR 0 "memory_operand" "m")
+ (match_operand:PTR 1 "memory_operand" "m")]
+ UNSPEC_SP_TEST))
+ (clobber (match_scratch:PTR 2 "=&r"))
(clobber (match_scratch:PTR 3 "=&r"))]
""
- "ldr\t%<w>3, %1\;ldr\t%<w>0, %2\;eor\t%<w>0, %<w>3, %<w>0"
- [(set_attr "length" "12")
+ "ldr\t%<w>2, %0\;ldr\t%<w>3, %1\;subs\t%<w>2, %<w>2, %<w>3\;mov\t%3, 0"
+ [(set_attr "length" "16")
(set_attr "type" "multiple")])
;; Write into the Floating-point Status or Control Register.
diff --git a/gcc/testsuite/gcc.target/aarch64/stack-protector-1.c b/gcc/testsuite/gcc.target/aarch64/stack-protector-1.c
new file mode 100644
index 0000000..73e83bc4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-1.c
@@ -0,0 +1,89 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fstack_protector } */
+/* { dg-options "-fstack-protector-all -O2" } */
+
+extern volatile long *stack_chk_guard_ptr;
+
+volatile long *
+get_ptr (void)
+{
+ return stack_chk_guard_ptr;
+}
+
+void __attribute__ ((noipa))
+f (void)
+{
+ volatile int x;
+ x = 1;
+ x += 1;
+}
+
+#define CHECK(REG) "\tcmp\tx0, " #REG "\n\tbeq\t1f\n"
+
+asm (
+" .pushsection .data\n"
+" .align 3\n"
+" .globl stack_chk_guard_ptr\n"
+"stack_chk_guard_ptr:\n"
+#if __ILP32__
+" .word __stack_chk_guard\n"
+#else
+" .xword __stack_chk_guard\n"
+#endif
+" .weak __stack_chk_guard\n"
+"__stack_chk_guard:\n"
+" .word 0xdead4321\n"
+" .word 0xbeef8765\n"
+" .text\n"
+" .globl main\n"
+" .type main, %function\n"
+"main:\n"
+" bl get_ptr\n"
+" str x0, [sp, #-16]!\n"
+" bl f\n"
+" str x0, [sp, #8]\n"
+" ldr x0, [sp]\n"
+#if __ILP32__
+" ldr w0, [x0]\n"
+#else
+" ldr x0, [x0]\n"
+#endif
+ CHECK (x1)
+ CHECK (x2)
+ CHECK (x3)
+ CHECK (x4)
+ CHECK (x5)
+ CHECK (x6)
+ CHECK (x7)
+ CHECK (x8)
+ CHECK (x9)
+ CHECK (x10)
+ CHECK (x11)
+ CHECK (x12)
+ CHECK (x13)
+ CHECK (x14)
+ CHECK (x15)
+ CHECK (x16)
+ CHECK (x17)
+ CHECK (x18)
+ CHECK (x19)
+ CHECK (x20)
+ CHECK (x21)
+ CHECK (x22)
+ CHECK (x23)
+ CHECK (x24)
+ CHECK (x25)
+ CHECK (x26)
+ CHECK (x27)
+ CHECK (x28)
+ CHECK (x29)
+ CHECK (x30)
+" ldr x1, [sp]\n"
+ CHECK (x1)
+" mov x0, #0\n"
+" b exit\n"
+"1:\n"
+" b abort\n"
+" .size main, .-main\n"
+" .popsection"
+);
diff --git a/gcc/testsuite/gcc.target/aarch64/stack-protector-2.c b/gcc/testsuite/gcc.target/aarch64/stack-protector-2.c
new file mode 100644
index 0000000..266c36f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-2.c
@@ -0,0 +1,6 @@
+/* { dg-do run } */
+/* { dg-require-effective-target fstack_protector } */
+/* { dg-require-effective-target fpic } */
+/* { dg-options "-fstack-protector-all -O2 -fpic" } */
+
+#include "stack-protector-1.c"