aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRichard Sandiford <richard.sandiford@arm.com>2020-08-06 19:19:41 +0100
committerRichard Sandiford <richard.sandiford@arm.com>2020-08-06 19:19:41 +0100
commit6a3f3e08723063ea2dadb7ddf503f02972a724e2 (patch)
treef821ca505e67f83acdea248bc5439ed8e5c5b09c
parent6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184 (diff)
downloadgcc-6a3f3e08723063ea2dadb7ddf503f02972a724e2.zip
gcc-6a3f3e08723063ea2dadb7ddf503f02972a724e2.tar.gz
gcc-6a3f3e08723063ea2dadb7ddf503f02972a724e2.tar.bz2
arm: 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/arm/arm.md (arm_stack_protect_test_insn): Zero out operand 2 after use. * config/arm/thumb1.md (thumb1_stack_protect_test_insn): Likewise. gcc/testsuite/ * gcc.target/arm/stack-protector-1.c: New test. * gcc.target/arm/stack-protector-2.c: Likewise.
-rw-r--r--gcc/config/arm/arm.md6
-rw-r--r--gcc/config/arm/thumb1.md8
-rw-r--r--gcc/testsuite/gcc.target/arm/stack-protector-1.c63
-rw-r--r--gcc/testsuite/gcc.target/arm/stack-protector-2.c6
4 files changed, 78 insertions, 5 deletions
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index a6a31f8..dd13c77 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -9320,6 +9320,8 @@
[(set_attr "arch" "t1,32")]
)
+;; 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 "arm_stack_protect_test_insn"
[(set (reg:CC_Z CC_REGNUM)
(compare:CC_Z (unspec:SI [(match_operand:SI 1 "memory_operand" "m,m")
@@ -9329,8 +9331,8 @@
(clobber (match_operand:SI 0 "register_operand" "=&l,&r"))
(clobber (match_dup 2))]
"TARGET_32BIT"
- "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0"
- [(set_attr "length" "8,12")
+ "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0\;mov\t%2, #0"
+ [(set_attr "length" "12,16")
(set_attr "conds" "set")
(set_attr "type" "multiple")
(set_attr "arch" "t,32")]
diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
index 2486163..0ff8190 100644
--- a/gcc/config/arm/thumb1.md
+++ b/gcc/config/arm/thumb1.md
@@ -2020,6 +2020,8 @@
[(set_attr "type" "mov_reg")]
)
+;; 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 "thumb1_stack_protect_test_insn"
[(set (match_operand:SI 0 "register_operand" "=&l")
(unspec:SI [(match_operand:SI 1 "memory_operand" "m")
@@ -2027,9 +2029,9 @@
UNSPEC_SP_TEST))
(clobber (match_dup 2))]
"TARGET_THUMB1"
- "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0"
- [(set_attr "length" "8")
- (set_attr "conds" "set")
+ "ldr\t%0, [%2]\;ldr\t%2, %1\;eors\t%0, %2, %0\;movs\t%2, #0"
+ [(set_attr "length" "10")
+ (set_attr "conds" "clob")
(set_attr "type" "multiple")]
)
diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-1.c b/gcc/testsuite/gcc.target/arm/stack-protector-1.c
new file mode 100644
index 0000000..b03ea14
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/stack-protector-1.c
@@ -0,0 +1,63 @@
+/* { 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\tr0, " #REG "\n\tbeq\t1f\n"
+
+asm (
+" .data\n"
+" .align 3\n"
+" .globl stack_chk_guard_ptr\n"
+"stack_chk_guard_ptr:\n"
+" .word __stack_chk_guard\n"
+" .weak __stack_chk_guard\n"
+"__stack_chk_guard:\n"
+" .word 0xdead4321\n"
+" .text\n"
+" .globl main\n"
+" .type main, %function\n"
+"main:\n"
+" bl get_ptr\n"
+" str r0, [sp, #-8]!\n"
+" bl f\n"
+" str r0, [sp, #4]\n"
+" ldr r0, [sp]\n"
+" ldr r0, [r0]\n"
+ CHECK (r1)
+ CHECK (r2)
+ CHECK (r3)
+ CHECK (r4)
+ CHECK (r5)
+ CHECK (r6)
+ CHECK (r7)
+ CHECK (r8)
+ CHECK (r9)
+ CHECK (r10)
+ CHECK (r11)
+ CHECK (r12)
+ CHECK (r13)
+ CHECK (r14)
+" ldr r1, [sp, #4]\n"
+ CHECK (r1)
+" mov r0, #0\n"
+" b exit\n"
+"1:\n"
+" b abort\n"
+" .size main, .-main"
+);
diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-2.c b/gcc/testsuite/gcc.target/arm/stack-protector-2.c
new file mode 100644
index 0000000..266c36f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/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"