public inbox for gcc-cvs@sourceware.org help / color / mirror / Atom feed
From: Richard Sandiford <rsandifo@gcc.gnu.org> To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/vendors/ARM/heads/CVE-2023-4039/gcc-8)] aarch64: Make stack smash canary protect saved registers Date: Tue, 12 Sep 2023 15:23:48 +0000 (GMT) [thread overview] Message-ID: <20230912152348.738383858423@sourceware.org> (raw) https://gcc.gnu.org/g:ee9111b567ddcd2f1b16da4f38b69b1b979df9ef commit ee9111b567ddcd2f1b16da4f38b69b1b979df9ef 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 | 46 ++++++++++++++-- .../gcc.target/aarch64/stack-protector-8.c | 62 ++++++++++++++++++++++ .../gcc.target/aarch64/stack-protector-9.c | 33 ++++++++++++ 3 files changed, 137 insertions(+), 4 deletions(-) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 861e928ebdc2..5c0e3fd484da 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -4078,6 +4078,20 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2) return ""; } +/* 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). */ @@ -4138,6 +4152,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 + = aligned_upper_bound (cfun->machine->frame.bytes_below_hard_fp, + STACK_BOUNDARY / BITS_PER_UNIT); + } + if (cfun->machine->frame.emit_frame_chain) { /* FP and LR are placed in the linkage record. */ @@ -4191,9 +4215,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 - = aligned_upper_bound (varargs_and_saved_regs_size - + get_frame_size (), + = aligned_upper_bound (cfun->machine->frame.bytes_above_hard_fp, STACK_BOUNDARY / BITS_PER_UNIT); /* Both these values are already aligned. */ @@ -4205,6 +4231,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; @@ -4912,10 +4941,10 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned int reg, | for register varargs | | | +-------------------------------+ - | local variables | <-- frame_pointer_rtx + | local variables (1) | <-- frame_pointer_rtx | | +-------------------------------+ - | padding0 | \ + | padding (1) | \ +-------------------------------+ | | callee-saved registers | | frame.saved_regs_size +-------------------------------+ | @@ -4923,6 +4952,10 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned int reg, +-------------------------------+ | | FP' | / <- hard_frame_pointer_rtx (aligned) +-------------------------------+ + | local variables (2) | + +-------------------------------+ + | padding (2) | + +-------------------------------+ | dynamic allocation | +-------------------------------+ | padding | @@ -4932,6 +4965,9 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned int reg, +-------------------------------+ | | <-- 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. */ @@ -5042,6 +5078,8 @@ aarch64_expand_prologue (void) aarch64_save_callee_saves (DFmode, callee_offset, V0_REGNUM, V31_REGNUM, callee_adjust != 0 || emit_frame_chain); aarch64_sub_sp (ip1_rtx, ip0_rtx, final_adjust, !frame_pointer_needed); + if (emit_frame_chain && maybe_ne (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..b808735762fd --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-8.c @@ -0,0 +1,62 @@ +/* { 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\] +** mov \2, *0 +** add x0, sp, #?8 +** bl g +** ... +** ldr x[0-9]+, \[\1\] +** ... +** bne .* +** ... +** 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 +** str (x[0-9]+), \[sp, #?16\] +** sub sp, sp, #1040 +** ... +** ldr (x[0-9]+), \[\1\] +** str \2, \[sp, #?1032\] +** mov \2, *0 +** add x0, sp, #?8 +** bl g +** ... +** ldr x[0-9]+, \[\1\] +** ... +** bne .* +** ... +** add sp, sp, #?1040 +** 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..58f322aa480a --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/stack-protector-9.c @@ -0,0 +1,33 @@ +/* { dg-options "-O2 -mcpu=neoverse-v1 -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; +}
reply other threads:[~2023-09-12 15:23 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230912152348.738383858423@sourceware.org \ --to=rsandifo@gcc.gnu.org \ --cc=gcc-cvs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).