* [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
@ 2017-07-31 5:36 Jeff Law
2017-08-18 14:36 ` Richard Biener
2017-08-20 18:31 ` Martin Sebor
0 siblings, 2 replies; 11+ messages in thread
From: Jeff Law @ 2017-07-31 5:36 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 545 bytes --]
This patch introduces the stack clash protection options
Changes since V2:
Adds two new params. The first controls the size of the guard area.
This controls the threshold for when a function prologue requires
probes. The second controls the probing interval -- ie, once probes are
needed, how often do we emit them. These are really meant more for
developers to experiment with than users. Regardless I did go ahead and
document them./PARAM
It also adds some sanity checking WRT combining stack clash protection
with -fstack-check.
Jeff
[-- Attachment #2: P1 --]
[-- Type: text/plain, Size: 12588 bytes --]
* common.opt (-fstack-clash-protection): New option.
* flag-types.h (enum stack_check_type): Note difference between
-fstack-check= and -fstack-clash-protection.
* params.h (set_param_value): Verify stack-clash-protection
params are powers of two.
* params.def (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE): New PARAM.
(PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL): Likewise.
* toplev.c (process_options): Issue warnings/errors for cases
not handled with -fstack-clash-protection.
* doc/invoke.texi (-fstack-clash-protection): Document new option.
(-fstack-check): Note additional problem with -fstack-check=generic.
Note that -fstack-check is primarily for Ada and refer users
to -fstack-clash-protection for stack-clash-protection.
Document new params for stack clash protection.
* toplev.c (process_options): Handle -fstack-clash-protection.
testsuite/
* gcc.dg/stack-check-2.c: New test.
* lib/target-supports.exp
(check_effective_target_supports_stack_clash_protection): New function.
(check_effective_target_frame_pointer_for_non_leaf): Likewise.
(check_effective_target_caller_implicit_probes): Likewise.
diff --git a/gcc/common.opt b/gcc/common.opt
index e81165c..cfaf2bc 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2306,6 +2306,11 @@ fstack-check
Common Alias(fstack-check=, specific, no)
Insert stack checking code into the program. Same as -fstack-check=specific.
+fstack-clash-protection
+Common Report Var(flag_stack_clash_protection)
+Insert code to probe each page of stack space as it is allocated to protect
+from stack-clash style attacks
+
fstack-limit
Common Var(common_deferred_options) Defer
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3e5cee8..8095dc2 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10128,6 +10128,20 @@ compilation without. The value for compilation with profile feedback
needs to be more conservative (higher) in order to make tracer
effective.
+@item stack-clash-protection-guard-size
+The size (in bytes) of the stack guard. Acceptable values are powers of 2
+between 4096 and 134217728 and defaults to 4096. Higher values may reduce the
+number of explicit probes, but a value larger than the operating system
+provided guard will leave code vulnerable to stack clash style attacks.
+
+@item stack-clash-protection-probe-interval
+Stack clash protection involves probing stack space as it is allocated. This
+param controls the maximum distance (in bytes) between probes into the stack.
+Acceptable values are powers of 2 between 4096 and 65536 and defaults to
+4096. Higher values may reduce the number of explicit probes, but a value
+larger than the operating system provided guard will leave code vulnerable to
+stack clash style attacks.
+
@item max-cse-path-length
The maximum number of basic blocks on path that CSE considers.
@@ -11333,7 +11347,8 @@ target support in the compiler but comes with the following drawbacks:
@enumerate
@item
Modified allocation strategy for large objects: they are always
-allocated dynamically if their size exceeds a fixed threshold.
+allocated dynamically if their size exceeds a fixed threshold. Note this
+may change the semantics of some code.
@item
Fixed limit on the size of the static frame of functions: when it is
@@ -11348,6 +11363,25 @@ generic implementation, code performance is hampered.
Note that old-style stack checking is also the fallback method for
@samp{specific} if no target support has been added in the compiler.
+@samp{-fstack-check=} is designed for Ada's needs to detect infinite recursion
+and stack overflows. @samp{specific} is an excellent choice when compiling
+Ada code. It is not generally sufficient to protect against stack-clash
+attacks. To protect against those you want @samp{-fstack-clash-protection}.
+
+@item -fstack-clash-protection
+@opindex fstack-clash-protection
+Generate code to prevent stack clash style attacks. When this option is
+enabled, the compiler will only allocate one page of stack space at a time
+and each page is accessed immediately after allocation. Thus, it prevents
+allocations from jumping over any stack guard page provided by the
+operating system.
+
+Most targets do not fully support stack clash protection. However, on
+those targets @option{-fstack-clash-protection} will protect dynamic stack
+allocations. @option{-fstack-clash-protection} may also provide limited
+protection for static stack allocations if the target supports
+@option{-fstack-check=specific}.
+
@item -fstack-limit-register=@var{reg}
@itemx -fstack-limit-symbol=@var{sym}
@itemx -fno-stack-limit
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index 5faade5..8874cba 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -166,7 +166,14 @@ enum permitted_flt_eval_methods
PERMITTED_FLT_EVAL_METHODS_C11
};
-/* Type of stack check. */
+/* Type of stack check.
+
+ Stack checking is designed to detect infinite recursion for Ada
+ programs. Furthermore stack checking tries to ensure that scenario
+ that enough stack space is left to run a signal handler.
+
+ -fstack-check= does not prevent stack-clash style attacks. For that
+ you want -fstack-clash-protection. */
enum stack_check_type
{
/* Do not check the stack. */
diff --git a/gcc/params.c b/gcc/params.c
index fab0ffa..8afe4c4 100644
--- a/gcc/params.c
+++ b/gcc/params.c
@@ -209,6 +209,11 @@ set_param_value (const char *name, int value,
error ("maximum value of parameter %qs is %u",
compiler_params[i].option,
compiler_params[i].max_value);
+ else if ((strcmp (name, "stack-clash-protection-guard-size") == 0
+ || strcmp (name, "stack-clash-protection-probe-interval") == 0)
+ && exact_log2 (value) == -1)
+ error ("value of parameter %qs must be a power of 2",
+ compiler_params[i].option);
else
set_param_value_internal ((compiler_param) i, value,
params, params_set, true);
diff --git a/gcc/params.def b/gcc/params.def
index 6b07518..2270031 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -213,6 +213,16 @@ DEFPARAM(PARAM_STACK_FRAME_GROWTH,
"Maximal stack frame growth due to inlining (in percent).",
1000, 0, 0)
+DEFPARAM(PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE,
+ "stack-clash-protection-guard-size",
+ "Minimum size (in bytes) of the stack guard, must be a power of 2 >= 4096.",
+ 4096, 4096, 134217728)
+
+DEFPARAM(PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL,
+ "stack-clash-protection-probe-interval",
+ "Interval (in bytes) in which to probe the stack.",
+ 4096, 1024, 65536)
+
/* The GCSE optimization will be disabled if it would require
significantly more memory than this value. */
DEFPARAM(PARAM_MAX_GCSE_MEMORY,
diff --git a/gcc/testsuite/gcc.dg/stack-check-2.c b/gcc/testsuite/gcc.dg/stack-check-2.c
new file mode 100644
index 0000000..196c4bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/stack-check-2.c
@@ -0,0 +1,66 @@
+/* The goal here is to ensure that we never consider a call to a noreturn
+ function as a potential tail call.
+
+ Right now GCC discovers potential tail calls by looking at the
+ predecessors of the exit block. A call to a non-return function
+ has no successors and thus can never match that first filter.
+
+ But that could change one day and we want to catch it. The problem
+ is the compiler could potentially optimize a tail call to a nonreturn
+ function, even if the caller has a frame. That breaks the assumption
+ that calls probe *sp when saving the return address that some targets
+ depend on to elide stack probes. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-clash-protection -fdump-tree-tailc -fdump-tree-optimized" } */
+/* { dg-require-effective-target supports_stack_clash_protection } */
+
+extern void foo (void) __attribute__ ((__noreturn__));
+
+
+void
+test_direct_1 (void)
+{
+ foo ();
+}
+
+void
+test_direct_2 (void)
+{
+ return foo ();
+}
+
+void (*indirect)(void)__attribute__ ((noreturn));
+
+
+void
+test_indirect_1 ()
+{
+ (*indirect)();
+}
+
+void
+test_indirect_2 (void)
+{
+ return (*indirect)();;
+}
+
+
+typedef void (*pvfn)() __attribute__ ((noreturn));
+
+void (*indirect_casted)(void);
+
+void
+test_indirect_casted_1 ()
+{
+ (*(pvfn)indirect_casted)();
+}
+
+void
+test_indirect_casted_2 (void)
+{
+ return (*(pvfn)indirect_casted)();
+}
+/* { dg-final { scan-tree-dump-not "tail call" "tailc" } } */
+/* { dg-final { scan-tree-dump-not "tail call" "optimized" } } */
+
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index fe5e777..25c3c80 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8468,3 +8468,78 @@ proc check_effective_target_arm_coproc4_ok { } {
return [check_cached_effective_target arm_coproc4_ok \
check_effective_target_arm_coproc4_ok_nocache]
}
+
+# Return 1 if the target has support for stack probing designed
+# to avoid stack-clash style attacks.
+#
+# This is used to restrict the stack-clash mitigation tests to
+# just those targets that have been explicitly supported.
+#
+# In addition to the prologue work on those targets, each target's
+# properties should be described in the functions below so that
+# tests do not become a mess of unreadable target conditions.
+#
+proc check_effective_target_supports_stack_clash_protection { } {
+ if { [istarget aarch*-*-*] || [istarget x86_64-*-*]
+ || [istarget i?86-*-*] || [istarget s390*-*-*]
+ || [istarget powerpc*-*-*] || [istarget rs6000*-*-*] } {
+ return 1
+ }
+ return 0
+}
+
+# Return 1 if the target creates a frame pointer for non-leaf functions
+# Note we ignore cases where we apply tail call optimization here.
+proc check_effective_target_frame_pointer_for_non_leaf { } {
+ if { [istarget aarch*-*-*] } {
+ return 1
+ }
+ return 0
+}
+
+# Return 1 if the target's calling sequence or its ABI
+# create implicit stack probes at or prior to function entry.
+proc check_effective_target_caller_implicit_probes { } {
+
+ # On x86/x86_64 the call instruction itself pushes the return
+ # address onto the stack. That is an implicit probe of *sp.
+ if { [istarget x86_64-*-*] || [istarget i?86-*-*] } {
+ return 1
+ }
+
+ # On PPC, the ABI mandates that the address of the outer
+ # frame be stored at *sp. Thus each allocation of stack
+ # space is itself an implicit probe of *sp.
+ if { [istarget powerpc*-*-*] || [istarget rs6000*-*-*] } {
+ return 1
+ }
+
+ # s390's ABI has a register save area allocated by the
+ # caller for use by the callee. The mere existence does
+ # not constitute a probe by the caller, but when the slots
+ # used by the callee those stores are implicit probes.
+ if { [istarget s390*-*-*] } {
+ return 1
+ }
+
+ # Not strictly true on aarch64, but we have agreed that we will
+ # consider any function that pushes SP more than 3kbytes into
+ # the guard page as broken. This essentially means that we can
+ # consider the aarch64 as having a caller implicit probe at
+ # *(sp + 1k).
+ if { [istarget aarch64*-*-*] } {
+ return 1;
+ }
+
+ return 0
+}
+
+# Targets that potentially realign the stack pointer often cause residual
+# stack allocations and make it difficult to elimination loops or residual
+# allocations for dynamic stack allocations
+proc check_effective_target_callee_realigns_stack { } {
+ if { [istarget x86_64-*-*] || [istarget i?86-*-*] } {
+ return 1
+ }
+ return 0
+}
diff --git a/gcc/toplev.c b/gcc/toplev.c
index e6c69a4..24a00df 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1591,6 +1591,26 @@ process_options (void)
flag_associative_math = 0;
}
+ /* -fstack-clash-protection is not currently supported on targets
+ where the stack grows up. */
+ if (flag_stack_clash_protection && !STACK_GROWS_DOWNWARD)
+ {
+ warning_at (UNKNOWN_LOCATION, 0,
+ "-fstack-clash_protection is not supproted on targets "
+ "where the stack grows from lower to higher addresses");
+ flag_stack_clash_protection = 0;
+ }
+
+ /* We can not support -fstack-check= and -fstack-clash-protection at
+ the same time. */
+ if (flag_stack_check != NO_STACK_CHECK && flag_stack_clash_protection)
+ {
+ warning_at (UNKNOWN_LOCATION, 0,
+ "-fstack-check= and -fstack-clash_protection are mutually "
+ "exclusive. Disabling -fstack-check=");
+ flag_stack_check = NO_STACK_CHECK;
+ }
+
/* With -fcx-limited-range, we do cheap and quick complex arithmetic. */
if (flag_cx_limited_range)
flag_complex_method = 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-07-31 5:36 [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3 Jeff Law
@ 2017-08-18 14:36 ` Richard Biener
2017-08-22 16:29 ` Jeff Law
2017-08-20 18:31 ` Martin Sebor
1 sibling, 1 reply; 11+ messages in thread
From: Richard Biener @ 2017-08-18 14:36 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Mon, Jul 31, 2017 at 7:35 AM, Jeff Law <law@redhat.com> wrote:
> This patch introduces the stack clash protection options
>
> Changes since V2:
>
> Adds two new params. The first controls the size of the guard area.
> This controls the threshold for when a function prologue requires
> probes. The second controls the probing interval -- ie, once probes are
> needed, how often do we emit them. These are really meant more for
> developers to experiment with than users. Regardless I did go ahead and
> document them./PARAM
>
> It also adds some sanity checking WRT combining stack clash protection
> with -fstack-check.
diff --git a/gcc/params.c b/gcc/params.c
index fab0ffa..8afe4c4 100644
--- a/gcc/params.c
+++ b/gcc/params.c
@@ -209,6 +209,11 @@ set_param_value (const char *name, int value,
error ("maximum value of parameter %qs is %u",
compiler_params[i].option,
compiler_params[i].max_value);
+ else if ((strcmp (name, "stack-clash-protection-guard-size") == 0
+ || strcmp (name, "stack-clash-protection-probe-interval") == 0)
+ && exact_log2 (value) == -1)
+ error ("value of parameter %qs must be a power of 2",
+ compiler_params[i].option);
else
set_param_value_internal ((compiler_param) i, value,
params, params_set, true);
I don't like this. Either use them as if they were power-of-two
(floor_log2/ceil_log2 as appropriate) or simply make them take
the logarithm instead (like -mincoming-stack-boundary and friends).
Both -fstack-clash-protection and -fstack-check cannot be turned
off per function. This means they would need merging in lto-wrapper.
The alternative is to mark them with 'Optimization' and allow per-function
specification (like we do for -fstack-protector).
Otherwise ok.
Richard.
> Jeff
>
>
> * common.opt (-fstack-clash-protection): New option.
> * flag-types.h (enum stack_check_type): Note difference between
> -fstack-check= and -fstack-clash-protection.
> * params.h (set_param_value): Verify stack-clash-protection
> params are powers of two.
> * params.def (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE): New PARAM.
> (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL): Likewise.
> * toplev.c (process_options): Issue warnings/errors for cases
> not handled with -fstack-clash-protection.
>
>
>
> * doc/invoke.texi (-fstack-clash-protection): Document new option.
> (-fstack-check): Note additional problem with -fstack-check=generic.
> Note that -fstack-check is primarily for Ada and refer users
> to -fstack-clash-protection for stack-clash-protection.
> Document new params for stack clash protection.
>
>
>
>
> * toplev.c (process_options): Handle -fstack-clash-protection.
>
> testsuite/
>
> * gcc.dg/stack-check-2.c: New test.
> * lib/target-supports.exp
> (check_effective_target_supports_stack_clash_protection): New function.
> (check_effective_target_frame_pointer_for_non_leaf): Likewise.
> (check_effective_target_caller_implicit_probes): Likewise.
>
>
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index e81165c..cfaf2bc 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2306,6 +2306,11 @@ fstack-check
> Common Alias(fstack-check=, specific, no)
> Insert stack checking code into the program. Same as -fstack-check=specific.
>
> +fstack-clash-protection
> +Common Report Var(flag_stack_clash_protection)
> +Insert code to probe each page of stack space as it is allocated to protect
> +from stack-clash style attacks
> +
> fstack-limit
> Common Var(common_deferred_options) Defer
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 3e5cee8..8095dc2 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -10128,6 +10128,20 @@ compilation without. The value for compilation with profile feedback
> needs to be more conservative (higher) in order to make tracer
> effective.
>
> +@item stack-clash-protection-guard-size
> +The size (in bytes) of the stack guard. Acceptable values are powers of 2
> +between 4096 and 134217728 and defaults to 4096. Higher values may reduce the
> +number of explicit probes, but a value larger than the operating system
> +provided guard will leave code vulnerable to stack clash style attacks.
> +
> +@item stack-clash-protection-probe-interval
> +Stack clash protection involves probing stack space as it is allocated. This
> +param controls the maximum distance (in bytes) between probes into the stack.
> +Acceptable values are powers of 2 between 4096 and 65536 and defaults to
> +4096. Higher values may reduce the number of explicit probes, but a value
> +larger than the operating system provided guard will leave code vulnerable to
> +stack clash style attacks.
> +
> @item max-cse-path-length
>
> The maximum number of basic blocks on path that CSE considers.
> @@ -11333,7 +11347,8 @@ target support in the compiler but comes with the following drawbacks:
> @enumerate
> @item
> Modified allocation strategy for large objects: they are always
> -allocated dynamically if their size exceeds a fixed threshold.
> +allocated dynamically if their size exceeds a fixed threshold. Note this
> +may change the semantics of some code.
>
> @item
> Fixed limit on the size of the static frame of functions: when it is
> @@ -11348,6 +11363,25 @@ generic implementation, code performance is hampered.
> Note that old-style stack checking is also the fallback method for
> @samp{specific} if no target support has been added in the compiler.
>
> +@samp{-fstack-check=} is designed for Ada's needs to detect infinite recursion
> +and stack overflows. @samp{specific} is an excellent choice when compiling
> +Ada code. It is not generally sufficient to protect against stack-clash
> +attacks. To protect against those you want @samp{-fstack-clash-protection}.
> +
> +@item -fstack-clash-protection
> +@opindex fstack-clash-protection
> +Generate code to prevent stack clash style attacks. When this option is
> +enabled, the compiler will only allocate one page of stack space at a time
> +and each page is accessed immediately after allocation. Thus, it prevents
> +allocations from jumping over any stack guard page provided by the
> +operating system.
> +
> +Most targets do not fully support stack clash protection. However, on
> +those targets @option{-fstack-clash-protection} will protect dynamic stack
> +allocations. @option{-fstack-clash-protection} may also provide limited
> +protection for static stack allocations if the target supports
> +@option{-fstack-check=specific}.
> +
> @item -fstack-limit-register=@var{reg}
> @itemx -fstack-limit-symbol=@var{sym}
> @itemx -fno-stack-limit
> diff --git a/gcc/flag-types.h b/gcc/flag-types.h
> index 5faade5..8874cba 100644
> --- a/gcc/flag-types.h
> +++ b/gcc/flag-types.h
> @@ -166,7 +166,14 @@ enum permitted_flt_eval_methods
> PERMITTED_FLT_EVAL_METHODS_C11
> };
>
> -/* Type of stack check. */
> +/* Type of stack check.
> +
> + Stack checking is designed to detect infinite recursion for Ada
> + programs. Furthermore stack checking tries to ensure that scenario
> + that enough stack space is left to run a signal handler.
> +
> + -fstack-check= does not prevent stack-clash style attacks. For that
> + you want -fstack-clash-protection. */
> enum stack_check_type
> {
> /* Do not check the stack. */
> diff --git a/gcc/params.c b/gcc/params.c
> index fab0ffa..8afe4c4 100644
> --- a/gcc/params.c
> +++ b/gcc/params.c
> @@ -209,6 +209,11 @@ set_param_value (const char *name, int value,
> error ("maximum value of parameter %qs is %u",
> compiler_params[i].option,
> compiler_params[i].max_value);
> + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0
> + || strcmp (name, "stack-clash-protection-probe-interval") == 0)
> + && exact_log2 (value) == -1)
> + error ("value of parameter %qs must be a power of 2",
> + compiler_params[i].option);
> else
> set_param_value_internal ((compiler_param) i, value,
> params, params_set, true);
> diff --git a/gcc/params.def b/gcc/params.def
> index 6b07518..2270031 100644
> --- a/gcc/params.def
> +++ b/gcc/params.def
> @@ -213,6 +213,16 @@ DEFPARAM(PARAM_STACK_FRAME_GROWTH,
> "Maximal stack frame growth due to inlining (in percent).",
> 1000, 0, 0)
>
> +DEFPARAM(PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE,
> + "stack-clash-protection-guard-size",
> + "Minimum size (in bytes) of the stack guard, must be a power of 2 >= 4096.",
> + 4096, 4096, 134217728)
> +
> +DEFPARAM(PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL,
> + "stack-clash-protection-probe-interval",
> + "Interval (in bytes) in which to probe the stack.",
> + 4096, 1024, 65536)
> +
> /* The GCSE optimization will be disabled if it would require
> significantly more memory than this value. */
> DEFPARAM(PARAM_MAX_GCSE_MEMORY,
> diff --git a/gcc/testsuite/gcc.dg/stack-check-2.c b/gcc/testsuite/gcc.dg/stack-check-2.c
> new file mode 100644
> index 0000000..196c4bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/stack-check-2.c
> @@ -0,0 +1,66 @@
> +/* The goal here is to ensure that we never consider a call to a noreturn
> + function as a potential tail call.
> +
> + Right now GCC discovers potential tail calls by looking at the
> + predecessors of the exit block. A call to a non-return function
> + has no successors and thus can never match that first filter.
> +
> + But that could change one day and we want to catch it. The problem
> + is the compiler could potentially optimize a tail call to a nonreturn
> + function, even if the caller has a frame. That breaks the assumption
> + that calls probe *sp when saving the return address that some targets
> + depend on to elide stack probes. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstack-clash-protection -fdump-tree-tailc -fdump-tree-optimized" } */
> +/* { dg-require-effective-target supports_stack_clash_protection } */
> +
> +extern void foo (void) __attribute__ ((__noreturn__));
> +
> +
> +void
> +test_direct_1 (void)
> +{
> + foo ();
> +}
> +
> +void
> +test_direct_2 (void)
> +{
> + return foo ();
> +}
> +
> +void (*indirect)(void)__attribute__ ((noreturn));
> +
> +
> +void
> +test_indirect_1 ()
> +{
> + (*indirect)();
> +}
> +
> +void
> +test_indirect_2 (void)
> +{
> + return (*indirect)();;
> +}
> +
> +
> +typedef void (*pvfn)() __attribute__ ((noreturn));
> +
> +void (*indirect_casted)(void);
> +
> +void
> +test_indirect_casted_1 ()
> +{
> + (*(pvfn)indirect_casted)();
> +}
> +
> +void
> +test_indirect_casted_2 (void)
> +{
> + return (*(pvfn)indirect_casted)();
> +}
> +/* { dg-final { scan-tree-dump-not "tail call" "tailc" } } */
> +/* { dg-final { scan-tree-dump-not "tail call" "optimized" } } */
> +
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index fe5e777..25c3c80 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -8468,3 +8468,78 @@ proc check_effective_target_arm_coproc4_ok { } {
> return [check_cached_effective_target arm_coproc4_ok \
> check_effective_target_arm_coproc4_ok_nocache]
> }
> +
> +# Return 1 if the target has support for stack probing designed
> +# to avoid stack-clash style attacks.
> +#
> +# This is used to restrict the stack-clash mitigation tests to
> +# just those targets that have been explicitly supported.
> +#
> +# In addition to the prologue work on those targets, each target's
> +# properties should be described in the functions below so that
> +# tests do not become a mess of unreadable target conditions.
> +#
> +proc check_effective_target_supports_stack_clash_protection { } {
> + if { [istarget aarch*-*-*] || [istarget x86_64-*-*]
> + || [istarget i?86-*-*] || [istarget s390*-*-*]
> + || [istarget powerpc*-*-*] || [istarget rs6000*-*-*] } {
> + return 1
> + }
> + return 0
> +}
> +
> +# Return 1 if the target creates a frame pointer for non-leaf functions
> +# Note we ignore cases where we apply tail call optimization here.
> +proc check_effective_target_frame_pointer_for_non_leaf { } {
> + if { [istarget aarch*-*-*] } {
> + return 1
> + }
> + return 0
> +}
> +
> +# Return 1 if the target's calling sequence or its ABI
> +# create implicit stack probes at or prior to function entry.
> +proc check_effective_target_caller_implicit_probes { } {
> +
> + # On x86/x86_64 the call instruction itself pushes the return
> + # address onto the stack. That is an implicit probe of *sp.
> + if { [istarget x86_64-*-*] || [istarget i?86-*-*] } {
> + return 1
> + }
> +
> + # On PPC, the ABI mandates that the address of the outer
> + # frame be stored at *sp. Thus each allocation of stack
> + # space is itself an implicit probe of *sp.
> + if { [istarget powerpc*-*-*] || [istarget rs6000*-*-*] } {
> + return 1
> + }
> +
> + # s390's ABI has a register save area allocated by the
> + # caller for use by the callee. The mere existence does
> + # not constitute a probe by the caller, but when the slots
> + # used by the callee those stores are implicit probes.
> + if { [istarget s390*-*-*] } {
> + return 1
> + }
> +
> + # Not strictly true on aarch64, but we have agreed that we will
> + # consider any function that pushes SP more than 3kbytes into
> + # the guard page as broken. This essentially means that we can
> + # consider the aarch64 as having a caller implicit probe at
> + # *(sp + 1k).
> + if { [istarget aarch64*-*-*] } {
> + return 1;
> + }
> +
> + return 0
> +}
> +
> +# Targets that potentially realign the stack pointer often cause residual
> +# stack allocations and make it difficult to elimination loops or residual
> +# allocations for dynamic stack allocations
> +proc check_effective_target_callee_realigns_stack { } {
> + if { [istarget x86_64-*-*] || [istarget i?86-*-*] } {
> + return 1
> + }
> + return 0
> +}
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index e6c69a4..24a00df 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1591,6 +1591,26 @@ process_options (void)
> flag_associative_math = 0;
> }
>
> + /* -fstack-clash-protection is not currently supported on targets
> + where the stack grows up. */
> + if (flag_stack_clash_protection && !STACK_GROWS_DOWNWARD)
> + {
> + warning_at (UNKNOWN_LOCATION, 0,
> + "-fstack-clash_protection is not supproted on targets "
> + "where the stack grows from lower to higher addresses");
> + flag_stack_clash_protection = 0;
> + }
> +
> + /* We can not support -fstack-check= and -fstack-clash-protection at
> + the same time. */
> + if (flag_stack_check != NO_STACK_CHECK && flag_stack_clash_protection)
> + {
> + warning_at (UNKNOWN_LOCATION, 0,
> + "-fstack-check= and -fstack-clash_protection are mutually "
> + "exclusive. Disabling -fstack-check=");
> + flag_stack_check = NO_STACK_CHECK;
> + }
> +
> /* With -fcx-limited-range, we do cheap and quick complex arithmetic. */
> if (flag_cx_limited_range)
> flag_complex_method = 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-07-31 5:36 [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3 Jeff Law
2017-08-18 14:36 ` Richard Biener
@ 2017-08-20 18:31 ` Martin Sebor
2017-08-22 15:58 ` Jeff Law
1 sibling, 1 reply; 11+ messages in thread
From: Martin Sebor @ 2017-08-20 18:31 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 07/30/2017 11:35 PM, Jeff Law wrote:
> This patch introduces the stack clash protection options
>
> Changes since V2:
>
> Adds two new params. The first controls the size of the guard area.
> This controls the threshold for when a function prologue requires
> probes. The second controls the probing interval -- ie, once probes are
> needed, how often do we emit them. These are really meant more for
> developers to experiment with than users. Regardless I did go ahead and
> document them./PARAM
>
> It also adds some sanity checking WRT combining stack clash protection
> with -fstack-check.
Just a minor nit and suggestion:
"supproted" -> "supported"
+ warning_at (UNKNOWN_LOCATION, 0,
+ "-fstack-clash_protection is not supproted on targets "
+ "where the stack grows from lower to higher addresses");
and quote the name of the options in diagnostics, i.e., use either
"%<"-fstack-clash_protection%> ..."
or
"%qs is not supported...", "-fstack-clash_protection"
as you did in error ("value of parameter %qs must be a power of 2",
ompiler_params[i].option);
Likewise in
+ warning_at (UNKNOWN_LOCATION, 0,
+ "-fstack-check= and -fstack-clash_protection are mutually "
+ "exclusive. Disabling -fstack-check=");
Martin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-20 18:31 ` Martin Sebor
@ 2017-08-22 15:58 ` Jeff Law
2017-08-22 16:39 ` David Malcolm
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-08-22 15:58 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
On 08/19/2017 12:22 PM, Martin Sebor wrote:
> On 07/30/2017 11:35 PM, Jeff Law wrote:
>> This patch introduces the stack clash protection options
>>
>> Changes since V2:
>>
>> Adds two new params. The first controls the size of the guard area.
>> This controls the threshold for when a function prologue requires
>> probes. The second controls the probing interval -- ie, once probes are
>> needed, how often do we emit them. These are really meant more for
>> developers to experiment with than users. Regardless I did go ahead and
>> document them./PARAM
>>
>> It also adds some sanity checking WRT combining stack clash protection
>> with -fstack-check.
>
> Just a minor nit and suggestion:
>
> "supproted" -> "supported"
>
> + warning_at (UNKNOWN_LOCATION, 0,
> + "-fstack-clash_protection is not supproted on targets "
> + "where the stack grows from lower to higher addresses");
>
> and quote the name of the options in diagnostics, i.e., use either
>
> "%<"-fstack-clash_protection%> ..."
>
> or
>
> "%qs is not supported...", "-fstack-clash_protection"
>
> as you did in error ("value of parameter %qs must be a power of 2",
> ompiler_params[i].option);
>
> Likewise in
>
> + warning_at (UNKNOWN_LOCATION, 0,
> + "-fstack-check= and -fstack-clash_protection are mutually "
> + "exclusive. Disabling -fstack-check=");
Thanks. I settled on the %< %> style. None of the other warnings in
that area use either. Otherwise I would have just selected whatever was
most commonly used in that code.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-18 14:36 ` Richard Biener
@ 2017-08-22 16:29 ` Jeff Law
2017-08-23 9:40 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-08-22 16:29 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On 08/18/2017 08:01 AM, Richard Biener wrote:
> On Mon, Jul 31, 2017 at 7:35 AM, Jeff Law <law@redhat.com> wrote:
>> This patch introduces the stack clash protection options
>>
>> Changes since V2:
>>
>> Adds two new params. The first controls the size of the guard area.
>> This controls the threshold for when a function prologue requires
>> probes. The second controls the probing interval -- ie, once probes are
>> needed, how often do we emit them. These are really meant more for
>> developers to experiment with than users. Regardless I did go ahead and
>> document them./PARAM
>>
>> It also adds some sanity checking WRT combining stack clash protection
>> with -fstack-check.
>
> diff --git a/gcc/params.c b/gcc/params.c
> index fab0ffa..8afe4c4 100644
> --- a/gcc/params.c
> +++ b/gcc/params.c
> @@ -209,6 +209,11 @@ set_param_value (const char *name, int value,
> error ("maximum value of parameter %qs is %u",
> compiler_params[i].option,
> compiler_params[i].max_value);
> + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0
> + || strcmp (name, "stack-clash-protection-probe-interval") == 0)
> + && exact_log2 (value) == -1)
> + error ("value of parameter %qs must be a power of 2",
> + compiler_params[i].option);
> else
> set_param_value_internal ((compiler_param) i, value,
> params, params_set, true);
>
> I don't like this. Either use them as if they were power-of-two
> (floor_log2/ceil_log2 as appropriate) or simply make them take
> the logarithm instead (like -mincoming-stack-boundary and friends).
Yes. I was torn on this for a variety of reasons, including the fact
that I don't actually expect anyone to be mucking with those :-)
Given we've already other stuff in log2 form, I'll use that -- it seems
less surprising to me than using floor/ceil.
>
> Both -fstack-clash-protection and -fstack-check cannot be turned
> off per function. This means they would need merging in lto-wrapper.
> The alternative is to mark them with 'Optimization' and allow per-function
> specification (like we do for -fstack-protector).
Do you have a strong preference here? I'd tend to go with tweaking
lto-wrapper as we really don't want to have this stuff varying per-function.
Presumably in lto-wrapper we just have to detect that both were enabled
and do something sensible. We drop -fstack-check in toplev.c when both
are specified, we could just as easily call that situation a fatal error
in both toplev.c and lto-wrapper.c
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-22 15:58 ` Jeff Law
@ 2017-08-22 16:39 ` David Malcolm
2017-08-22 17:32 ` Jeff Law
0 siblings, 1 reply; 11+ messages in thread
From: David Malcolm @ 2017-08-22 16:39 UTC (permalink / raw)
To: Jeff Law, Martin Sebor, gcc-patches
On Tue, 2017-08-22 at 09:35 -0600, Jeff Law wrote:
> On 08/19/2017 12:22 PM, Martin Sebor wrote:
> > On 07/30/2017 11:35 PM, Jeff Law wrote:
> > > This patch introduces the stack clash protection options
> > >
> > > Changes since V2:
> > >
> > > Adds two new params. The first controls the size of the guard
> > > area.
> > > This controls the threshold for when a function prologue requires
> > > probes. The second controls the probing interval -- ie, once
> > > probes are
> > > needed, how often do we emit them. These are really meant more
> > > for
> > > developers to experiment with than users. Regardless I did go
> > > ahead and
> > > document them./PARAM
> > >
> > > It also adds some sanity checking WRT combining stack clash
> > > protection
> > > with -fstack-check.
> >
> > Just a minor nit and suggestion:
> >
> > "supproted" -> "supported"
> >
> > + warning_at (UNKNOWN_LOCATION, 0,
> > + "-fstack-clash_protection is not supproted on targets "
> > + "where the stack grows from lower to higher addresses");
> >
> > and quote the name of the options in diagnostics, i.e., use either
> >
> > "%<"-fstack-clash_protection%> ..."
> >
> > or
> >
> > "%qs is not supported...", "-fstack-clash_protection"
> >
> > as you did in error ("value of parameter %qs must be a power of 2",
> > ompiler_params[i].option);
> >
> > Likewise in
> >
> > + warning_at (UNKNOWN_LOCATION, 0,
> > + "-fstack-check= and -fstack-clash_protection are
> > mutually "
> > + "exclusive. Disabling -fstack-check=");
>
> Thanks. I settled on the %< %> style. None of the other warnings in
> that area use either. Otherwise I would have just selected whatever
> was
> most commonly used in that code.
A nit that don't seem to have been mentioned: the patch refers to
-fstack-clash_protection (erroneous underscore)
in two places, which should be:
-fstack-clash-protection (all dashes)
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-22 16:39 ` David Malcolm
@ 2017-08-22 17:32 ` Jeff Law
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-08-22 17:32 UTC (permalink / raw)
To: David Malcolm, Martin Sebor, gcc-patches
On 08/22/2017 10:26 AM, David Malcolm wrote:
>> Thanks. I settled on the %< %> style. None of the other warnings in
>> that area use either. Otherwise I would have just selected whatever
>> was
>> most commonly used in that code.
>
> A nit that don't seem to have been mentioned: the patch refers to
> -fstack-clash_protection (erroneous underscore)
> in two places, which should be:
> -fstack-clash-protection (all dashes)
I already fixed that locally :-)
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-22 16:29 ` Jeff Law
@ 2017-08-23 9:40 ` Richard Biener
2017-08-23 17:16 ` Jeff Law
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2017-08-23 9:40 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Tue, Aug 22, 2017 at 5:52 PM, Jeff Law <law@redhat.com> wrote:
> On 08/18/2017 08:01 AM, Richard Biener wrote:
>> On Mon, Jul 31, 2017 at 7:35 AM, Jeff Law <law@redhat.com> wrote:
>>> This patch introduces the stack clash protection options
>>>
>>> Changes since V2:
>>>
>>> Adds two new params. The first controls the size of the guard area.
>>> This controls the threshold for when a function prologue requires
>>> probes. The second controls the probing interval -- ie, once probes are
>>> needed, how often do we emit them. These are really meant more for
>>> developers to experiment with than users. Regardless I did go ahead and
>>> document them./PARAM
>>>
>>> It also adds some sanity checking WRT combining stack clash protection
>>> with -fstack-check.
>>
>> diff --git a/gcc/params.c b/gcc/params.c
>> index fab0ffa..8afe4c4 100644
>> --- a/gcc/params.c
>> +++ b/gcc/params.c
>> @@ -209,6 +209,11 @@ set_param_value (const char *name, int value,
>> error ("maximum value of parameter %qs is %u",
>> compiler_params[i].option,
>> compiler_params[i].max_value);
>> + else if ((strcmp (name, "stack-clash-protection-guard-size") == 0
>> + || strcmp (name, "stack-clash-protection-probe-interval") == 0)
>> + && exact_log2 (value) == -1)
>> + error ("value of parameter %qs must be a power of 2",
>> + compiler_params[i].option);
>> else
>> set_param_value_internal ((compiler_param) i, value,
>> params, params_set, true);
>>
>> I don't like this. Either use them as if they were power-of-two
>> (floor_log2/ceil_log2 as appropriate) or simply make them take
>> the logarithm instead (like -mincoming-stack-boundary and friends).
> Yes. I was torn on this for a variety of reasons, including the fact
> that I don't actually expect anyone to be mucking with those :-)
>
> Given we've already other stuff in log2 form, I'll use that -- it seems
> less surprising to me than using floor/ceil.
>
>>
>> Both -fstack-clash-protection and -fstack-check cannot be turned
>> off per function. This means they would need merging in lto-wrapper.
>> The alternative is to mark them with 'Optimization' and allow per-function
>> specification (like we do for -fstack-protector).
> Do you have a strong preference here? I'd tend to go with tweaking
> lto-wrapper as we really don't want to have this stuff varying per-function.
Any reason? I see no technical one at least, it might break IPA assumptions
you make when instrumenting? I guess you just don't want to think about
the implications when mixing them (or mixing -fstack-clash-protection
with -fno-stack-clash-protection)?
> Presumably in lto-wrapper we just have to detect that both were enabled
> and do something sensible. We drop -fstack-check in toplev.c when both
> are specified, we could just as easily call that situation a fatal error
> in both toplev.c and lto-wrapper.c
So what happens if you LTO link Ada (-fstack-check) with C
(-fstack-clash-protection)?
The user is already allowed to (without LTO) specify things on a CU granularity.
Richard.
>
> jeff
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-23 9:40 ` Richard Biener
@ 2017-08-23 17:16 ` Jeff Law
2017-08-23 17:31 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2017-08-23 17:16 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On 08/23/2017 03:07 AM, Richard Biener wrote:
>>> Both -fstack-clash-protection and -fstack-check cannot be turned
>>> off per function. This means they would need merging in lto-wrapper.
>>> The alternative is to mark them with 'Optimization' and allow per-function
>>> specification (like we do for -fstack-protector).
>> Do you have a strong preference here? I'd tend to go with tweaking
>> lto-wrapper as we really don't want to have this stuff varying per-function.
>
> Any reason? I see no technical one at least, it might break IPA assumptions
> you make when instrumenting? I guess you just don't want to think about
> the implications when mixing them (or mixing -fstack-clash-protection
> with -fno-stack-clash-protection)?
On some targets (s390 for example), a stack guard jump can be
accomplished by a combination of caller and callee stack adjustments --
even though neither the caller nor the callee have a large enough
adjustment individually to jump the guard.
So a mis-informed developer could audit their code, declare various
functions as safe & turn off protections on a per-function basis based
on that analysis. But that would be mis-guided as it opens them up to a
potential attack where the combination of caller and callee stack
adjustments are used to jump the guard.
>
>> Presumably in lto-wrapper we just have to detect that both were enabled
>> and do something sensible. We drop -fstack-check in toplev.c when both
>> are specified, we could just as easily call that situation a fatal error
>> in both toplev.c and lto-wrapper.c
>
> So what happens if you LTO link Ada (-fstack-check) with C
> (-fstack-clash-protection)?
When in the Ada code you'll be mostly protected from stack clashes and
you can detect stack overflows and catch the signal properly. When in C
code, you'd be fully protected from stack clash, but not necessarily
able to take a signal.
At the boundaries where you call from Ada to C, you're potentially
vulnerable to stack clash on architectures like aarch64/s390. When
calling from C into Ada, you're not guaranteed to be able to handle a
signal when you overflow the stack.
> The user is already allowed to (without LTO) specify things on a CU
granularity.
True. And you end up with a mixture of protections depending on what
code is executing or what boundary you're crossing.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-23 17:16 ` Jeff Law
@ 2017-08-23 17:31 ` Richard Biener
2017-09-08 22:00 ` Jeff Law
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2017-08-23 17:31 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On August 23, 2017 6:36:32 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>On 08/23/2017 03:07 AM, Richard Biener wrote:
>
>>>> Both -fstack-clash-protection and -fstack-check cannot be turned
>>>> off per function. This means they would need merging in
>lto-wrapper.
>>>> The alternative is to mark them with 'Optimization' and allow
>per-function
>>>> specification (like we do for -fstack-protector).
>>> Do you have a strong preference here? I'd tend to go with tweaking
>>> lto-wrapper as we really don't want to have this stuff varying
>per-function.
>>
>> Any reason? I see no technical one at least, it might break IPA
>assumptions
>> you make when instrumenting? I guess you just don't want to think
>about
>> the implications when mixing them (or mixing -fstack-clash-protection
>> with -fno-stack-clash-protection)?
>On some targets (s390 for example), a stack guard jump can be
>accomplished by a combination of caller and callee stack adjustments --
>even though neither the caller nor the callee have a large enough
>adjustment individually to jump the guard.
>
>So a mis-informed developer could audit their code, declare various
>functions as safe & turn off protections on a per-function basis based
>on that analysis. But that would be mis-guided as it opens them up to
>a
>potential attack where the combination of caller and callee stack
>adjustments are used to jump the guard.
>
>
>
>
>
>>
>>> Presumably in lto-wrapper we just have to detect that both were
>enabled
>>> and do something sensible. We drop -fstack-check in toplev.c when
>both
>>> are specified, we could just as easily call that situation a fatal
>error
>>> in both toplev.c and lto-wrapper.c
>>
>> So what happens if you LTO link Ada (-fstack-check) with C
>> (-fstack-clash-protection)?
>When in the Ada code you'll be mostly protected from stack clashes and
>you can detect stack overflows and catch the signal properly. When in
>C
>code, you'd be fully protected from stack clash, but not necessarily
>able to take a signal.
>
>At the boundaries where you call from Ada to C, you're potentially
>vulnerable to stack clash on architectures like aarch64/s390. When
>calling from C into Ada, you're not guaranteed to be able to handle a
>signal when you overflow the stack.
>
>> The user is already allowed to (without LTO) specify things on a CU
>granularity.
>True. And you end up with a mixture of protections depending on what
>code is executing or what boundary you're crossing.
So the natural thing for LTO ist to make it function granular. That way you don't need any magic in LTO-wrapper.
Richard.
>
>Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3
2017-08-23 17:31 ` Richard Biener
@ 2017-09-08 22:00 ` Jeff Law
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2017-09-08 22:00 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On 08/23/2017 10:39 AM, Richard Biener wrote:
> On August 23, 2017 6:36:32 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>> On 08/23/2017 03:07 AM, Richard Biener wrote:
>>
>>>>> Both -fstack-clash-protection and -fstack-check cannot be turned
>>>>> off per function. This means they would need merging in
>> lto-wrapper.
>>>>> The alternative is to mark them with 'Optimization' and allow
>> per-function
>>>>> specification (like we do for -fstack-protector).
>>>> Do you have a strong preference here? I'd tend to go with tweaking
>>>> lto-wrapper as we really don't want to have this stuff varying
>> per-function.
>>>
>>> Any reason? I see no technical one at least, it might break IPA
>> assumptions
>>> you make when instrumenting? I guess you just don't want to think
>> about
>>> the implications when mixing them (or mixing -fstack-clash-protection
>>> with -fno-stack-clash-protection)?
>> On some targets (s390 for example), a stack guard jump can be
>> accomplished by a combination of caller and callee stack adjustments --
>> even though neither the caller nor the callee have a large enough
>> adjustment individually to jump the guard.
>>
>> So a mis-informed developer could audit their code, declare various
>> functions as safe & turn off protections on a per-function basis based
>> on that analysis. But that would be mis-guided as it opens them up to
>> a
>> potential attack where the combination of caller and callee stack
>> adjustments are used to jump the guard.
>>
>>
>>
>>
>>
>>>
>>>> Presumably in lto-wrapper we just have to detect that both were
>> enabled
>>>> and do something sensible. We drop -fstack-check in toplev.c when
>> both
>>>> are specified, we could just as easily call that situation a fatal
>> error
>>>> in both toplev.c and lto-wrapper.c
>>>
>>> So what happens if you LTO link Ada (-fstack-check) with C
>>> (-fstack-clash-protection)?
>> When in the Ada code you'll be mostly protected from stack clashes and
>> you can detect stack overflows and catch the signal properly. When in
>> C
>> code, you'd be fully protected from stack clash, but not necessarily
>> able to take a signal.
>>
>> At the boundaries where you call from Ada to C, you're potentially
>> vulnerable to stack clash on architectures like aarch64/s390. When
>> calling from C into Ada, you're not guaranteed to be able to handle a
>> signal when you overflow the stack.
>>
>>> The user is already allowed to (without LTO) specify things on a CU
>> granularity.
>> True. And you end up with a mixture of protections depending on what
>> code is executing or what boundary you're crossing.
>
> So the natural thing for LTO ist to make it function granular. That way you don't need any magic in LTO-wrapper.
Done.
jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-09-08 22:00 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-31 5:36 [PATCH][RFA/RFC] Stack clash mitigation patch 01/08 - V3 Jeff Law
2017-08-18 14:36 ` Richard Biener
2017-08-22 16:29 ` Jeff Law
2017-08-23 9:40 ` Richard Biener
2017-08-23 17:16 ` Jeff Law
2017-08-23 17:31 ` Richard Biener
2017-09-08 22:00 ` Jeff Law
2017-08-20 18:31 ` Martin Sebor
2017-08-22 15:58 ` Jeff Law
2017-08-22 16:39 ` David Malcolm
2017-08-22 17:32 ` Jeff Law
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).