[gcc(refs/vendors/ARM/heads/CVE-2023-4039/gcc-7)] aarch64: Make stack smash canary protect saved registers

[gcc(refs/vendors/ARM/heads/CVE-2023-4039/gcc-7)] aarch64: Make stack smash canary protect saved registers

[Richard Sandiford]

https://gcc.gnu.org/g:c2c772172f0478315ab94386ebf9963efd36eb75

commit c2c772172f0478315ab94386ebf9963efd36eb75
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Thu Jun 15 19:16:52 2023 +0100

    aarch64: Make stack smash canary protect saved registers
    
    AArch64 normally puts the saved registers near the bottom of the frame,
    immediately above any dynamic allocations.  But this means that a
    stack-smash attack on those dynamic allocations could overwrite the
    saved registers without needing to reach as far as the stack smash
    canary.
    
    The same thing could also happen for variable-sized arguments that are
    passed by value, since those are allocated before a call and popped on
    return.
    
    This patch avoids that by putting the locals (and thus the canary) below
    the saved registers when stack smash protection is active.
    
    The patch fixes CVE-2023-4039.
    
    gcc/
            * config/aarch64/aarch64.c (aarch64_save_regs_above_locals_p):
            New function.
            (aarch64_layout_frame): Use it to decide whether locals should
            go above or below the saved registers.
            (aarch64_expand_prologue): Update stack layout comment.
            Emit a stack tie after the final adjustment.
    
    gcc/testsuite/
            * gcc.target/aarch64/stack-protector-8.c: New test.
            * gcc.target/aarch64/stack-protector-9.c: Likewise.

Diff:
---
 gcc/config/aarch64/aarch64.c                       | 45 ++++++++++-
 .../gcc.target/aarch64/stack-protector-8.c         | 86 ++++++++++++++++++++++
 .../gcc.target/aarch64/stack-protector-9.c         | 33 +++++++++
 3 files changed, 161 insertions(+), 3 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e3c36abb1e35..8708b4fe4ea2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2780,6 +2780,20 @@ aarch64_frame_pointer_required (void)
   return false;
 }
 
+/* Return true if the current function should save registers above
+   the locals area, rather than below it.  */
+
+static bool
+aarch64_save_regs_above_locals_p ()
+{
+  /* When using stack smash protection, make sure that the canary slot
+     comes between the locals and the saved registers.  Otherwise,
+     it would be possible for a carefully sized smash attack to change
+     the saved registers (particularly LR and FP) without reaching the
+     canary.  */
+  return crtl->stack_protect_guard;
+}
+
 /* Mark the registers that need to be saved by the callee and calculate
    the size of the callee-saved registers area and frame record (both FP
    and LR may be omitted).  */
@@ -2828,6 +2842,16 @@ aarch64_layout_frame (void)
 
   cfun->machine->frame.bytes_below_hard_fp = crtl->outgoing_args_size;
 
