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.

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 25d77256b96b0e477f3efe0aedbaed965d65421f..9b20dd0b1a0e917789da1d8db1152e2bb1f1b625 100644 (file)
--- 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 (file)
index 0000000..73e83bc
--- /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 (file)
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"