* [PATCH v5 0/1] implement TLS register based stack canary for ARM @ 2021-11-15 18:04 Ard Biesheuvel 2021-11-15 18:04 ` [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2021-11-15 18:04 UTC (permalink / raw) To: linux-hardening Cc: Ard Biesheuvel, Keith Packard, thomas.preudhomme, adhemerval.zanella, Qing Zhao, Richard Sandiford, Kyrylo Tkachov, gcc-patches Bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102352 In the Linux kernel, user processes calling into the kernel are essentially threads running in the same address space, of a program that never terminates. This means that using a global variable for the stack protector canary value is problematic on SMP systems, as we can never change it unless we reboot the system. (Processes that sleep for any reason will do so on a call into the kernel, which means that there will always be live kernel stack frames carrying copies of the canary taken when the function was entered) AArch64 implements -mstack-protector-guard=sysreg for this purpose, as this permits the kernel to use different memory addresses for the stack canary for each CPU, and context switch the chosen system register with the rest of the process, allowing each process to use its own unique value for the stack canary. This patch implements something similar, but for the 32-bit ARM kernel, which will start using the user space TLS register TPIDRURO to index per-process metadata while running in the kernel. This means we can just add an offset to TPIDRURO to obtain the address from which to load the canary value. Changes since v4: - add a couple of test cases - incorporate feedback received from Qing and Kyrylo Changes since v3: - force a reload of the TLS register before performing the stack protector check, so that we never rely on the stack for the address of the canary Changes since v2: - fix the template for stack_protect_test_tls so it correctly conveys the fact that it sets the Z flag Cc: Keith Packard <keithpac@amazon.com> Cc: thomas.preudhomme@celest.fr Cc: adhemerval.zanella@linaro.org Cc: Qing Zhao <qing.zhao@oracle.com> Cc: Richard Sandiford <richard.sandiford@arm.com> Cc: Kyrylo Tkachov <kyryo.tkachov@arm.com> Cc: gcc-patches@gcc.gnu.org Ard Biesheuvel (1): [ARM] Add support for TLS register based stack protector canary access gcc/config/arm/arm-opts.h | 6 ++ gcc/config/arm/arm-protos.h | 2 + gcc/config/arm/arm.c | 55 +++++++++++++++ gcc/config/arm/arm.md | 71 +++++++++++++++++++- gcc/config/arm/arm.opt | 22 ++++++ gcc/doc/invoke.texi | 11 +++ gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ 8 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/stack-protector-7.c create mode 100644 gcc/testsuite/gcc.target/arm/stack-protector-8.c -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2021-11-15 18:04 [PATCH v5 0/1] implement TLS register based stack canary for ARM Ard Biesheuvel @ 2021-11-15 18:04 ` Ard Biesheuvel 2021-11-17 17:12 ` Ard Biesheuvel 2022-01-19 16:54 ` Kyrylo Tkachov 0 siblings, 2 replies; 8+ messages in thread From: Ard Biesheuvel @ 2021-11-15 18:04 UTC (permalink / raw) To: linux-hardening Cc: Ard Biesheuvel, Keith Packard, thomas.preudhomme, adhemerval.zanella, Qing Zhao, Richard Sandiford, Kyrylo Tkachov, gcc-patches Add support for accessing the stack canary value via the TLS register, so that multiple threads running in the same address space can use distinct canary values. This is intended for the Linux kernel running in SMP mode, where processes entering the kernel are essentially threads running the same program concurrently: using a global variable for the canary in that context is problematic because it can never be rotated, and so the OS is forced to use the same value as long as it remains up. Using the TLS register to index the stack canary helps with this, as it allows each CPU to context switch the TLS register along with the rest of the process, permitting each process to use its own value for the stack canary. 2021-11-15 Ard Biesheuvel <ardb@kernel.org> * config/arm/arm-opts.h (enum stack_protector_guard): New * config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem): New * config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define (arm_option_override_internal): Handle and put in error checks for stack protector guard options. (arm_option_reconfigure_globals): Likewise (arm_stack_protect_tls_canary_mem): New (arm_stack_protect_guard): New * config/arm/arm.md (stack_protect_set): New (stack_protect_set_tls): Likewise (stack_protect_test): Likewise (stack_protect_test_tls): Likewise (reload_tp_hard): Likewise * config/arm/arm.opt (-mstack-protector-guard): New (-mstack-protector-guard-offset): New. * doc/invoke.texi: Document new options gcc/testsuite/ChangeLog: * gcc.target/arm/stack-protector-7.c: New test. * gcc.target/arm/stack-protector-8.c: New test. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- gcc/config/arm/arm-opts.h | 6 ++ gcc/config/arm/arm-protos.h | 2 + gcc/config/arm/arm.c | 55 +++++++++++++++ gcc/config/arm/arm.md | 71 +++++++++++++++++++- gcc/config/arm/arm.opt | 22 ++++++ gcc/doc/invoke.texi | 11 +++ gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ 8 files changed, 180 insertions(+), 2 deletions(-) diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h index 5c4b62f404f7..581ba3c4fbbb 100644 --- a/gcc/config/arm/arm-opts.h +++ b/gcc/config/arm/arm-opts.h @@ -69,4 +69,10 @@ enum arm_tls_type { TLS_GNU, TLS_GNU2 }; + +/* Where to get the canary for the stack protector. */ +enum stack_protector_guard { + SSP_TLSREG, /* per-thread canary in TLS register */ + SSP_GLOBAL /* global canary */ +}; #endif diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index 9b1f61394ad7..d8d605920c97 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -195,6 +195,8 @@ extern void arm_split_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx, rtx); extern rtx arm_load_tp (rtx); extern bool arm_coproc_builtin_available (enum unspecv); extern bool arm_coproc_ldc_stc_legitimate_address (rtx); +extern rtx arm_stack_protect_tls_canary_mem (bool); + #if defined TREE_CODE extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index a5b403eb3e49..e5077348ce07 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -829,6 +829,9 @@ static const struct attribute_spec arm_attribute_table[] = #undef TARGET_MD_ASM_ADJUST #define TARGET_MD_ASM_ADJUST arm_md_asm_adjust + +#undef TARGET_STACK_PROTECT_GUARD +#define TARGET_STACK_PROTECT_GUARD arm_stack_protect_guard \f /* Obstack for minipool constant handling. */ static struct obstack minipool_obstack; @@ -3176,6 +3179,26 @@ arm_option_override_internal (struct gcc_options *opts, if (TARGET_THUMB2_P (opts->x_target_flags)) opts->x_inline_asm_unified = true; + if (arm_stack_protector_guard == SSP_GLOBAL + && opts->x_arm_stack_protector_guard_offset_str) + { + error ("incompatible options %'-mstack-protector-guard=global%' and" + "%'-mstack-protector-guard-offset=%qs%'", + arm_stack_protector_guard_offset_str); + } + + if (opts->x_arm_stack_protector_guard_offset_str) + { + char *end; + const char *str = arm_stack_protector_guard_offset_str; + errno = 0; + long offs = strtol (arm_stack_protector_guard_offset_str, &end, 0); + if (!*str || *end || errno) + error ("%qs is not a valid offset in %qs", str, + "-mstack-protector-guard-offset="); + arm_stack_protector_guard_offset = offs; + } + #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; #endif @@ -3843,6 +3866,9 @@ arm_option_reconfigure_globals (void) else target_thread_pointer = TP_SOFT; } + + if (!TARGET_HARD_TP && arm_stack_protector_guard == SSP_TLSREG) + error("%'-mstack-protector-guard=tls%' needs a hardware TLS register"); } /* Perform some validation between the desired architecture and the rest of the @@ -8108,6 +8134,23 @@ legitimize_pic_address (rtx orig, machine_mode mode, rtx reg, rtx pic_reg, } +/* Generate insns that produce the address of the stack canary */ +rtx +arm_stack_protect_tls_canary_mem (bool reload) +{ + rtx tp = gen_reg_rtx (SImode); + if (reload) + emit_insn (gen_reload_tp_hard (tp)); + else + emit_insn (gen_load_tp_hard (tp)); + + rtx reg = gen_reg_rtx (SImode); + rtx offset = GEN_INT (arm_stack_protector_guard_offset); + emit_set_insn (reg, gen_rtx_PLUS (SImode, tp, offset)); + return gen_rtx_MEM (SImode, reg); +} + + /* Whether a register is callee saved or not. This is necessary because high registers are marked as caller saved when optimizing for size on Thumb-1 targets despite being callee saved in order to avoid using them. */ @@ -34075,6 +34118,18 @@ arm_run_selftests (void) #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests #endif /* CHECKING_P */ +/* Implement TARGET_STACK_PROTECT_GUARD. In case of a + global variable based guard use the default else + return a null tree. */ +static tree +arm_stack_protect_guard (void) +{ + if (arm_stack_protector_guard == SSP_GLOBAL) + return default_stack_protect_guard (); + + return NULL_TREE; +} + /* Worker function for TARGET_MD_ASM_ADJUST, while in thumb1 mode. Unlike the arm version, we do NOT implement asm flag outputs. */ diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 4adc976b8b67..45a54d63f5b3 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -9183,7 +9183,7 @@ (define_expand "stack_protect_combined_set" UNSPEC_SP_SET)) (clobber (match_scratch:SI 2 "")) (clobber (match_scratch:SI 3 ""))])] - "" + "arm_stack_protector_guard == SSP_GLOBAL" "" ) @@ -9267,7 +9267,7 @@ (define_expand "stack_protect_combined_test" (clobber (match_scratch:SI 3 "")) (clobber (match_scratch:SI 4 "")) (clobber (reg:CC CC_REGNUM))])] - "" + "arm_stack_protector_guard == SSP_GLOBAL" "" ) @@ -9361,6 +9361,64 @@ (define_insn "arm_stack_protect_test_insn" (set_attr "arch" "t,32")] ) +(define_expand "stack_protect_set" + [(match_operand:SI 0 "memory_operand") + (match_operand:SI 1 "memory_operand")] + "arm_stack_protector_guard == SSP_TLSREG" + " +{ + operands[1] = arm_stack_protect_tls_canary_mem (false /* reload */); + emit_insn (gen_stack_protect_set_tls (operands[0], operands[1])); + DONE; +}" +) + +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the +;; canary value does not live beyond the life of this sequence. +(define_insn "stack_protect_set_tls" + [(set (match_operand:SI 0 "memory_operand" "=m") + (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] + UNSPEC_SP_SET)) + (set (match_scratch:SI 2 "=&r") (const_int 0))] + "" + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, #0" + [(set_attr "length" "12") + (set_attr "conds" "unconditional") + (set_attr "type" "multiple")] +) + +(define_expand "stack_protect_test" + [(match_operand:SI 0 "memory_operand") + (match_operand:SI 1 "memory_operand") + (match_operand:SI 2)] + "arm_stack_protector_guard == SSP_TLSREG" + " +{ + operands[1] = arm_stack_protect_tls_canary_mem (true /* reload */); + emit_insn (gen_stack_protect_test_tls (operands[0], operands[1])); + + rtx cc_reg = gen_rtx_REG (CC_Zmode, CC_REGNUM); + rtx eq = gen_rtx_EQ (CC_Zmode, cc_reg, const0_rtx); + emit_jump_insn (gen_arm_cond_branch (operands[2], eq, cc_reg)); + DONE; +}" +) + +(define_insn "stack_protect_test_tls" + [(set (reg:CC_Z CC_REGNUM) + (compare:CC_Z (unspec:SI [(match_operand:SI 0 "memory_operand" "m") + (match_operand:SI 1 "memory_operand" "m")] + UNSPEC_SP_TEST) + (const_int 0))) + (clobber (match_scratch:SI 2 "=&r")) + (clobber (match_scratch:SI 3 "=&r"))] + "" + "ldr\t%2, %0\;ldr\t%3, %1\;eors\t%2, %3, %2\;mov\t%3, #0" + [(set_attr "length" "16") + (set_attr "conds" "set") + (set_attr "type" "multiple")] +) + (define_expand "casesi" [(match_operand:SI 0 "s_register_operand") ; index to jump on (match_operand:SI 1 "const_int_operand") ; lower bound @@ -12133,6 +12191,15 @@ (define_insn "load_tp_hard" (set_attr "type" "mrs")] ) +;; Used by the TLS register based stack protector +(define_insn "reload_tp_hard" + [(set (match_operand:SI 0 "register_operand" "=r") + (unspec_volatile:SI [(const_int 0)] VUNSPEC_MRC))] + "TARGET_HARD_TP" + "mrc\\tp15, 0, %0, c13, c0, 3\\t@ reload_tp_hard" + [(set_attr "type" "mrs")] +) + ;; Doesn't clobber R1-R3. Must use r0 for the first operand. (define_insn "load_tp_soft_fdpic" [(set (reg:SI 0) (unspec:SI [(const_int 0)] UNSPEC_TLS)) diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt index a7677eeb45c8..4b3e17bc319c 100644 --- a/gcc/config/arm/arm.opt +++ b/gcc/config/arm/arm.opt @@ -311,3 +311,25 @@ Generate code which uses the core registers only (r0-r14). mfdpic Target Mask(FDPIC) Enable Function Descriptor PIC mode. + +mstack-protector-guard= +Target RejectNegative Joined Enum(stack_protector_guard) Var(arm_stack_protector_guard) Init(SSP_GLOBAL) +Use given stack-protector guard. + +Enum +Name(stack_protector_guard) Type(enum stack_protector_guard) +Valid arguments to -mstack-protector-guard=: + +EnumValue +Enum(stack_protector_guard) String(tls) Value(SSP_TLSREG) + +EnumValue +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL) + +mstack-protector-guard-offset= +Target Joined RejectNegative String Var(arm_stack_protector_guard_offset_str) +Use an immediate to offset from the TLS register. This option is for use with +fstack-protector-guard=tls and not for use in user-land code. + +TargetVariable +long arm_stack_protector_guard_offset = 0 diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 6070288856c0..13bebbcbf0fb 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -816,6 +816,7 @@ Objective-C and Objective-C++ Dialects}. -mpure-code @gol -mcmse @gol -mfix-cmse-cve-2021-35465 @gol +-mstack-protector-guard=@var{guard} -mstack-protector-guard-offset=@var{offset} @gol -mfdpic} @emph{AVR Options} @@ -21099,6 +21100,16 @@ enabled by default when the option @option{-mcpu=} is used with @code{cortex-m33}, @code{cortex-m35p} or @code{cortex-m55}. The option @option{-mno-fix-cmse-cve-2021-35465} can be used to disable the mitigation. +@item -mstack-protector-guard=@var{guard} +@itemx -mstack-protector-guard-offset=@var{offset} +@opindex mstack-protector-guard +@opindex mstack-protector-guard-offset +Generate stack protection code using canary at @var{guard}. Supported +locations are @samp{global} for a global canary or @samp{tls} for a +canary accessible via the TLS register. The option +@option{-mstack-protector-guard-offset=} is for use with +@option{-fstack-protector-guard=tls} and not for use in user-land code. + @item -mfdpic @itemx -mno-fdpic @opindex mfdpic diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-7.c b/gcc/testsuite/gcc.target/arm/stack-protector-7.c new file mode 100644 index 000000000000..874648060d8d --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/stack-protector-7.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv7-a -mfpu=vfp -fstack-protector-all -Os -mstack-protector-guard=tls -mstack-protector-guard-offset=1296 -mtp=cp15" } */ + +#include "stack-protector-5.c" + +/* See the comment in stack-protector-5.c. */ +/* { dg-final { scan-assembler-times {\tstr\t} 1 } } */ +/* Expect two TLS register accesses and two occurrences of the offset */ +/* { dg-final { scan-assembler-times {\tmrc\t} 2 } } */ +/* { dg-final { scan-assembler-times {1296} 2 } } */ diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-8.c b/gcc/testsuite/gcc.target/arm/stack-protector-8.c new file mode 100644 index 000000000000..bcb2f6b82bdf --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/stack-protector-8.c @@ -0,0 +1,5 @@ +/* { dg-do compile } */ +/* { dg-error "needs a hardware TLS register" "missing error when using TLS stack protector without hardware TLS register" { target *-*-* } 0 } */ +/* { dg-options "-fstack-protector-all -Os -mstack-protector-guard=tls -mtp=soft" } */ + +int foo; -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2021-11-15 18:04 ` [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access Ard Biesheuvel @ 2021-11-17 17:12 ` Ard Biesheuvel 2022-01-12 18:29 ` Ard Biesheuvel 2022-01-19 16:54 ` Kyrylo Tkachov 1 sibling, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2021-11-17 17:12 UTC (permalink / raw) To: linux-hardening, Ramana Radhakrishnan Cc: Keith Packard, thomas.preudhomme, adhemerval.zanella, Qing Zhao, Richard Sandiford, Kyrylo Tkachov, gcc-patches (+ Ramana) On Mon, 15 Nov 2021 at 19:04, Ard Biesheuvel <ardb@kernel.org> wrote: > > Add support for accessing the stack canary value via the TLS register, > so that multiple threads running in the same address space can use > distinct canary values. This is intended for the Linux kernel running in > SMP mode, where processes entering the kernel are essentially threads > running the same program concurrently: using a global variable for the > canary in that context is problematic because it can never be rotated, > and so the OS is forced to use the same value as long as it remains up. > > Using the TLS register to index the stack canary helps with this, as it > allows each CPU to context switch the TLS register along with the rest > of the process, permitting each process to use its own value for the > stack canary. > > 2021-11-15 Ard Biesheuvel <ardb@kernel.org> > > * config/arm/arm-opts.h (enum stack_protector_guard): New > * config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem): > New > * config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define > (arm_option_override_internal): Handle and put in error checks > for stack protector guard options. > (arm_option_reconfigure_globals): Likewise > (arm_stack_protect_tls_canary_mem): New > (arm_stack_protect_guard): New > * config/arm/arm.md (stack_protect_set): New > (stack_protect_set_tls): Likewise > (stack_protect_test): Likewise > (stack_protect_test_tls): Likewise > (reload_tp_hard): Likewise > * config/arm/arm.opt (-mstack-protector-guard): New > (-mstack-protector-guard-offset): New. > * doc/invoke.texi: Document new options > > gcc/testsuite/ChangeLog: > > * gcc.target/arm/stack-protector-7.c: New test. > * gcc.target/arm/stack-protector-8.c: New test. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > gcc/config/arm/arm-opts.h | 6 ++ > gcc/config/arm/arm-protos.h | 2 + > gcc/config/arm/arm.c | 55 +++++++++++++++ > gcc/config/arm/arm.md | 71 +++++++++++++++++++- > gcc/config/arm/arm.opt | 22 ++++++ > gcc/doc/invoke.texi | 11 +++ > gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ > gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ > 8 files changed, 180 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h > index 5c4b62f404f7..581ba3c4fbbb 100644 > --- a/gcc/config/arm/arm-opts.h > +++ b/gcc/config/arm/arm-opts.h > @@ -69,4 +69,10 @@ enum arm_tls_type { > TLS_GNU, > TLS_GNU2 > }; > + > +/* Where to get the canary for the stack protector. */ > +enum stack_protector_guard { > + SSP_TLSREG, /* per-thread canary in TLS register */ > + SSP_GLOBAL /* global canary */ > +}; > #endif > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index 9b1f61394ad7..d8d605920c97 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -195,6 +195,8 @@ extern void arm_split_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx, rtx); > extern rtx arm_load_tp (rtx); > extern bool arm_coproc_builtin_available (enum unspecv); > extern bool arm_coproc_ldc_stc_legitimate_address (rtx); > +extern rtx arm_stack_protect_tls_canary_mem (bool); > + > > #if defined TREE_CODE > extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index a5b403eb3e49..e5077348ce07 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -829,6 +829,9 @@ static const struct attribute_spec arm_attribute_table[] = > > #undef TARGET_MD_ASM_ADJUST > #define TARGET_MD_ASM_ADJUST arm_md_asm_adjust > + > +#undef TARGET_STACK_PROTECT_GUARD > +#define TARGET_STACK_PROTECT_GUARD arm_stack_protect_guard > > /* Obstack for minipool constant handling. */ > static struct obstack minipool_obstack; > @@ -3176,6 +3179,26 @@ arm_option_override_internal (struct gcc_options *opts, > if (TARGET_THUMB2_P (opts->x_target_flags)) > opts->x_inline_asm_unified = true; > > + if (arm_stack_protector_guard == SSP_GLOBAL > + && opts->x_arm_stack_protector_guard_offset_str) > + { > + error ("incompatible options %'-mstack-protector-guard=global%' and" > + "%'-mstack-protector-guard-offset=%qs%'", > + arm_stack_protector_guard_offset_str); > + } > + > + if (opts->x_arm_stack_protector_guard_offset_str) > + { > + char *end; > + const char *str = arm_stack_protector_guard_offset_str; > + errno = 0; > + long offs = strtol (arm_stack_protector_guard_offset_str, &end, 0); > + if (!*str || *end || errno) > + error ("%qs is not a valid offset in %qs", str, > + "-mstack-protector-guard-offset="); > + arm_stack_protector_guard_offset = offs; > + } > + > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; > #endif > @@ -3843,6 +3866,9 @@ arm_option_reconfigure_globals (void) > else > target_thread_pointer = TP_SOFT; > } > + > + if (!TARGET_HARD_TP && arm_stack_protector_guard == SSP_TLSREG) > + error("%'-mstack-protector-guard=tls%' needs a hardware TLS register"); > } > > /* Perform some validation between the desired architecture and the rest of the > @@ -8108,6 +8134,23 @@ legitimize_pic_address (rtx orig, machine_mode mode, rtx reg, rtx pic_reg, > } > > > +/* Generate insns that produce the address of the stack canary */ > +rtx > +arm_stack_protect_tls_canary_mem (bool reload) > +{ > + rtx tp = gen_reg_rtx (SImode); > + if (reload) > + emit_insn (gen_reload_tp_hard (tp)); > + else > + emit_insn (gen_load_tp_hard (tp)); > + > + rtx reg = gen_reg_rtx (SImode); > + rtx offset = GEN_INT (arm_stack_protector_guard_offset); > + emit_set_insn (reg, gen_rtx_PLUS (SImode, tp, offset)); > + return gen_rtx_MEM (SImode, reg); > +} > + > + > /* Whether a register is callee saved or not. This is necessary because high > registers are marked as caller saved when optimizing for size on Thumb-1 > targets despite being callee saved in order to avoid using them. */ > @@ -34075,6 +34118,18 @@ arm_run_selftests (void) > #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests > #endif /* CHECKING_P */ > > +/* Implement TARGET_STACK_PROTECT_GUARD. In case of a > + global variable based guard use the default else > + return a null tree. */ > +static tree > +arm_stack_protect_guard (void) > +{ > + if (arm_stack_protector_guard == SSP_GLOBAL) > + return default_stack_protect_guard (); > + > + return NULL_TREE; > +} > + > /* Worker function for TARGET_MD_ASM_ADJUST, while in thumb1 mode. > Unlike the arm version, we do NOT implement asm flag outputs. */ > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 4adc976b8b67..45a54d63f5b3 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -9183,7 +9183,7 @@ (define_expand "stack_protect_combined_set" > UNSPEC_SP_SET)) > (clobber (match_scratch:SI 2 "")) > (clobber (match_scratch:SI 3 ""))])] > - "" > + "arm_stack_protector_guard == SSP_GLOBAL" > "" > ) > > @@ -9267,7 +9267,7 @@ (define_expand "stack_protect_combined_test" > (clobber (match_scratch:SI 3 "")) > (clobber (match_scratch:SI 4 "")) > (clobber (reg:CC CC_REGNUM))])] > - "" > + "arm_stack_protector_guard == SSP_GLOBAL" > "" > ) > > @@ -9361,6 +9361,64 @@ (define_insn "arm_stack_protect_test_insn" > (set_attr "arch" "t,32")] > ) > > +(define_expand "stack_protect_set" > + [(match_operand:SI 0 "memory_operand") > + (match_operand:SI 1 "memory_operand")] > + "arm_stack_protector_guard == SSP_TLSREG" > + " > +{ > + operands[1] = arm_stack_protect_tls_canary_mem (false /* reload */); > + emit_insn (gen_stack_protect_set_tls (operands[0], operands[1])); > + DONE; > +}" > +) > + > +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the > +;; canary value does not live beyond the life of this sequence. > +(define_insn "stack_protect_set_tls" > + [(set (match_operand:SI 0 "memory_operand" "=m") > + (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] > + UNSPEC_SP_SET)) > + (set (match_scratch:SI 2 "=&r") (const_int 0))] > + "" > + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, #0" > + [(set_attr "length" "12") > + (set_attr "conds" "unconditional") > + (set_attr "type" "multiple")] > +) > + > +(define_expand "stack_protect_test" > + [(match_operand:SI 0 "memory_operand") > + (match_operand:SI 1 "memory_operand") > + (match_operand:SI 2)] > + "arm_stack_protector_guard == SSP_TLSREG" > + " > +{ > + operands[1] = arm_stack_protect_tls_canary_mem (true /* reload */); > + emit_insn (gen_stack_protect_test_tls (operands[0], operands[1])); > + > + rtx cc_reg = gen_rtx_REG (CC_Zmode, CC_REGNUM); > + rtx eq = gen_rtx_EQ (CC_Zmode, cc_reg, const0_rtx); > + emit_jump_insn (gen_arm_cond_branch (operands[2], eq, cc_reg)); > + DONE; > +}" > +) > + > +(define_insn "stack_protect_test_tls" > + [(set (reg:CC_Z CC_REGNUM) > + (compare:CC_Z (unspec:SI [(match_operand:SI 0 "memory_operand" "m") > + (match_operand:SI 1 "memory_operand" "m")] > + UNSPEC_SP_TEST) > + (const_int 0))) > + (clobber (match_scratch:SI 2 "=&r")) > + (clobber (match_scratch:SI 3 "=&r"))] > + "" > + "ldr\t%2, %0\;ldr\t%3, %1\;eors\t%2, %3, %2\;mov\t%3, #0" > + [(set_attr "length" "16") > + (set_attr "conds" "set") > + (set_attr "type" "multiple")] > +) > + > (define_expand "casesi" > [(match_operand:SI 0 "s_register_operand") ; index to jump on > (match_operand:SI 1 "const_int_operand") ; lower bound > @@ -12133,6 +12191,15 @@ (define_insn "load_tp_hard" > (set_attr "type" "mrs")] > ) > > +;; Used by the TLS register based stack protector > +(define_insn "reload_tp_hard" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (unspec_volatile:SI [(const_int 0)] VUNSPEC_MRC))] > + "TARGET_HARD_TP" > + "mrc\\tp15, 0, %0, c13, c0, 3\\t@ reload_tp_hard" > + [(set_attr "type" "mrs")] > +) > + > ;; Doesn't clobber R1-R3. Must use r0 for the first operand. > (define_insn "load_tp_soft_fdpic" > [(set (reg:SI 0) (unspec:SI [(const_int 0)] UNSPEC_TLS)) > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt > index a7677eeb45c8..4b3e17bc319c 100644 > --- a/gcc/config/arm/arm.opt > +++ b/gcc/config/arm/arm.opt > @@ -311,3 +311,25 @@ Generate code which uses the core registers only (r0-r14). > mfdpic > Target Mask(FDPIC) > Enable Function Descriptor PIC mode. > + > +mstack-protector-guard= > +Target RejectNegative Joined Enum(stack_protector_guard) Var(arm_stack_protector_guard) Init(SSP_GLOBAL) > +Use given stack-protector guard. > + > +Enum > +Name(stack_protector_guard) Type(enum stack_protector_guard) > +Valid arguments to -mstack-protector-guard=: > + > +EnumValue > +Enum(stack_protector_guard) String(tls) Value(SSP_TLSREG) > + > +EnumValue > +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL) > + > +mstack-protector-guard-offset= > +Target Joined RejectNegative String Var(arm_stack_protector_guard_offset_str) > +Use an immediate to offset from the TLS register. This option is for use with > +fstack-protector-guard=tls and not for use in user-land code. > + > +TargetVariable > +long arm_stack_protector_guard_offset = 0 > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 6070288856c0..13bebbcbf0fb 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -816,6 +816,7 @@ Objective-C and Objective-C++ Dialects}. > -mpure-code @gol > -mcmse @gol > -mfix-cmse-cve-2021-35465 @gol > +-mstack-protector-guard=@var{guard} -mstack-protector-guard-offset=@var{offset} @gol > -mfdpic} > > @emph{AVR Options} > @@ -21099,6 +21100,16 @@ enabled by default when the option @option{-mcpu=} is used with > @code{cortex-m33}, @code{cortex-m35p} or @code{cortex-m55}. The option > @option{-mno-fix-cmse-cve-2021-35465} can be used to disable the mitigation. > > +@item -mstack-protector-guard=@var{guard} > +@itemx -mstack-protector-guard-offset=@var{offset} > +@opindex mstack-protector-guard > +@opindex mstack-protector-guard-offset > +Generate stack protection code using canary at @var{guard}. Supported > +locations are @samp{global} for a global canary or @samp{tls} for a > +canary accessible via the TLS register. The option > +@option{-mstack-protector-guard-offset=} is for use with > +@option{-fstack-protector-guard=tls} and not for use in user-land code. > + > @item -mfdpic > @itemx -mno-fdpic > @opindex mfdpic > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-7.c b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > new file mode 100644 > index 000000000000..874648060d8d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=armv7-a -mfpu=vfp -fstack-protector-all -Os -mstack-protector-guard=tls -mstack-protector-guard-offset=1296 -mtp=cp15" } */ > + > +#include "stack-protector-5.c" > + > +/* See the comment in stack-protector-5.c. */ > +/* { dg-final { scan-assembler-times {\tstr\t} 1 } } */ > +/* Expect two TLS register accesses and two occurrences of the offset */ > +/* { dg-final { scan-assembler-times {\tmrc\t} 2 } } */ > +/* { dg-final { scan-assembler-times {1296} 2 } } */ > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-8.c b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > new file mode 100644 > index 000000000000..bcb2f6b82bdf > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-error "needs a hardware TLS register" "missing error when using TLS stack protector without hardware TLS register" { target *-*-* } 0 } */ > +/* { dg-options "-fstack-protector-all -Os -mstack-protector-guard=tls -mtp=soft" } */ > + > +int foo; > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2021-11-17 17:12 ` Ard Biesheuvel @ 2022-01-12 18:29 ` Ard Biesheuvel 2022-01-14 10:05 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2022-01-12 18:29 UTC (permalink / raw) To: linux-hardening, Ramana Radhakrishnan Cc: Keith Packard, thomas.preudhomme, adhemerval.zanella, Qing Zhao, Richard Sandiford, Kyrylo Tkachov, gcc-patches On Wed, 17 Nov 2021 at 18:12, Ard Biesheuvel <ardb@kernel.org> wrote: > > (+ Ramana) > Ping? > On Mon, 15 Nov 2021 at 19:04, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > Add support for accessing the stack canary value via the TLS register, > > so that multiple threads running in the same address space can use > > distinct canary values. This is intended for the Linux kernel running in > > SMP mode, where processes entering the kernel are essentially threads > > running the same program concurrently: using a global variable for the > > canary in that context is problematic because it can never be rotated, > > and so the OS is forced to use the same value as long as it remains up. > > > > Using the TLS register to index the stack canary helps with this, as it > > allows each CPU to context switch the TLS register along with the rest > > of the process, permitting each process to use its own value for the > > stack canary. > > > > 2021-11-15 Ard Biesheuvel <ardb@kernel.org> > > > > * config/arm/arm-opts.h (enum stack_protector_guard): New > > * config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem): > > New > > * config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define > > (arm_option_override_internal): Handle and put in error checks > > for stack protector guard options. > > (arm_option_reconfigure_globals): Likewise > > (arm_stack_protect_tls_canary_mem): New > > (arm_stack_protect_guard): New > > * config/arm/arm.md (stack_protect_set): New > > (stack_protect_set_tls): Likewise > > (stack_protect_test): Likewise > > (stack_protect_test_tls): Likewise > > (reload_tp_hard): Likewise > > * config/arm/arm.opt (-mstack-protector-guard): New > > (-mstack-protector-guard-offset): New. > > * doc/invoke.texi: Document new options > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/arm/stack-protector-7.c: New test. > > * gcc.target/arm/stack-protector-8.c: New test. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > gcc/config/arm/arm-opts.h | 6 ++ > > gcc/config/arm/arm-protos.h | 2 + > > gcc/config/arm/arm.c | 55 +++++++++++++++ > > gcc/config/arm/arm.md | 71 +++++++++++++++++++- > > gcc/config/arm/arm.opt | 22 ++++++ > > gcc/doc/invoke.texi | 11 +++ > > gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ > > gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ > > 8 files changed, 180 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h > > index 5c4b62f404f7..581ba3c4fbbb 100644 > > --- a/gcc/config/arm/arm-opts.h > > +++ b/gcc/config/arm/arm-opts.h > > @@ -69,4 +69,10 @@ enum arm_tls_type { > > TLS_GNU, > > TLS_GNU2 > > }; > > + > > +/* Where to get the canary for the stack protector. */ > > +enum stack_protector_guard { > > + SSP_TLSREG, /* per-thread canary in TLS register */ > > + SSP_GLOBAL /* global canary */ > > +}; > > #endif > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > > index 9b1f61394ad7..d8d605920c97 100644 > > --- a/gcc/config/arm/arm-protos.h > > +++ b/gcc/config/arm/arm-protos.h > > @@ -195,6 +195,8 @@ extern void arm_split_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx, rtx); > > extern rtx arm_load_tp (rtx); > > extern bool arm_coproc_builtin_available (enum unspecv); > > extern bool arm_coproc_ldc_stc_legitimate_address (rtx); > > +extern rtx arm_stack_protect_tls_canary_mem (bool); > > + > > > > #if defined TREE_CODE > > extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > > index a5b403eb3e49..e5077348ce07 100644 > > --- a/gcc/config/arm/arm.c > > +++ b/gcc/config/arm/arm.c > > @@ -829,6 +829,9 @@ static const struct attribute_spec arm_attribute_table[] = > > > > #undef TARGET_MD_ASM_ADJUST > > #define TARGET_MD_ASM_ADJUST arm_md_asm_adjust > > + > > +#undef TARGET_STACK_PROTECT_GUARD > > +#define TARGET_STACK_PROTECT_GUARD arm_stack_protect_guard > > > > /* Obstack for minipool constant handling. */ > > static struct obstack minipool_obstack; > > @@ -3176,6 +3179,26 @@ arm_option_override_internal (struct gcc_options *opts, > > if (TARGET_THUMB2_P (opts->x_target_flags)) > > opts->x_inline_asm_unified = true; > > > > + if (arm_stack_protector_guard == SSP_GLOBAL > > + && opts->x_arm_stack_protector_guard_offset_str) > > + { > > + error ("incompatible options %'-mstack-protector-guard=global%' and" > > + "%'-mstack-protector-guard-offset=%qs%'", > > + arm_stack_protector_guard_offset_str); > > + } > > + > > + if (opts->x_arm_stack_protector_guard_offset_str) > > + { > > + char *end; > > + const char *str = arm_stack_protector_guard_offset_str; > > + errno = 0; > > + long offs = strtol (arm_stack_protector_guard_offset_str, &end, 0); > > + if (!*str || *end || errno) > > + error ("%qs is not a valid offset in %qs", str, > > + "-mstack-protector-guard-offset="); > > + arm_stack_protector_guard_offset = offs; > > + } > > + > > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS > > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; > > #endif > > @@ -3843,6 +3866,9 @@ arm_option_reconfigure_globals (void) > > else > > target_thread_pointer = TP_SOFT; > > } > > + > > + if (!TARGET_HARD_TP && arm_stack_protector_guard == SSP_TLSREG) > > + error("%'-mstack-protector-guard=tls%' needs a hardware TLS register"); > > } > > > > /* Perform some validation between the desired architecture and the rest of the > > @@ -8108,6 +8134,23 @@ legitimize_pic_address (rtx orig, machine_mode mode, rtx reg, rtx pic_reg, > > } > > > > > > +/* Generate insns that produce the address of the stack canary */ > > +rtx > > +arm_stack_protect_tls_canary_mem (bool reload) > > +{ > > + rtx tp = gen_reg_rtx (SImode); > > + if (reload) > > + emit_insn (gen_reload_tp_hard (tp)); > > + else > > + emit_insn (gen_load_tp_hard (tp)); > > + > > + rtx reg = gen_reg_rtx (SImode); > > + rtx offset = GEN_INT (arm_stack_protector_guard_offset); > > + emit_set_insn (reg, gen_rtx_PLUS (SImode, tp, offset)); > > + return gen_rtx_MEM (SImode, reg); > > +} > > + > > + > > /* Whether a register is callee saved or not. This is necessary because high > > registers are marked as caller saved when optimizing for size on Thumb-1 > > targets despite being callee saved in order to avoid using them. */ > > @@ -34075,6 +34118,18 @@ arm_run_selftests (void) > > #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests > > #endif /* CHECKING_P */ > > > > +/* Implement TARGET_STACK_PROTECT_GUARD. In case of a > > + global variable based guard use the default else > > + return a null tree. */ > > +static tree > > +arm_stack_protect_guard (void) > > +{ > > + if (arm_stack_protector_guard == SSP_GLOBAL) > > + return default_stack_protect_guard (); > > + > > + return NULL_TREE; > > +} > > + > > /* Worker function for TARGET_MD_ASM_ADJUST, while in thumb1 mode. > > Unlike the arm version, we do NOT implement asm flag outputs. */ > > > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > > index 4adc976b8b67..45a54d63f5b3 100644 > > --- a/gcc/config/arm/arm.md > > +++ b/gcc/config/arm/arm.md > > @@ -9183,7 +9183,7 @@ (define_expand "stack_protect_combined_set" > > UNSPEC_SP_SET)) > > (clobber (match_scratch:SI 2 "")) > > (clobber (match_scratch:SI 3 ""))])] > > - "" > > + "arm_stack_protector_guard == SSP_GLOBAL" > > "" > > ) > > > > @@ -9267,7 +9267,7 @@ (define_expand "stack_protect_combined_test" > > (clobber (match_scratch:SI 3 "")) > > (clobber (match_scratch:SI 4 "")) > > (clobber (reg:CC CC_REGNUM))])] > > - "" > > + "arm_stack_protector_guard == SSP_GLOBAL" > > "" > > ) > > > > @@ -9361,6 +9361,64 @@ (define_insn "arm_stack_protect_test_insn" > > (set_attr "arch" "t,32")] > > ) > > > > +(define_expand "stack_protect_set" > > + [(match_operand:SI 0 "memory_operand") > > + (match_operand:SI 1 "memory_operand")] > > + "arm_stack_protector_guard == SSP_TLSREG" > > + " > > +{ > > + operands[1] = arm_stack_protect_tls_canary_mem (false /* reload */); > > + emit_insn (gen_stack_protect_set_tls (operands[0], operands[1])); > > + DONE; > > +}" > > +) > > + > > +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the > > +;; canary value does not live beyond the life of this sequence. > > +(define_insn "stack_protect_set_tls" > > + [(set (match_operand:SI 0 "memory_operand" "=m") > > + (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] > > + UNSPEC_SP_SET)) > > + (set (match_scratch:SI 2 "=&r") (const_int 0))] > > + "" > > + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, #0" > > + [(set_attr "length" "12") > > + (set_attr "conds" "unconditional") > > + (set_attr "type" "multiple")] > > +) > > + > > +(define_expand "stack_protect_test" > > + [(match_operand:SI 0 "memory_operand") > > + (match_operand:SI 1 "memory_operand") > > + (match_operand:SI 2)] > > + "arm_stack_protector_guard == SSP_TLSREG" > > + " > > +{ > > + operands[1] = arm_stack_protect_tls_canary_mem (true /* reload */); > > + emit_insn (gen_stack_protect_test_tls (operands[0], operands[1])); > > + > > + rtx cc_reg = gen_rtx_REG (CC_Zmode, CC_REGNUM); > > + rtx eq = gen_rtx_EQ (CC_Zmode, cc_reg, const0_rtx); > > + emit_jump_insn (gen_arm_cond_branch (operands[2], eq, cc_reg)); > > + DONE; > > +}" > > +) > > + > > +(define_insn "stack_protect_test_tls" > > + [(set (reg:CC_Z CC_REGNUM) > > + (compare:CC_Z (unspec:SI [(match_operand:SI 0 "memory_operand" "m") > > + (match_operand:SI 1 "memory_operand" "m")] > > + UNSPEC_SP_TEST) > > + (const_int 0))) > > + (clobber (match_scratch:SI 2 "=&r")) > > + (clobber (match_scratch:SI 3 "=&r"))] > > + "" > > + "ldr\t%2, %0\;ldr\t%3, %1\;eors\t%2, %3, %2\;mov\t%3, #0" > > + [(set_attr "length" "16") > > + (set_attr "conds" "set") > > + (set_attr "type" "multiple")] > > +) > > + > > (define_expand "casesi" > > [(match_operand:SI 0 "s_register_operand") ; index to jump on > > (match_operand:SI 1 "const_int_operand") ; lower bound > > @@ -12133,6 +12191,15 @@ (define_insn "load_tp_hard" > > (set_attr "type" "mrs")] > > ) > > > > +;; Used by the TLS register based stack protector > > +(define_insn "reload_tp_hard" > > + [(set (match_operand:SI 0 "register_operand" "=r") > > + (unspec_volatile:SI [(const_int 0)] VUNSPEC_MRC))] > > + "TARGET_HARD_TP" > > + "mrc\\tp15, 0, %0, c13, c0, 3\\t@ reload_tp_hard" > > + [(set_attr "type" "mrs")] > > +) > > + > > ;; Doesn't clobber R1-R3. Must use r0 for the first operand. > > (define_insn "load_tp_soft_fdpic" > > [(set (reg:SI 0) (unspec:SI [(const_int 0)] UNSPEC_TLS)) > > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt > > index a7677eeb45c8..4b3e17bc319c 100644 > > --- a/gcc/config/arm/arm.opt > > +++ b/gcc/config/arm/arm.opt > > @@ -311,3 +311,25 @@ Generate code which uses the core registers only (r0-r14). > > mfdpic > > Target Mask(FDPIC) > > Enable Function Descriptor PIC mode. > > + > > +mstack-protector-guard= > > +Target RejectNegative Joined Enum(stack_protector_guard) Var(arm_stack_protector_guard) Init(SSP_GLOBAL) > > +Use given stack-protector guard. > > + > > +Enum > > +Name(stack_protector_guard) Type(enum stack_protector_guard) > > +Valid arguments to -mstack-protector-guard=: > > + > > +EnumValue > > +Enum(stack_protector_guard) String(tls) Value(SSP_TLSREG) > > + > > +EnumValue > > +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL) > > + > > +mstack-protector-guard-offset= > > +Target Joined RejectNegative String Var(arm_stack_protector_guard_offset_str) > > +Use an immediate to offset from the TLS register. This option is for use with > > +fstack-protector-guard=tls and not for use in user-land code. > > + > > +TargetVariable > > +long arm_stack_protector_guard_offset = 0 > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > index 6070288856c0..13bebbcbf0fb 100644 > > --- a/gcc/doc/invoke.texi > > +++ b/gcc/doc/invoke.texi > > @@ -816,6 +816,7 @@ Objective-C and Objective-C++ Dialects}. > > -mpure-code @gol > > -mcmse @gol > > -mfix-cmse-cve-2021-35465 @gol > > +-mstack-protector-guard=@var{guard} -mstack-protector-guard-offset=@var{offset} @gol > > -mfdpic} > > > > @emph{AVR Options} > > @@ -21099,6 +21100,16 @@ enabled by default when the option @option{-mcpu=} is used with > > @code{cortex-m33}, @code{cortex-m35p} or @code{cortex-m55}. The option > > @option{-mno-fix-cmse-cve-2021-35465} can be used to disable the mitigation. > > > > +@item -mstack-protector-guard=@var{guard} > > +@itemx -mstack-protector-guard-offset=@var{offset} > > +@opindex mstack-protector-guard > > +@opindex mstack-protector-guard-offset > > +Generate stack protection code using canary at @var{guard}. Supported > > +locations are @samp{global} for a global canary or @samp{tls} for a > > +canary accessible via the TLS register. The option > > +@option{-mstack-protector-guard-offset=} is for use with > > +@option{-fstack-protector-guard=tls} and not for use in user-land code. > > + > > @item -mfdpic > > @itemx -mno-fdpic > > @opindex mfdpic > > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-7.c b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > > new file mode 100644 > > index 000000000000..874648060d8d > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > > @@ -0,0 +1,10 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-march=armv7-a -mfpu=vfp -fstack-protector-all -Os -mstack-protector-guard=tls -mstack-protector-guard-offset=1296 -mtp=cp15" } */ > > + > > +#include "stack-protector-5.c" > > + > > +/* See the comment in stack-protector-5.c. */ > > +/* { dg-final { scan-assembler-times {\tstr\t} 1 } } */ > > +/* Expect two TLS register accesses and two occurrences of the offset */ > > +/* { dg-final { scan-assembler-times {\tmrc\t} 2 } } */ > > +/* { dg-final { scan-assembler-times {1296} 2 } } */ > > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-8.c b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > > new file mode 100644 > > index 000000000000..bcb2f6b82bdf > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > > @@ -0,0 +1,5 @@ > > +/* { dg-do compile } */ > > +/* { dg-error "needs a hardware TLS register" "missing error when using TLS stack protector without hardware TLS register" { target *-*-* } 0 } */ > > +/* { dg-options "-fstack-protector-all -Os -mstack-protector-guard=tls -mtp=soft" } */ > > + > > +int foo; > > -- > > 2.30.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2022-01-12 18:29 ` Ard Biesheuvel @ 2022-01-14 10:05 ` Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2022-01-14 10:05 UTC (permalink / raw) To: linux-hardening, Ramana Radhakrishnan, richard.earnshaw, Kyrylo Tkachov Cc: Keith Packard, thomas.preudhomme, adhemerval.zanella, Qing Zhao, Richard Sandiford, gcc-patches, spender (+ Richard Earnshaw) On Wed, 12 Jan 2022 at 19:29, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 17 Nov 2021 at 18:12, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > (+ Ramana) > > > > Ping? > > > On Mon, 15 Nov 2021 at 19:04, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > Add support for accessing the stack canary value via the TLS register, > > > so that multiple threads running in the same address space can use > > > distinct canary values. This is intended for the Linux kernel running in > > > SMP mode, where processes entering the kernel are essentially threads > > > running the same program concurrently: using a global variable for the > > > canary in that context is problematic because it can never be rotated, > > > and so the OS is forced to use the same value as long as it remains up. > > > > > > Using the TLS register to index the stack canary helps with this, as it > > > allows each CPU to context switch the TLS register along with the rest > > > of the process, permitting each process to use its own value for the > > > stack canary. > > > > > > 2021-11-15 Ard Biesheuvel <ardb@kernel.org> > > > > > > * config/arm/arm-opts.h (enum stack_protector_guard): New > > > * config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem): > > > New > > > * config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define > > > (arm_option_override_internal): Handle and put in error checks > > > for stack protector guard options. > > > (arm_option_reconfigure_globals): Likewise > > > (arm_stack_protect_tls_canary_mem): New > > > (arm_stack_protect_guard): New > > > * config/arm/arm.md (stack_protect_set): New > > > (stack_protect_set_tls): Likewise > > > (stack_protect_test): Likewise > > > (stack_protect_test_tls): Likewise > > > (reload_tp_hard): Likewise > > > * config/arm/arm.opt (-mstack-protector-guard): New > > > (-mstack-protector-guard-offset): New. > > > * doc/invoke.texi: Document new options > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/arm/stack-protector-7.c: New test. > > > * gcc.target/arm/stack-protector-8.c: New test. > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > gcc/config/arm/arm-opts.h | 6 ++ > > > gcc/config/arm/arm-protos.h | 2 + > > > gcc/config/arm/arm.c | 55 +++++++++++++++ > > > gcc/config/arm/arm.md | 71 +++++++++++++++++++- > > > gcc/config/arm/arm.opt | 22 ++++++ > > > gcc/doc/invoke.texi | 11 +++ > > > gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ > > > gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ > > > 8 files changed, 180 insertions(+), 2 deletions(-) > > > > > > diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h > > > index 5c4b62f404f7..581ba3c4fbbb 100644 > > > --- a/gcc/config/arm/arm-opts.h > > > +++ b/gcc/config/arm/arm-opts.h > > > @@ -69,4 +69,10 @@ enum arm_tls_type { > > > TLS_GNU, > > > TLS_GNU2 > > > }; > > > + > > > +/* Where to get the canary for the stack protector. */ > > > +enum stack_protector_guard { > > > + SSP_TLSREG, /* per-thread canary in TLS register */ > > > + SSP_GLOBAL /* global canary */ > > > +}; > > > #endif > > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > > > index 9b1f61394ad7..d8d605920c97 100644 > > > --- a/gcc/config/arm/arm-protos.h > > > +++ b/gcc/config/arm/arm-protos.h > > > @@ -195,6 +195,8 @@ extern void arm_split_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx, rtx); > > > extern rtx arm_load_tp (rtx); > > > extern bool arm_coproc_builtin_available (enum unspecv); > > > extern bool arm_coproc_ldc_stc_legitimate_address (rtx); > > > +extern rtx arm_stack_protect_tls_canary_mem (bool); > > > + > > > > > > #if defined TREE_CODE > > > extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > > > index a5b403eb3e49..e5077348ce07 100644 > > > --- a/gcc/config/arm/arm.c > > > +++ b/gcc/config/arm/arm.c > > > @@ -829,6 +829,9 @@ static const struct attribute_spec arm_attribute_table[] = > > > > > > #undef TARGET_MD_ASM_ADJUST > > > #define TARGET_MD_ASM_ADJUST arm_md_asm_adjust > > > + > > > +#undef TARGET_STACK_PROTECT_GUARD > > > +#define TARGET_STACK_PROTECT_GUARD arm_stack_protect_guard > > > > > > /* Obstack for minipool constant handling. */ > > > static struct obstack minipool_obstack; > > > @@ -3176,6 +3179,26 @@ arm_option_override_internal (struct gcc_options *opts, > > > if (TARGET_THUMB2_P (opts->x_target_flags)) > > > opts->x_inline_asm_unified = true; > > > > > > + if (arm_stack_protector_guard == SSP_GLOBAL > > > + && opts->x_arm_stack_protector_guard_offset_str) > > > + { > > > + error ("incompatible options %'-mstack-protector-guard=global%' and" > > > + "%'-mstack-protector-guard-offset=%qs%'", > > > + arm_stack_protector_guard_offset_str); > > > + } > > > + > > > + if (opts->x_arm_stack_protector_guard_offset_str) > > > + { > > > + char *end; > > > + const char *str = arm_stack_protector_guard_offset_str; > > > + errno = 0; > > > + long offs = strtol (arm_stack_protector_guard_offset_str, &end, 0); > > > + if (!*str || *end || errno) > > > + error ("%qs is not a valid offset in %qs", str, > > > + "-mstack-protector-guard-offset="); > > > + arm_stack_protector_guard_offset = offs; > > > + } > > > + > > > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS > > > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; > > > #endif > > > @@ -3843,6 +3866,9 @@ arm_option_reconfigure_globals (void) > > > else > > > target_thread_pointer = TP_SOFT; > > > } > > > + > > > + if (!TARGET_HARD_TP && arm_stack_protector_guard == SSP_TLSREG) > > > + error("%'-mstack-protector-guard=tls%' needs a hardware TLS register"); > > > } > > > > > > /* Perform some validation between the desired architecture and the rest of the > > > @@ -8108,6 +8134,23 @@ legitimize_pic_address (rtx orig, machine_mode mode, rtx reg, rtx pic_reg, > > > } > > > > > > > > > +/* Generate insns that produce the address of the stack canary */ > > > +rtx > > > +arm_stack_protect_tls_canary_mem (bool reload) > > > +{ > > > + rtx tp = gen_reg_rtx (SImode); > > > + if (reload) > > > + emit_insn (gen_reload_tp_hard (tp)); > > > + else > > > + emit_insn (gen_load_tp_hard (tp)); > > > + > > > + rtx reg = gen_reg_rtx (SImode); > > > + rtx offset = GEN_INT (arm_stack_protector_guard_offset); > > > + emit_set_insn (reg, gen_rtx_PLUS (SImode, tp, offset)); > > > + return gen_rtx_MEM (SImode, reg); > > > +} > > > + > > > + > > > /* Whether a register is callee saved or not. This is necessary because high > > > registers are marked as caller saved when optimizing for size on Thumb-1 > > > targets despite being callee saved in order to avoid using them. */ > > > @@ -34075,6 +34118,18 @@ arm_run_selftests (void) > > > #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests > > > #endif /* CHECKING_P */ > > > > > > +/* Implement TARGET_STACK_PROTECT_GUARD. In case of a > > > + global variable based guard use the default else > > > + return a null tree. */ > > > +static tree > > > +arm_stack_protect_guard (void) > > > +{ > > > + if (arm_stack_protector_guard == SSP_GLOBAL) > > > + return default_stack_protect_guard (); > > > + > > > + return NULL_TREE; > > > +} > > > + > > > /* Worker function for TARGET_MD_ASM_ADJUST, while in thumb1 mode. > > > Unlike the arm version, we do NOT implement asm flag outputs. */ > > > > > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > > > index 4adc976b8b67..45a54d63f5b3 100644 > > > --- a/gcc/config/arm/arm.md > > > +++ b/gcc/config/arm/arm.md > > > @@ -9183,7 +9183,7 @@ (define_expand "stack_protect_combined_set" > > > UNSPEC_SP_SET)) > > > (clobber (match_scratch:SI 2 "")) > > > (clobber (match_scratch:SI 3 ""))])] > > > - "" > > > + "arm_stack_protector_guard == SSP_GLOBAL" > > > "" > > > ) > > > > > > @@ -9267,7 +9267,7 @@ (define_expand "stack_protect_combined_test" > > > (clobber (match_scratch:SI 3 "")) > > > (clobber (match_scratch:SI 4 "")) > > > (clobber (reg:CC CC_REGNUM))])] > > > - "" > > > + "arm_stack_protector_guard == SSP_GLOBAL" > > > "" > > > ) > > > > > > @@ -9361,6 +9361,64 @@ (define_insn "arm_stack_protect_test_insn" > > > (set_attr "arch" "t,32")] > > > ) > > > > > > +(define_expand "stack_protect_set" > > > + [(match_operand:SI 0 "memory_operand") > > > + (match_operand:SI 1 "memory_operand")] > > > + "arm_stack_protector_guard == SSP_TLSREG" > > > + " > > > +{ > > > + operands[1] = arm_stack_protect_tls_canary_mem (false /* reload */); > > > + emit_insn (gen_stack_protect_set_tls (operands[0], operands[1])); > > > + DONE; > > > +}" > > > +) > > > + > > > +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the > > > +;; canary value does not live beyond the life of this sequence. > > > +(define_insn "stack_protect_set_tls" > > > + [(set (match_operand:SI 0 "memory_operand" "=m") > > > + (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] > > > + UNSPEC_SP_SET)) > > > + (set (match_scratch:SI 2 "=&r") (const_int 0))] > > > + "" > > > + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, #0" > > > + [(set_attr "length" "12") > > > + (set_attr "conds" "unconditional") > > > + (set_attr "type" "multiple")] > > > +) > > > + > > > +(define_expand "stack_protect_test" > > > + [(match_operand:SI 0 "memory_operand") > > > + (match_operand:SI 1 "memory_operand") > > > + (match_operand:SI 2)] > > > + "arm_stack_protector_guard == SSP_TLSREG" > > > + " > > > +{ > > > + operands[1] = arm_stack_protect_tls_canary_mem (true /* reload */); > > > + emit_insn (gen_stack_protect_test_tls (operands[0], operands[1])); > > > + > > > + rtx cc_reg = gen_rtx_REG (CC_Zmode, CC_REGNUM); > > > + rtx eq = gen_rtx_EQ (CC_Zmode, cc_reg, const0_rtx); > > > + emit_jump_insn (gen_arm_cond_branch (operands[2], eq, cc_reg)); > > > + DONE; > > > +}" > > > +) > > > + > > > +(define_insn "stack_protect_test_tls" > > > + [(set (reg:CC_Z CC_REGNUM) > > > + (compare:CC_Z (unspec:SI [(match_operand:SI 0 "memory_operand" "m") > > > + (match_operand:SI 1 "memory_operand" "m")] > > > + UNSPEC_SP_TEST) > > > + (const_int 0))) > > > + (clobber (match_scratch:SI 2 "=&r")) > > > + (clobber (match_scratch:SI 3 "=&r"))] > > > + "" > > > + "ldr\t%2, %0\;ldr\t%3, %1\;eors\t%2, %3, %2\;mov\t%3, #0" > > > + [(set_attr "length" "16") > > > + (set_attr "conds" "set") > > > + (set_attr "type" "multiple")] > > > +) > > > + > > > (define_expand "casesi" > > > [(match_operand:SI 0 "s_register_operand") ; index to jump on > > > (match_operand:SI 1 "const_int_operand") ; lower bound > > > @@ -12133,6 +12191,15 @@ (define_insn "load_tp_hard" > > > (set_attr "type" "mrs")] > > > ) > > > > > > +;; Used by the TLS register based stack protector > > > +(define_insn "reload_tp_hard" > > > + [(set (match_operand:SI 0 "register_operand" "=r") > > > + (unspec_volatile:SI [(const_int 0)] VUNSPEC_MRC))] > > > + "TARGET_HARD_TP" > > > + "mrc\\tp15, 0, %0, c13, c0, 3\\t@ reload_tp_hard" > > > + [(set_attr "type" "mrs")] > > > +) > > > + > > > ;; Doesn't clobber R1-R3. Must use r0 for the first operand. > > > (define_insn "load_tp_soft_fdpic" > > > [(set (reg:SI 0) (unspec:SI [(const_int 0)] UNSPEC_TLS)) > > > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt > > > index a7677eeb45c8..4b3e17bc319c 100644 > > > --- a/gcc/config/arm/arm.opt > > > +++ b/gcc/config/arm/arm.opt > > > @@ -311,3 +311,25 @@ Generate code which uses the core registers only (r0-r14). > > > mfdpic > > > Target Mask(FDPIC) > > > Enable Function Descriptor PIC mode. > > > + > > > +mstack-protector-guard= > > > +Target RejectNegative Joined Enum(stack_protector_guard) Var(arm_stack_protector_guard) Init(SSP_GLOBAL) > > > +Use given stack-protector guard. > > > + > > > +Enum > > > +Name(stack_protector_guard) Type(enum stack_protector_guard) > > > +Valid arguments to -mstack-protector-guard=: > > > + > > > +EnumValue > > > +Enum(stack_protector_guard) String(tls) Value(SSP_TLSREG) > > > + > > > +EnumValue > > > +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL) > > > + > > > +mstack-protector-guard-offset= > > > +Target Joined RejectNegative String Var(arm_stack_protector_guard_offset_str) > > > +Use an immediate to offset from the TLS register. This option is for use with > > > +fstack-protector-guard=tls and not for use in user-land code. > > > + > > > +TargetVariable > > > +long arm_stack_protector_guard_offset = 0 > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > index 6070288856c0..13bebbcbf0fb 100644 > > > --- a/gcc/doc/invoke.texi > > > +++ b/gcc/doc/invoke.texi > > > @@ -816,6 +816,7 @@ Objective-C and Objective-C++ Dialects}. > > > -mpure-code @gol > > > -mcmse @gol > > > -mfix-cmse-cve-2021-35465 @gol > > > +-mstack-protector-guard=@var{guard} -mstack-protector-guard-offset=@var{offset} @gol > > > -mfdpic} > > > > > > @emph{AVR Options} > > > @@ -21099,6 +21100,16 @@ enabled by default when the option @option{-mcpu=} is used with > > > @code{cortex-m33}, @code{cortex-m35p} or @code{cortex-m55}. The option > > > @option{-mno-fix-cmse-cve-2021-35465} can be used to disable the mitigation. > > > > > > +@item -mstack-protector-guard=@var{guard} > > > +@itemx -mstack-protector-guard-offset=@var{offset} > > > +@opindex mstack-protector-guard > > > +@opindex mstack-protector-guard-offset > > > +Generate stack protection code using canary at @var{guard}. Supported > > > +locations are @samp{global} for a global canary or @samp{tls} for a > > > +canary accessible via the TLS register. The option > > > +@option{-mstack-protector-guard-offset=} is for use with > > > +@option{-fstack-protector-guard=tls} and not for use in user-land code. > > > + > > > @item -mfdpic > > > @itemx -mno-fdpic > > > @opindex mfdpic > > > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-7.c b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > > > new file mode 100644 > > > index 000000000000..874648060d8d > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > > > @@ -0,0 +1,10 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-march=armv7-a -mfpu=vfp -fstack-protector-all -Os -mstack-protector-guard=tls -mstack-protector-guard-offset=1296 -mtp=cp15" } */ > > > + > > > +#include "stack-protector-5.c" > > > + > > > +/* See the comment in stack-protector-5.c. */ > > > +/* { dg-final { scan-assembler-times {\tstr\t} 1 } } */ > > > +/* Expect two TLS register accesses and two occurrences of the offset */ > > > +/* { dg-final { scan-assembler-times {\tmrc\t} 2 } } */ > > > +/* { dg-final { scan-assembler-times {1296} 2 } } */ > > > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-8.c b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > > > new file mode 100644 > > > index 000000000000..bcb2f6b82bdf > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > > > @@ -0,0 +1,5 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-error "needs a hardware TLS register" "missing error when using TLS stack protector without hardware TLS register" { target *-*-* } 0 } */ > > > +/* { dg-options "-fstack-protector-all -Os -mstack-protector-guard=tls -mtp=soft" } */ > > > + > > > +int foo; > > > -- > > > 2.30.2 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2021-11-15 18:04 ` [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access Ard Biesheuvel 2021-11-17 17:12 ` Ard Biesheuvel @ 2022-01-19 16:54 ` Kyrylo Tkachov 2022-01-19 17:02 ` Ard Biesheuvel 1 sibling, 1 reply; 8+ messages in thread From: Kyrylo Tkachov @ 2022-01-19 16:54 UTC (permalink / raw) To: Ard Biesheuvel, linux-hardening Cc: Richard Sandiford, thomas.preudhomme, Keith Packard, Kyrylo Tkachov, Richard Earnshaw, Ramana Radhakrishnan, gcc-patches Hi Ard, > -----Original Message----- > From: Gcc-patches <gcc-patches- > bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Ard > Biesheuvel via Gcc-patches > Sent: Monday, November 15, 2021 6:04 PM > To: linux-hardening@vger.kernel.org > Cc: Richard Sandiford <Richard.Sandiford@arm.com>; > thomas.preudhomme@celest.fr; Keith Packard <keithpac@amazon.com>; > gcc-patches@gcc.gnu.org; Kyrylo Tkachov <kyryo.tkachov@arm.com>; Ard > Biesheuvel <ardb@kernel.org> > Subject: [PATCH v5 1/1] [ARM] Add support for TLS register based stack > protector canary access > > Add support for accessing the stack canary value via the TLS register, > so that multiple threads running in the same address space can use > distinct canary values. This is intended for the Linux kernel running in > SMP mode, where processes entering the kernel are essentially threads > running the same program concurrently: using a global variable for the > canary in that context is problematic because it can never be rotated, > and so the OS is forced to use the same value as long as it remains up. > > Using the TLS register to index the stack canary helps with this, as it > allows each CPU to context switch the TLS register along with the rest > of the process, permitting each process to use its own value for the > stack canary. I've tested this patch on an arm-none-linux-gnueabihf target and the results look clean. Have you tested this patch with a kernel build as well? (since the functionality is intended for that use). If so, the patch is okay but please rebase it and repost so that we can commit it taking into account.... > > 2021-11-15 Ard Biesheuvel <ardb@kernel.org> > > * config/arm/arm-opts.h (enum stack_protector_guard): New > * config/arm/arm-protos.h (arm_stack_protect_tls_canary_mem): > New > * config/arm/arm.c (TARGET_STACK_PROTECT_GUARD): Define ... this file has now be renamed to arm.cc and... > (arm_option_override_internal): Handle and put in error checks > for stack protector guard options. > (arm_option_reconfigure_globals): Likewise > (arm_stack_protect_tls_canary_mem): New > (arm_stack_protect_guard): New > * config/arm/arm.md (stack_protect_set): New > (stack_protect_set_tls): Likewise > (stack_protect_test): Likewise > (stack_protect_test_tls): Likewise > (reload_tp_hard): Likewise > * config/arm/arm.opt (-mstack-protector-guard): New > (-mstack-protector-guard-offset): New. > * doc/invoke.texi: Document new options > > gcc/testsuite/ChangeLog: > > * gcc.target/arm/stack-protector-7.c: New test. > * gcc.target/arm/stack-protector-8.c: New test. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > gcc/config/arm/arm-opts.h | 6 ++ > gcc/config/arm/arm-protos.h | 2 + > gcc/config/arm/arm.c | 55 +++++++++++++++ > gcc/config/arm/arm.md | 71 +++++++++++++++++++- > gcc/config/arm/arm.opt | 22 ++++++ > gcc/doc/invoke.texi | 11 +++ > gcc/testsuite/gcc.target/arm/stack-protector-7.c | 10 +++ > gcc/testsuite/gcc.target/arm/stack-protector-8.c | 5 ++ > 8 files changed, 180 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h > index 5c4b62f404f7..581ba3c4fbbb 100644 > --- a/gcc/config/arm/arm-opts.h > +++ b/gcc/config/arm/arm-opts.h > @@ -69,4 +69,10 @@ enum arm_tls_type { > TLS_GNU, > TLS_GNU2 > }; > + > +/* Where to get the canary for the stack protector. */ > +enum stack_protector_guard { > + SSP_TLSREG, /* per-thread canary in TLS register */ > + SSP_GLOBAL /* global canary */ > +}; > #endif > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index 9b1f61394ad7..d8d605920c97 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -195,6 +195,8 @@ extern void arm_split_atomic_op (enum rtx_code, > rtx, rtx, rtx, rtx, rtx, rtx); > extern rtx arm_load_tp (rtx); > extern bool arm_coproc_builtin_available (enum unspecv); > extern bool arm_coproc_ldc_stc_legitimate_address (rtx); > +extern rtx arm_stack_protect_tls_canary_mem (bool); > + > > #if defined TREE_CODE > extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree); > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index a5b403eb3e49..e5077348ce07 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -829,6 +829,9 @@ static const struct attribute_spec > arm_attribute_table[] = > > #undef TARGET_MD_ASM_ADJUST > #define TARGET_MD_ASM_ADJUST arm_md_asm_adjust > + > +#undef TARGET_STACK_PROTECT_GUARD > +#define TARGET_STACK_PROTECT_GUARD arm_stack_protect_guard > > > > /* Obstack for minipool constant handling. */ > static struct obstack minipool_obstack; > @@ -3176,6 +3179,26 @@ arm_option_override_internal (struct > gcc_options *opts, > if (TARGET_THUMB2_P (opts->x_target_flags)) > opts->x_inline_asm_unified = true; > > + if (arm_stack_protector_guard == SSP_GLOBAL > + && opts->x_arm_stack_protector_guard_offset_str) > + { > + error ("incompatible options %'-mstack-protector-guard=global%' and" > + "%'-mstack-protector-guard-offset=%qs%'", > + arm_stack_protector_guard_offset_str); > + } > + > + if (opts->x_arm_stack_protector_guard_offset_str) > + { > + char *end; > + const char *str = arm_stack_protector_guard_offset_str; > + errno = 0; > + long offs = strtol (arm_stack_protector_guard_offset_str, &end, 0); > + if (!*str || *end || errno) > + error ("%qs is not a valid offset in %qs", str, > + "-mstack-protector-guard-offset="); > + arm_stack_protector_guard_offset = offs; > + } > + > #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS > SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; > #endif > @@ -3843,6 +3866,9 @@ arm_option_reconfigure_globals (void) > else > target_thread_pointer = TP_SOFT; > } > + > + if (!TARGET_HARD_TP && arm_stack_protector_guard == SSP_TLSREG) > + error("%'-mstack-protector-guard=tls%' needs a hardware TLS register"); > } > > /* Perform some validation between the desired architecture and the rest of > the > @@ -8108,6 +8134,23 @@ legitimize_pic_address (rtx orig, machine_mode > mode, rtx reg, rtx pic_reg, > } > > > +/* Generate insns that produce the address of the stack canary */ > +rtx > +arm_stack_protect_tls_canary_mem (bool reload) > +{ > + rtx tp = gen_reg_rtx (SImode); > + if (reload) > + emit_insn (gen_reload_tp_hard (tp)); > + else > + emit_insn (gen_load_tp_hard (tp)); > + > + rtx reg = gen_reg_rtx (SImode); > + rtx offset = GEN_INT (arm_stack_protector_guard_offset); > + emit_set_insn (reg, gen_rtx_PLUS (SImode, tp, offset)); > + return gen_rtx_MEM (SImode, reg); > +} > + > + > /* Whether a register is callee saved or not. This is necessary because high > registers are marked as caller saved when optimizing for size on Thumb-1 > targets despite being callee saved in order to avoid using them. */ > @@ -34075,6 +34118,18 @@ arm_run_selftests (void) > #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests > #endif /* CHECKING_P */ > > +/* Implement TARGET_STACK_PROTECT_GUARD. In case of a > + global variable based guard use the default else > + return a null tree. */ > +static tree > +arm_stack_protect_guard (void) > +{ > + if (arm_stack_protector_guard == SSP_GLOBAL) > + return default_stack_protect_guard (); > + > + return NULL_TREE; > +} > + > /* Worker function for TARGET_MD_ASM_ADJUST, while in thumb1 mode. > Unlike the arm version, we do NOT implement asm flag outputs. */ > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 4adc976b8b67..45a54d63f5b3 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -9183,7 +9183,7 @@ (define_expand "stack_protect_combined_set" > UNSPEC_SP_SET)) > (clobber (match_scratch:SI 2 "")) > (clobber (match_scratch:SI 3 ""))])] > - "" > + "arm_stack_protector_guard == SSP_GLOBAL" > "" > ) > > @@ -9267,7 +9267,7 @@ (define_expand "stack_protect_combined_test" > (clobber (match_scratch:SI 3 "")) > (clobber (match_scratch:SI 4 "")) > (clobber (reg:CC CC_REGNUM))])] > - "" > + "arm_stack_protector_guard == SSP_GLOBAL" > "" > ) > > @@ -9361,6 +9361,64 @@ (define_insn "arm_stack_protect_test_insn" > (set_attr "arch" "t,32")] > ) > > +(define_expand "stack_protect_set" > + [(match_operand:SI 0 "memory_operand") > + (match_operand:SI 1 "memory_operand")] > + "arm_stack_protector_guard == SSP_TLSREG" > + " > +{ > + operands[1] = arm_stack_protect_tls_canary_mem (false /* reload */); > + emit_insn (gen_stack_protect_set_tls (operands[0], operands[1])); > + DONE; > +}" > +) > + > +;; DO NOT SPLIT THIS PATTERN. It is important for security reasons that the > +;; canary value does not live beyond the life of this sequence. > +(define_insn "stack_protect_set_tls" > + [(set (match_operand:SI 0 "memory_operand" "=m") > + (unspec:SI [(match_operand:SI 1 "memory_operand" "m")] > + UNSPEC_SP_SET)) > + (set (match_scratch:SI 2 "=&r") (const_int 0))] > + "" > + "ldr\\t%2, %1\;str\\t%2, %0\;mov\t%2, #0" > + [(set_attr "length" "12") > + (set_attr "conds" "unconditional") > + (set_attr "type" "multiple")] > +) > + > +(define_expand "stack_protect_test" > + [(match_operand:SI 0 "memory_operand") > + (match_operand:SI 1 "memory_operand") > + (match_operand:SI 2)] > + "arm_stack_protector_guard == SSP_TLSREG" > + " > +{ > + operands[1] = arm_stack_protect_tls_canary_mem (true /* reload */); > + emit_insn (gen_stack_protect_test_tls (operands[0], operands[1])); > + > + rtx cc_reg = gen_rtx_REG (CC_Zmode, CC_REGNUM); > + rtx eq = gen_rtx_EQ (CC_Zmode, cc_reg, const0_rtx); > + emit_jump_insn (gen_arm_cond_branch (operands[2], eq, cc_reg)); > + DONE; > +}" > +) > + > +(define_insn "stack_protect_test_tls" > + [(set (reg:CC_Z CC_REGNUM) > + (compare:CC_Z (unspec:SI [(match_operand:SI 0 "memory_operand" > "m") > + (match_operand:SI 1 "memory_operand" > "m")] > + UNSPEC_SP_TEST) > + (const_int 0))) > + (clobber (match_scratch:SI 2 "=&r")) > + (clobber (match_scratch:SI 3 "=&r"))] > + "" > + "ldr\t%2, %0\;ldr\t%3, %1\;eors\t%2, %3, %2\;mov\t%3, #0" > + [(set_attr "length" "16") > + (set_attr "conds" "set") > + (set_attr "type" "multiple")] > +) > + > (define_expand "casesi" > [(match_operand:SI 0 "s_register_operand") ; index to jump on > (match_operand:SI 1 "const_int_operand") ; lower bound > @@ -12133,6 +12191,15 @@ (define_insn "load_tp_hard" > (set_attr "type" "mrs")] > ) > > +;; Used by the TLS register based stack protector > +(define_insn "reload_tp_hard" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (unspec_volatile:SI [(const_int 0)] VUNSPEC_MRC))] > + "TARGET_HARD_TP" > + "mrc\\tp15, 0, %0, c13, c0, 3\\t@ reload_tp_hard" > + [(set_attr "type" "mrs")] > +) > + > ;; Doesn't clobber R1-R3. Must use r0 for the first operand. > (define_insn "load_tp_soft_fdpic" > [(set (reg:SI 0) (unspec:SI [(const_int 0)] UNSPEC_TLS)) > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt > index a7677eeb45c8..4b3e17bc319c 100644 > --- a/gcc/config/arm/arm.opt > +++ b/gcc/config/arm/arm.opt > @@ -311,3 +311,25 @@ Generate code which uses the core registers only > (r0-r14). > mfdpic > Target Mask(FDPIC) > Enable Function Descriptor PIC mode. > + > +mstack-protector-guard= > +Target RejectNegative Joined Enum(stack_protector_guard) > Var(arm_stack_protector_guard) Init(SSP_GLOBAL) > +Use given stack-protector guard. > + > +Enum > +Name(stack_protector_guard) Type(enum stack_protector_guard) > +Valid arguments to -mstack-protector-guard=: > + > +EnumValue > +Enum(stack_protector_guard) String(tls) Value(SSP_TLSREG) > + > +EnumValue > +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL) > + > +mstack-protector-guard-offset= > +Target Joined RejectNegative String > Var(arm_stack_protector_guard_offset_str) > +Use an immediate to offset from the TLS register. This option is for use with > +fstack-protector-guard=tls and not for use in user-land code. > + > +TargetVariable > +long arm_stack_protector_guard_offset = 0 > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 6070288856c0..13bebbcbf0fb 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -816,6 +816,7 @@ Objective-C and Objective-C++ Dialects}. > -mpure-code @gol > -mcmse @gol > -mfix-cmse-cve-2021-35465 @gol > +-mstack-protector-guard=@var{guard} -mstack-protector-guard- > offset=@var{offset} @gol > -mfdpic} > > @emph{AVR Options} > @@ -21099,6 +21100,16 @@ enabled by default when the option @option{- > mcpu=} is used with > @code{cortex-m33}, @code{cortex-m35p} or @code{cortex-m55}. The > option > @option{-mno-fix-cmse-cve-2021-35465} can be used to disable the > mitigation. > > +@item -mstack-protector-guard=@var{guard} > +@itemx -mstack-protector-guard-offset=@var{offset} > +@opindex mstack-protector-guard > +@opindex mstack-protector-guard-offset > +Generate stack protection code using canary at @var{guard}. Supported > +locations are @samp{global} for a global canary or @samp{tls} for a > +canary accessible via the TLS register. The option > +@option{-mstack-protector-guard-offset=} is for use with > +@option{-fstack-protector-guard=tls} and not for use in user-land code. > + > @item -mfdpic > @itemx -mno-fdpic > @opindex mfdpic > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-7.c > b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > new file mode 100644 > index 000000000000..874648060d8d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-7.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=armv7-a -mfpu=vfp -fstack-protector-all -Os - > mstack-protector-guard=tls -mstack-protector-guard-offset=1296 -mtp=cp15" > } */ ... These dg-options can cause fails when the test is run with undesirable multilib options. These tests should have a guard something like before the dg-options /* { dg-require-effective-target arm_hard_vfp_ok } */ /* { dg-require-effective-target arm_arch_v7a_ok } */ Thanks, Kyrill > + > +#include "stack-protector-5.c" > + > +/* See the comment in stack-protector-5.c. */ > +/* { dg-final { scan-assembler-times {\tstr\t} 1 } } */ > +/* Expect two TLS register accesses and two occurrences of the offset */ > +/* { dg-final { scan-assembler-times {\tmrc\t} 2 } } */ > +/* { dg-final { scan-assembler-times {1296} 2 } } */ > diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-8.c > b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > new file mode 100644 > index 000000000000..bcb2f6b82bdf > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/stack-protector-8.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-error "needs a hardware TLS register" "missing error when using TLS > stack protector without hardware TLS register" { target *-*-* } 0 } */ > +/* { dg-options "-fstack-protector-all -Os -mstack-protector-guard=tls - > mtp=soft" } */ > + > +int foo; > -- > 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2022-01-19 16:54 ` Kyrylo Tkachov @ 2022-01-19 17:02 ` Ard Biesheuvel 2022-01-20 16:57 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2022-01-19 17:02 UTC (permalink / raw) To: Kyrylo Tkachov Cc: linux-hardening, Richard Sandiford, thomas.preudhomme, Keith Packard, Kyrylo Tkachov, Richard Earnshaw, Ramana Radhakrishnan, gcc-patches On Wed, 19 Jan 2022 at 17:54, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote: > > Hi Ard, > > > -----Original Message----- > > From: Gcc-patches <gcc-patches- > > bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Ard > > Biesheuvel via Gcc-patches > > Sent: Monday, November 15, 2021 6:04 PM > > To: linux-hardening@vger.kernel.org > > Cc: Richard Sandiford <Richard.Sandiford@arm.com>; > > thomas.preudhomme@celest.fr; Keith Packard <keithpac@amazon.com>; > > gcc-patches@gcc.gnu.org; Kyrylo Tkachov <kyryo.tkachov@arm.com>; Ard > > Biesheuvel <ardb@kernel.org> > > Subject: [PATCH v5 1/1] [ARM] Add support for TLS register based stack > > protector canary access > > > > Add support for accessing the stack canary value via the TLS register, > > so that multiple threads running in the same address space can use > > distinct canary values. This is intended for the Linux kernel running in > > SMP mode, where processes entering the kernel are essentially threads > > running the same program concurrently: using a global variable for the > > canary in that context is problematic because it can never be rotated, > > and so the OS is forced to use the same value as long as it remains up. > > > > Using the TLS register to index the stack canary helps with this, as it > > allows each CPU to context switch the TLS register along with the rest > > of the process, permitting each process to use its own value for the > > stack canary. > > I've tested this patch on an arm-none-linux-gnueabihf target and the results look clean. > Have you tested this patch with a kernel build as well? (since the functionality is intended for that use). Of course. > If so, the patch is okay but please rebase it and repost so that we can commit it taking into account.... > Will do. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access 2022-01-19 17:02 ` Ard Biesheuvel @ 2022-01-20 16:57 ` Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2022-01-20 16:57 UTC (permalink / raw) To: Kyrylo Tkachov Cc: linux-hardening, Richard Sandiford, thomas.preudhomme, Keith Packard, Kyrylo Tkachov, Richard Earnshaw, Ramana Radhakrishnan, gcc-patches On Wed, 19 Jan 2022 at 18:02, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 19 Jan 2022 at 17:54, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote: > > > > Hi Ard, > > > > > -----Original Message----- > > > From: Gcc-patches <gcc-patches- > > > bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Ard > > > Biesheuvel via Gcc-patches > > > Sent: Monday, November 15, 2021 6:04 PM > > > To: linux-hardening@vger.kernel.org > > > Cc: Richard Sandiford <Richard.Sandiford@arm.com>; > > > thomas.preudhomme@celest.fr; Keith Packard <keithpac@amazon.com>; > > > gcc-patches@gcc.gnu.org; Kyrylo Tkachov <kyryo.tkachov@arm.com>; Ard > > > Biesheuvel <ardb@kernel.org> > > > Subject: [PATCH v5 1/1] [ARM] Add support for TLS register based stack > > > protector canary access > > > > > > Add support for accessing the stack canary value via the TLS register, > > > so that multiple threads running in the same address space can use > > > distinct canary values. This is intended for the Linux kernel running in > > > SMP mode, where processes entering the kernel are essentially threads > > > running the same program concurrently: using a global variable for the > > > canary in that context is problematic because it can never be rotated, > > > and so the OS is forced to use the same value as long as it remains up. > > > > > > Using the TLS register to index the stack canary helps with this, as it > > > allows each CPU to context switch the TLS register along with the rest > > > of the process, permitting each process to use its own value for the > > > stack canary. > > > > I've tested this patch on an arm-none-linux-gnueabihf target and the results look clean. > > Have you tested this patch with a kernel build as well? (since the functionality is intended for that use). > > Of course. > > > If so, the patch is okay but please rebase it and repost so that we can commit it taking into account.... > > > > Will do. I have sent out my v6 - please let me know if there is anything else I need to do to get this landed. Thanks, Ard. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-20 16:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-15 18:04 [PATCH v5 0/1] implement TLS register based stack canary for ARM Ard Biesheuvel 2021-11-15 18:04 ` [PATCH v5 1/1] [ARM] Add support for TLS register based stack protector canary access Ard Biesheuvel 2021-11-17 17:12 ` Ard Biesheuvel 2022-01-12 18:29 ` Ard Biesheuvel 2022-01-14 10:05 ` Ard Biesheuvel 2022-01-19 16:54 ` Kyrylo Tkachov 2022-01-19 17:02 ` Ard Biesheuvel 2022-01-20 16:57 ` Ard Biesheuvel
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).