+  bool regs_at_top_p = aarch64_save_regs_above_locals_p ();
+
+  if (regs_at_top_p)
+    {
+      cfun->machine->frame.bytes_below_hard_fp += get_frame_size ();
+      cfun->machine->frame.bytes_below_hard_fp
+	= ROUND_UP (cfun->machine->frame.bytes_below_hard_fp,
+		    STACK_BOUNDARY / BITS_PER_UNIT);
+    }
+
   if (frame_pointer_needed)
     {
       /* FP and LR are placed in the linkage record.  */
@@ -2881,8 +2905,11 @@ aarch64_layout_frame (void)
   HOST_WIDE_INT varargs_and_saved_regs_size
     = offset + cfun->machine->frame.saved_varargs_size;
 
+  cfun->machine->frame.bytes_above_hard_fp = varargs_and_saved_regs_size;
+  if (!regs_at_top_p)
+    cfun->machine->frame.bytes_above_hard_fp += get_frame_size ();
   cfun->machine->frame.bytes_above_hard_fp
-    = ROUND_UP (varargs_and_saved_regs_size + get_frame_size (),
+    = ROUND_UP (cfun->machine->frame.bytes_above_hard_fp,
 		STACK_BOUNDARY / BITS_PER_UNIT);
 
   cfun->machine->frame.frame_size
@@ -2892,6 +2919,9 @@ aarch64_layout_frame (void)
 
   cfun->machine->frame.bytes_above_locals
     = cfun->machine->frame.saved_varargs_size;
+  if (regs_at_top_p)
+    cfun->machine->frame.bytes_above_locals
+      += cfun->machine->frame.saved_regs_size;
 
   cfun->machine->frame.initial_adjust = 0;
   cfun->machine->frame.final_adjust = 0;
@@ -3537,10 +3567,10 @@ aarch64_set_handled_components (sbitmap components)
 	|  for register varargs         |
 	|                               |
 	+-------------------------------+
-	|  local variables              | <-- frame_pointer_rtx
+	|  local variables (1)          | <-- frame_pointer_rtx
 	|                               |
 	+-------------------------------+
-	|  padding0                     | \
+	|  padding (1)                  | \
 	+-------------------------------+  |
 	|  callee-saved registers       |  | frame.saved_regs_size
 	+-------------------------------+  |
@@ -3548,6 +3578,10 @@ aarch64_set_handled_components (sbitmap components)
 	+-------------------------------+  |
 	|  FP'                          | / <- hard_frame_pointer_rtx (aligned)
         +-------------------------------+
+	|  local variables (2)          |
+	+-------------------------------+
+	|  padding (2)                  |
+	+-------------------------------+
 	|  dynamic allocation           |
 	+-------------------------------+
 	|  padding                      |
@@ -3557,6 +3591,9 @@ aarch64_set_handled_components (sbitmap components)
 	+-------------------------------+
 	|                               | <-- stack_pointer_rtx (aligned)
 
+   The regions marked (1) and (2) are mutually exclusive.  (2) is used
+   when aarch64_save_regs_above_locals_p is true.
+
    Dynamic stack allocations via alloca() decrease stack_pointer_rtx
    but leave frame_pointer_rtx and hard_frame_pointer_rtx
    unchanged.  */
@@ -3626,6 +3663,8 @@ aarch64_expand_prologue (void)
   aarch64_save_callee_saves (DFmode, callee_offset, V0_REGNUM, V31_REGNUM,
 			     callee_adjust != 0 || frame_pointer_needed);
   aarch64_sub_sp (IP1_REGNUM, final_adjust, !frame_pointer_needed);
+  if (frame_pointer_needed && final_adjust != 0)
+    emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
 }
 
 /* Return TRUE if we can use a simple_return insn.
diff --git a/gcc/testsuite/gcc.target/aarch64/stack-protector-8.c b/gcc/testsuite/gcc.target/aarch64/stack-protector-8.c
new file mode 100644
index 000000000000..2575abbec739
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-8.c
@@ -0,0 +1,86 @@
+/* { dg-options " -O -fstack-protector-strong" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+void g(void *);
+
+/*
+** test1:
+**	sub	sp, sp, #304
+**	stp	x29, x30, \[sp, #?272\]
+**	add	x29, sp, #?272
+**	str	(x[0-9]+), \[sp, #?288\]
+**	...
+**	ldr	(x[0-9]+), \[\1\]
+**	str	\2, \[(?:sp, #?264|x29, #?-8)\]
+**	mov	\2, *0
+** (
+**	add	x0, sp, #?8
+** |
+**	sub	x0, x29, #?264
+** )
+**	bl	g
+**	...
+**	ldr	x[0-9]+, \[\1\]
+**	...
+** (
+**	bne	.*
+** |
+**	cbnz	.*
+** )
+**	...
+**	ldr	\1, \[sp, #?288\]
+**	ldp	x29, x30, \[sp, #?272\]
+**	add	sp, sp, #?304
+**	ret
+**	bl	__stack_chk_fail
+*/
+int test1() {
+  int y[0x40];
+  g(y);
+  return 1;
+}
+
+/*
+** test2:
+**	stp	x29, x30, \[sp, #?-32\]!
+** (
+**	mov	x29, sp
+** |
+**	add	x29, sp, #?0
+** )
+**	str	(x[0-9]+), \[sp, #?16\]
+**	sub	sp, sp, #1040
+**	...
+**	ldr	(x[0-9]+), \[\1\]
+**	str	\2, \[(?:sp, #?1032|x29, #?-8)\]
+**	mov	\2, *0
+** (
+**	add	x0, sp, #?8
+** |
+**	sub	x0, x29, #?1032
+** )
+**	bl	g
+**	...
+**	ldr	x[0-9]+, \[\1\]
+**	...
+** (
+**	bne	.*
+** |
+**	cbnz	.*
+** )
+**	...
+** (
+**	add	sp, sp, #?1040
+** |
+**	add	sp, x29, #?0
+** )
+**	ldr	\1, \[sp, #?16\]
+**	ldp	x29, x30, \[sp\], #?32
+**	ret
+**	bl	__stack_chk_fail
+*/
+int test2() {
+  int y[0x100];
+  g(y);
+  return 1;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/stack-protector-9.c b/gcc/testsuite/gcc.target/aarch64/stack-protector-9.c
new file mode 100644
index 000000000000..1d2673438061
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-9.c
@@ -0,0 +1,33 @@
+/* { dg-options "-O2 -mcpu=cortex-a73 -fstack-protector-all" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** main:
+**	...
+**	stp	x29, x30, \[sp, #?-[0-9]+\]!
+**	...
+**	sub	sp, sp, #[0-9]+
+**	...
+**	str	x[0-9]+, \[x29, #?-8\]
+**	...
+*/
+int f(const char *);
+void g(void *);
+int main(int argc, char* argv[])
+{
+  int a;
+  int b;
+  char c[2+f(argv[1])];
+  int d[0x100];
+  char y;
+
+  y=42; a=4; b=10;
+  c[0] = 'h'; c[1] = '\0';
+
+  c[f(argv[2])] = '\0';
+
+  __builtin_printf("%d %d\n%s\n", a, b, c);
+  g(d);
+
+  return 0;
+}