From: Richard Sandiford <richard.sandiford@arm.com>
Date: Fri, 7 Aug 2020 11:15:01 +0000 (+0100)
Subject: aarch64: Clear canary value after stack_protect_test [PR96191]
X-Git-Tag: releases/gcc-10.3.0~1081
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bab5fdaf9abb1236a7a56258d1d36265068b4827;p=thirdparty%2Fgcc.git

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.

(cherry picked from commit fe1a26429038d7cd17abc53f96a6f3e2639b605f)
---

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 13d5e88b36d9..a5d865a884ae 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -7166,10 +7166,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
@@ -7194,29 +7192,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 Floating-point 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 000000000000..73e83bc413f6
--- /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 000000000000..266c36fdbc6b
--- /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"