This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][RFA/RFC] Stack clash mitigation patch 01/08 V2


The biggest change in this update to patch 01/08 is moving of stack
clash protection out of -fstack-check= and into its own option,
-fstack-clash-protection.  I believe other issues raised by reviewers
have been addressed as well.

--

This patch introduces a new option -fstack-clash-protection designed to
protect the code GCC generates against stack-clash style attacks.

This patch also introduces 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?



	* common.opt (-fstack-clash-protection): New option.
	* flag-types.h (enum stack_check_type): Note difference between
	-fstack-check= and -fstack-clash-protection.
	* toplev.c (process_options): Handle -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.
	(-fstack-clash-protection): Document.

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..bf1298d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11333,7 +11333,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 +11349,19 @@ 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.
+
 @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/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;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]