[PATCH][RFA/RFC] Stack clash mitigation patch 01/08
Jeff Law
law@redhat.com
Tue Jul 11 21:20:00 GMT 2017
This is the first patch in the stack-clash mitigation patches.
It introduces a new style of stack probing -fstack-check=clash,
including documentation of the new option, how it differs from
-fstack-check=specific, etc.
FWIW -fstack-check=specific is dreadfully named. I haven't tried to
address that.
It also introduces some dejagnu bits that are later used in tests. The
idea was to introduce dejagnu functions which describe aspects of the
target and have the tests adjust their expectations based on those
dejagnu functions rather than on a target name.
Finally, this patch introduces one new test of note. Some targets have
call instructions that store a return pointer into the stack and we take
advantage of that ISA feature to avoid some explicit probes.
This optimization is restricted to cases where the caller does not have
a frame of its own (because there's no reasonable way to tear that frame
down on the return path).
However, a sufficiently smart compiler could realize that a call to a
noreturn function could be converted into a jump, even if the caller has
a frame because that frame need not be torn down. Thus it would be
possible for a function calling a noreturn function to advance the stack
into the guard without actually touching the guard page, which breaks
the assumption that the call instruction would touch the guard
triggering a fault for that case.
GCC doesn't currently optimize that case for various reasons, but it
seemed prudent to go ahead and explicitly verify that with a test.
Thoughts? Ok for the trunk?
-------------- next part --------------
* common.opt (-fstack-check=clash): New option.
* flag-types.h (enum stack_check_type): Improve comments.
(STACK_CLASH_BUILTIN_STACK_CHECK): New stack_check_type.
* opts.c (common_handle_option): Handle -fstack-check=clash.
* doc/invoke.texi (-fstack-check=clash): Document new option.
(-fstack-check): Note additional problem with -fstack-check=generic.
Note differences between "clash" and "specific", fallbacks
and recommendations based on expected use.
testsuite/
* gcc.dg/stack-check-2.c: New test.
* lib/target-supports.exp
(check_effective_target_stack_clash_protected): New function.
(check_effective_target_frame_pointer_for_non_leaf): Likewise.
(check_effective_target_caller_implicit_probes): Likewise.
commit 018fffb569512eccd6b77410e28caa159009df5d
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date: Thu Jun 29 16:36:21 2017 -0400
Recongize new option + test of noreturn functions
Dejagnu primitivies for stack probe checking
diff --git a/gcc/common.opt b/gcc/common.opt
index e81165c..8eec29f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2300,7 +2300,7 @@ Apply variable expansion when loops are unrolled.
fstack-check=
Common Report RejectNegative Joined
--fstack-check=[no|generic|specific] Insert stack checking code into the program.
+-fstack-check=[no|generic|clash|specific] Insert stack checking code into the program.
fstack-check
Common Alias(fstack-check=, specific, no)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3e5cee8..e72f3e9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11324,8 +11324,9 @@ generation of code to ensure that they see the stack being extended.
You can additionally specify a string parameter: @samp{no} means no
checking, @samp{generic} means force the use of old-style checking,
-@samp{specific} means use the best checking method and is equivalent
-to bare @option{-fstack-check}.
+@samp{clash} means use a checking method designed to prevent stack clash
+style attacks, @samp{specific} means use target specific
+checking methods and is equivalent to bare @option{-fstack-check}.
Old-style checking is a generic mechanism that requires no specific
target support in the compiler but comes with the following drawbacks:
@@ -11333,7 +11334,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
@@ -11345,8 +11347,25 @@ Inefficiency: because of both the modified allocation strategy and the
generic implementation, code performance is hampered.
@end enumerate
-Note that old-style stack checking is also the fallback method for
-@samp{specific} if no target support has been added in the compiler.
+Note that old-style stack checking is the fallback method for @samp{clash}
+and @samp{specific} if no target support for either of those has been added
+in the compiler.
+
+Also note that @samp{clash} requires target dependent prologue sequences that
+are different than @samp{specific}. However, some targets may use
+@samp{specific} style prologues if they have not defined @samp{clash} style
+prologues at the loss of some functionality (such as @command{valgrind}) and
+protection in certain difficult to trigger corner cases.
+
+@samp{specific} style checking 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 attacks that jump the stack guard page.
+
+@samp{clash} is designed to prevent jumping the stack guard page as seen in
+stack clash style attacks. However, it does not guarantee sufficient stack
+space to handle a signal in the event that the program hits the stack guard
+and is thus incompatible with Ada's needs.
@item -fstack-limit-register=@var{reg}
@itemx -fstack-limit-symbol=@var{sym}
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index 5faade5..9fad822 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -178,11 +178,27 @@ enum stack_check_type
/* Check the stack and rely on the target configuration files to
check the static frame of functions, i.e. use the generic
- mechanism only for dynamic stack allocations. */
+ mechanism only for dynamic stack allocations.
+
+ This approach attempts to guarantee enough stack space is always
+ available to handle a signal and assumes the entire program is
+ compiled with stack checking. */
STATIC_BUILTIN_STACK_CHECK,
+ /* Check the stack and rely on the target configuration files to
+ check the static frame of functions, i.e. use the generic
+ mechanism only for dynamic stack allocations.
+
+ This approach attempts to make code immune to attacks which jump
+ the stack guard page and stack clash style attacks. It works
+ is mixed code, but does not guarantee the ability to handle a
+ signal. */
+ STACK_CLASH_BUILTIN_STACK_CHECK,
+
/* Check the stack and entirely rely on the target configuration
- files, i.e. do not use the generic mechanism at all. */
+ files, i.e. do not use the generic mechanism at all. This
+ does not prevent stack guard jumping and stack clash style
+ attacks. */
FULL_BUILTIN_STACK_CHECK
};
diff --git a/gcc/opts.c b/gcc/opts.c
index 7460c2b..61f5bb0 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2243,6 +2243,19 @@ common_handle_option (struct gcc_options *opts,
opts->x_flag_stack_check = STACK_CHECK_BUILTIN
? FULL_BUILTIN_STACK_CHECK
: GENERIC_STACK_CHECK;
+ else if (!strcmp (arg, "clash"))
+ {
+ /* This is the stack checking method, designed to prevent
+ stack-clash attacks. */
+ if (!STACK_GROWS_DOWNWARD)
+ sorry ("-fstack-check=clash not implemented on this target");
+ else
+ opts->x_flag_stack_check = (STACK_CHECK_BUILTIN
+ ? FULL_BUILTIN_STACK_CHECK
+ : (STACK_CHECK_STATIC_BUILTIN
+ ? STACK_CLASH_BUILTIN_STACK_CHECK
+ : GENERIC_STACK_CHECK));
+ }
else if (!strcmp (arg, "specific"))
/* This is the new stack checking method. */
opts->x_flag_stack_check = STACK_CHECK_BUILTIN
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..8be7dee
--- /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-check=clash -fdump-tree-tailc -fdump-tree-optimized" } */
+/* { dg-require-effective-target stack_clash_protected } */
+
+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..61ffb68 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8468,3 +8468,61 @@ 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_stack_clash_protected { } {
+ 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 *sp at function
+# entry.
+proc check_effective_target_caller_implicit_probes { } {
+ if { [istarget x86_64-*-*] || [istarget i?86-*-*]
+ || [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
+ }
+
+ return 0
+}
+
+# The stack realignment often causes residual stack allocations and
+# can also make it difficult to elide loops or residual allocations
+# for dynamic allocations
+proc check_effective_target_callee_realigns_stack { } {
+ if { [istarget x86_64-*-*] || [istarget i?86-*-*] } {
+ return 1
+ }
+ return 0
+}
More information about the Gcc-patches
mailing list