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.

(cherry picked from commit 6a3f3e08723063ea2dadb7ddf503f02972a724e2)

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index a6a31f8f4ef8d3c8c96c61b450dbd7c28d08b55c..dd13c77e889a3d8b9c31dabdaf50cafbf63196f4 100644 (file)
--- 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 24861635fa537fbf1b957dfdcae7f090db0e3873..0ff819090d94c58dbb92d3f30159c4064319de70 100644 (file)
--- 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")]
 )
 \f

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 (file)
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 (file)
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"