Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Feb 2014 01:15:56 GMT
From:      Olivier <software-freebsd@interfasys.ch>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   ports/186852: [patch] Add 'fstack-protector-strong' to lang/gcc48
Message-ID:  <201402180115.s1I1FuFc096668@cgiserv.freebsd.org>
Resent-Message-ID: <201402180120.s1I1K0an013892@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         186852
>Category:       ports
>Synopsis:       [patch] Add 'fstack-protector-strong' to lang/gcc48
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-ports-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Tue Feb 18 01:20:00 UTC 2014
>Closed-Date:
>Last-Modified:
>Originator:     Olivier
>Release:        9.2
>Organization:
interfaSys sàrl
>Environment:
FreeBSD 9.2-RELEASE-p2 #0 r259303
>Description:
FreeBSD has recently added the WITH_SSP_PORTS makefile option [1] which adds "-fstack-protector" and "-fstack-protector-all" compilation directives which adds extra code to check for buffer overflows to compiled ports [2].

While this is a good first step, those switches offer too little or too much protection and over 2 years ago, Google delivered "-fstack-protector-strong" [3] to try and solve the problem:
"The stack-protector option is over-simplified, which ignores pointer cast, address computation, while the stack-protector-all is over-killing, using this option brings too much performance overhead..." [4]

The new directive has been added to the 4.9 branch of gcc and several Linux distros have backported the patch, but so far, FreeBSD maintainer has decided not to do it, so here are patches which work against lang/gcc48.
There are based on the gcc 4.9 commits [5]

[1]https://wiki.freebsd.org/201309DevSummit/Ports
[2]http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/Optimize-Options.html
[3]https://codereview.appspot.com/5461043/
[4]https://docs.google.com/document/d/1xXBH6rRZue4f296vGt9YQcuLVQHeE516stHwt8M9xyU/edit?hl=en_US
[5]http://repo.or.cz/w/official-gcc.git/commitdiff/b156ec373ccf27f4fcce7972de5e043d35acea43
>How-To-Repeat:

>Fix:
Patch lang/gcc48

Patch attached with submission follows:

# This is a shell archive.  Save it in a file, remove anything before
# this line, and then unpack it by entering "sh file".  Note, it may
# create directories; files and directories will be owned by you and
# have default permissions.
#
# This archive contains:
#
#	files/
#	files/patch-spp-gcc_testsuite_gcc.dg_fstack-protector-strong.c
#	files/patch-spp-gcc_c-family_c-cppbuiltin.c
#	files/patch-spp-gcc_common.opt
#	files/patch-spp-gcc_doc_invoke.texi
#	files/patch-spp-gcc_gcc.c
#	files/patch-spp-gcc_testsuite_g++.dg_fstack-protector-strong.C
#	files/patch-spp-gcc_cfgexpand.c
#	files/patch-spp-gcc_doc_cpp.texi
#
echo c - files/
mkdir -p files/ > /dev/null 2>&1
echo x - files/patch-spp-gcc_testsuite_gcc.dg_fstack-protector-strong.c
sed 's/^X//' >files/patch-spp-gcc_testsuite_gcc.dg_fstack-protector-strong.c << '5efd84437f680fb94c6ab18572545f84'
X--- /dev/null
X+++ gcc/testsuite/gcc.dg/fstack-protector-strong.c
X@@ -0,0 +1,135 @@
X+/* Test that stack protection is done on chosen functions. */
X+
X+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
X+/* { dg-options "-O2 -fstack-protector-strong" } */
X+
X+#include<string.h>
X+#include<stdlib.h>
X+
X+extern int g0;
X+extern int* pg0;
X+int
X+goo (int *);
X+int
X+hoo (int);
X+
X+/* Function frame address escaped function call. */
X+int
X+foo1 ()
X+{
X+  int i;
X+  return goo (&i);
X+}
X+
X+struct ArrayStruct
X+{
X+  int a;
X+  int array[10];
X+};
X+
X+struct AA
X+{
X+  int b;
X+  struct ArrayStruct as;
X+};
X+
X+/* Function frame contains array. */
X+int
X+foo2 ()
X+{
X+  struct AA aa;
X+  int i;
X+  for (i = 0; i < 10; ++i)
X+    {
X+      aa.as.array[i] = i * (i-1) + i / 2;
X+    }
X+  return aa.as.array[5];
X+}
X+
X+/* Address computation based on a function frame address. */
X+int
X+foo3 ()
X+{
X+  int a;
X+  int *p;
X+  p = &a + 5;
X+  return goo (p);
X+}
X+
X+/* Address cast based on a function frame address. */
X+int
X+foo4 ()
X+{
X+  int a;
X+  return goo (g0 << 2 ? (int *)(3 * (long)(void *)(&a)) : 0);
X+}
X+
X+/* Address cast based on a local array. */
X+int
X+foo5 ()
X+{
X+  short array[10];
X+  return goo ((int *)(array + 5));
X+}
X+
X+struct BB
X+{
X+  int one;
X+  int two;
X+  int three;
X+};
X+
X+/* Address computaton based on a function frame address.*/
X+int
X+foo6 ()
X+{
X+  struct BB bb;
X+  return goo (&bb.one + sizeof(int));
X+}
X+
X+/* Function frame address escaped via global variable. */
X+int
X+foo7 ()
X+{
X+  int a;
X+  pg0 = &a;
X+  goo (pg0);
X+  return *pg0;
X+}
X+
X+/* Check that this covers -fstack-protector. */
X+int
X+foo8 ()
X+{
X+  char base[100];
X+  memcpy ((void *)base, (const void *)pg0, 105);
X+  return (int)(base[32]);
X+}
X+
X+/* Check that this covers -fstack-protector. */
X+int
X+foo9 ()
X+{
X+  char* p = alloca (100);
X+  return goo ((int *)(p + 50));
X+}
X+
X+int
X+global2 (struct BB* pbb);
X+
X+/* Address taken on struct. */
X+int
X+foo10 ()
X+{
X+  struct BB bb;
X+  int i;
X+  bb.one = global2 (&bb);
X+  for (i = 0; i < 10; ++i)
X+    {
X+      bb.two = bb.one + bb.two;
X+      bb.three = bb.one + bb.two + bb.three;
X+    }
X+  return bb.three;
X+}
X+
X+/* { dg-final { scan-assembler-times "stack_chk_fail" 10 } } */
5efd84437f680fb94c6ab18572545f84
echo x - files/patch-spp-gcc_c-family_c-cppbuiltin.c
sed 's/^X//' >files/patch-spp-gcc_c-family_c-cppbuiltin.c << '53d0fafd10387edcb001a309cd997d90'
X--- gcc/c-family/c-cppbuiltin.c.orig
X+++ gcc/c-family/c-cppbuiltin.c
X@@ -888,6 +888,8 @@ c_cpp_builtins (cpp_reader *pfile)
X   /* Make the choice of the stack protector runtime visible to source code.
X      The macro names and values here were chosen for compatibility with an
X      earlier implementation, i.e. ProPolice.  */
X+  if (flag_stack_protect == 3)
X+    cpp_define (pfile, "__SSP_STRONG__=3");
X   if (flag_stack_protect == 2)
X     cpp_define (pfile, "__SSP_ALL__=2");
X   else if (flag_stack_protect == 1)
53d0fafd10387edcb001a309cd997d90
echo x - files/patch-spp-gcc_common.opt
sed 's/^X//' >files/patch-spp-gcc_common.opt << '5479a21560f42180e388d5edf83a2b73'
X--- gcc/common.opt.orig
X+++ gcc/common.opt
X@@ -1942,6 +1942,10 @@ fstack-protector-all
X Common Report RejectNegative Var(flag_stack_protect, 2)
X Use a stack protection method for every function
X 
X+fstack-protector-strong
X+Common Report RejectNegative Var(flag_stack_protect, 3)
X+Use a smart stack protection method for certain functions
X+
X fstack-usage
X Common RejectNegative Var(flag_stack_usage)
X Output stack usage information on a per-function basis
5479a21560f42180e388d5edf83a2b73
echo x - files/patch-spp-gcc_doc_invoke.texi
sed 's/^X//' >files/patch-spp-gcc_doc_invoke.texi << 'a4bec8dda17ef79e9206fa45f2532876'
X--- gcc/doc/invoke.texi.orig
X+++ gcc/doc/invoke.texi
X@@ -407,8 +407,8 @@ Objective-C and Objective-C++ Dialects}.
X -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol
X -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol
X -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol
X--fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol
X--fthread-jumps -ftracer -ftree-bit-ccp @gol
X+-fstack-protector-all -fstack-protector-strong -fstrict-aliasing @gol
X+-fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
X -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol
X -ftree-coalesce-inline-vars -ftree-coalesce-vars -ftree-copy-prop @gol
X -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol
X@@ -8957,6 +8957,12 @@ If a guard check fails, an error message is printed and the program exits.
X @opindex fstack-protector-all
X Like @option{-fstack-protector} except that all functions are protected.
X 
X+@item -fstack-protector-strong
X+@opindex fstack-protector-strong
X+Like @option{-fstack-protector} but includes additional functions to
X+be protected --- those that have local array definitions, or have
X+references to local frame addresses.
X+
X @item -fsection-anchors
X @opindex fsection-anchors
X Try to reduce the number of symbolic address calculations by using
a4bec8dda17ef79e9206fa45f2532876
echo x - files/patch-spp-gcc_gcc.c
sed 's/^X//' >files/patch-spp-gcc_gcc.c << 'eaa3214d488ae4d93f50460d26bc9f25'
X--- gcc/gcc.c.orig
X+++ gcc/gcc.c
X@@ -655,7 +655,7 @@ proper position among the other output files.  */
X #ifdef TARGET_LIBC_PROVIDES_SSP
X #define LINK_SSP_SPEC "%{fstack-protector:}"
X #else
X-#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared -lssp}"
X+#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-all:-lssp_nonshared -lssp}"
X #endif
X #endif
eaa3214d488ae4d93f50460d26bc9f25
echo x - files/patch-spp-gcc_testsuite_g++.dg_fstack-protector-strong.C
sed 's/^X//' >files/patch-spp-gcc_testsuite_g++.dg_fstack-protector-strong.C << 'df9c4c3ff2667261548ed8c7d81128ed'
X--- /dev/null
X+++ gcc/testsuite/g++.dg/fstack-protector-strong.C
X@@ -0,0 +1,35 @@
X+/* Test that stack protection is done on chosen functions. */
X+
X+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
X+/* { dg-options "-O2 -fstack-protector-strong" } */
X+
X+class A
X+{
X+public:
X+  A() {}
X+  ~A() {}
X+  void method();
X+  int state;
X+};
X+
X+/* Frame address exposed to A::method via "this". */
X+int
X+foo1 ()
X+{
X+  A a;
X+  a.method ();
X+  return a.state;
X+}
X+
X+/* Possible destroying foo2's stack via &a. */
X+int
X+global_func (A& a);
X+
X+/* Frame address exposed to global_func. */
X+int foo2 ()
X+{
X+  A a;
X+  return global_func (a);
X+}
X+
X+/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */
df9c4c3ff2667261548ed8c7d81128ed
echo x - files/patch-spp-gcc_cfgexpand.c
sed 's/^X//' >files/patch-spp-gcc_cfgexpand.c << 'a085e1880c7ab319861f3730bcf7b267'
X--- gcc/cfgexpand.c.orig
X+++ gcc/cfgexpand.c
X@@ -1291,6 +1291,12 @@
X     clear_tree_used (t);
X }
X 
X+ enum {
X+   SPCT_FLAG_DEFAULT = 1,
X+   SPCT_FLAG_ALL = 2,
X+   SPCT_FLAG_STRONG = 3
X+ };
X+
X /* Examine TYPE and determine a bit mask of the following features.  */
X 
X #define SPCT_HAS_LARGE_CHAR_ARRAY	1
X@@ -1360,7 +1366,8 @@
X   if (bits & SPCT_HAS_SMALL_CHAR_ARRAY)
X     has_short_buffer = true;
X 
X-  if (flag_stack_protect == 2)
X+  if (flag_stack_protect == SPCT_FLAG_ALL
X+      || flag_stack_protect == SPCT_FLAG_STRONG)
X     {
X       if ((bits & (SPCT_HAS_SMALL_CHAR_ARRAY | SPCT_HAS_LARGE_CHAR_ARRAY))
X 	  && !(bits & SPCT_HAS_AGGREGATE))
X@@ -1514,6 +1521,27 @@
X   return size;
X }
X 
X+/* Helper routine to check if a record or union contains an array field. */
X+
X+static int
X+record_or_union_type_has_array_p (const_tree tree_type)
X+{
X+  tree fields = TYPE_FIELDS (tree_type);
X+  tree f;
X+
X+  for (f = fields; f; f = DECL_CHAIN (f))
X+    if (TREE_CODE (f) == FIELD_DECL)
X+	  {
X+	    tree field_type = TREE_TYPE (f);
X+	    if (RECORD_OR_UNION_TYPE_P (field_type)
X+	        && record_or_union_type_has_array_p (field_type))
X+		  return 1;
X+	    if (TREE_CODE (field_type) == ARRAY_TYPE)
X+	      return 1;
X+	  }
X+  return 0;
X+}
X+
X /* Expand all variables used in the function.  */
X 
X static rtx
X@@ -1525,6 +1553,7 @@
X   struct pointer_map_t *ssa_name_decls;
X   unsigned i;
X   unsigned len;
X+  bool gen_stack_protect_signal = false;
X 
X   /* Compute the phase of the stack frame for this function.  */
X   {
X@@ -1575,6 +1604,24 @@
X 	}
X     }
X   pointer_map_destroy (ssa_name_decls);
X+   
X+  if (flag_stack_protect == SPCT_FLAG_STRONG)
X+    FOR_EACH_LOCAL_DECL (cfun, i, var)
X+      if (!is_global_var (var))
X+        {
X+	      tree var_type = TREE_TYPE (var);
X+	      /* Examine local referenced variables that have their addresses taken,
X+	         contain an array, or are arrays.  */
X+	      if (TREE_CODE (var) == VAR_DECL
X+	          && (TREE_CODE (var_type) == ARRAY_TYPE
X+	          || TREE_ADDRESSABLE (var)
X+	          || (RECORD_OR_UNION_TYPE_P (var_type)
X+	          && record_or_union_type_has_array_p (var_type))))
X+	        {
X+	          gen_stack_protect_signal = true;
X+	          break;
X+	        }
X+        }
X 
X   /* At this point all variables on the local_decls with TREE_USED
X      set are not associated with any block scope.  Lay them out.  */
X@@ -1662,12 +1709,32 @@
X 	dump_stack_var_partition ();
X     }
X 
X-  /* There are several conditions under which we should create a
X-     stack guard: protect-all, alloca used, protected decls present.  */
X-  if (flag_stack_protect == 2
X-      || (flag_stack_protect
X-	  && (cfun->calls_alloca || has_protected_decls)))
X-    create_stack_guard ();
X+  /* Create stack guard, if
X+     a) "-fstack-protector-all" - always;
X+     b) "-fstack-protector-strong" - if there are arrays, memory
X+     references to local variables, alloca used, or protected decls present;
X+     c) "-fstack-protector" - if alloca used, or protected decls present  */
X+	 
X+  switch (flag_stack_protect)
X+    {
X+    case SPCT_FLAG_ALL:
X+      create_stack_guard ();
X+      break;
X+
X+    case SPCT_FLAG_STRONG:
X+      if (gen_stack_protect_signal
X+	      || cfun->calls_alloca || has_protected_decls)
X+	    create_stack_guard ();
X+      break;
X+
X+    case SPCT_FLAG_DEFAULT:
X+      if (cfun->calls_alloca || has_protected_decls)
X+	    create_stack_guard ();
X+      break;
X+
X+    default:
X+      ;
X+    }
X 
X   /* Assign rtl to each variable based on these partitions.  */
X   if (stack_vars_num > 0)
X@@ -1688,7 +1755,7 @@
X 	  expand_stack_vars (stack_protect_decl_phase_1, &data);
X 
X 	  /* Phase 2 contains other kinds of arrays.  */
X-	  if (flag_stack_protect == 2)
X+	  if (flag_stack_protect == SPCT_FLAG_ALL)
X 	    expand_stack_vars (stack_protect_decl_phase_2, &data);
X 	}
X 
a085e1880c7ab319861f3730bcf7b267
echo x - files/patch-spp-gcc_doc_cpp.texi
sed 's/^X//' >files/patch-spp-gcc_doc_cpp.texi << '8d281a43eb37e5755700ca40315994da'
X--- gcc/doc/cpp.texi.orig
X+++ gcc/doc/cpp.texi
X@@ -2349,6 +2349,10 @@ use.
X This macro is defined, with value 2, when @option{-fstack-protector-all} is
X in use.
X 
X+@item __SSP_STRONG__
X+This macro is defined, with value 3, when @option{-fstack-protector-strong} is
X+in use.
X+
X @item __SANITIZE_ADDRESS__
X This macro is defined, with value 1, when @option{-fsanitize=address} is
X in use.
8d281a43eb37e5755700ca40315994da
exit



>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201402180115.s1I1FuFc096668>