public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: Introduce -fhardened to enable security-related flags
@ 2023-08-29 19:42 Marek Polacek
  2023-08-29 20:11 ` Sam James
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: Marek Polacek @ 2023-08-29 19:42 UTC (permalink / raw)
  To: GCC Patches

Improving the security of software has been a major trend in the recent
years.  Fortunately, GCC offers a wide variety of flags that enable extra
hardening.  These flags aren't enabled by default, though.  And since
there are a lot of hardening flags, with more to come, it's been difficult
to keep on top of them; more so for the users of GCC who ought not to be
expected to keep track of all the new options.

To alleviate some of the problems I mentioned, we thought it would
be useful to provide a new umbrella option that enables a reasonable set
of hardening flags.  What's "reasonable" in this context is not easy to
pin down.  Surely, there must be no ABI impact, the option cannot cause
severe performance issues, and, I suspect, it should not cause build
errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
-Wint-conversion).  Including a controversial option in -fhardened
would likely cause that users would not use -fhardened at all.  It's
roughly akin to -Wall or -O2 -- those also enable a reasonable set of
options, and evolve over time, and are not kept in sync with other
compilers.

Currently, -fhardened enables:

  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
  -D_GLIBCXX_ASSERTIONS
  -ftrivial-auto-var-init=zero
  -fPIE  -pie  -Wl,-z,relro,-z,now
  -fstack-protector-strong
  -fstack-clash-protection
  -fcf-protection=full (x86 GNU/Linux only)

-fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
also liable to break a lot of code so I didn't include it.

Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
yet.  A fairly crucial point is that -fhardened will not override options
that were specified on the command line (before or after -fhardened).  For
example,
     
     -D_FORTIFY_SOURCE=1 -fhardened

means that _FORTIFY_SOURCE=1 will be used.  Similarly,

      -fhardened -fstack-protector

will not enable -fstack-protector-strong.

Thoughts?

---
 gcc/c-family/c-opts.cc                     | 25 ++++++++++++++++
 gcc/common.opt                             |  4 +++
 gcc/config/i386/i386-options.cc            | 11 ++++++-
 gcc/doc/invoke.texi                        | 29 +++++++++++++++++-
 gcc/gcc.cc                                 | 35 +++++++++++++++++++++-
 gcc/opts.cc                                | 15 ++++++++--
 gcc/testsuite/c-c++-common/fhardened-1.S   |  6 ++++
 gcc/testsuite/c-c++-common/fhardened-1.c   | 18 +++++++++++
 gcc/testsuite/c-c++-common/fhardened-10.c  | 10 +++++++
 gcc/testsuite/c-c++-common/fhardened-2.c   | 12 ++++++++
 gcc/testsuite/c-c++-common/fhardened-3.c   | 12 ++++++++
 gcc/testsuite/c-c++-common/fhardened-5.c   | 11 +++++++
 gcc/testsuite/c-c++-common/fhardened-6.c   | 11 +++++++
 gcc/testsuite/c-c++-common/fhardened-7.c   |  7 +++++
 gcc/testsuite/c-c++-common/fhardened-8.c   |  7 +++++
 gcc/testsuite/c-c++-common/fhardened-9.c   |  6 ++++
 gcc/testsuite/gcc.misc-tests/help.exp      |  2 ++
 gcc/testsuite/gcc.target/i386/cf_check-6.c | 12 ++++++++
 gcc/toplev.cc                              |  6 ++++
 19 files changed, 233 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.S
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-10.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-2.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-3.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-5.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-6.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-7.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-8.c
 create mode 100644 gcc/testsuite/c-c++-common/fhardened-9.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c

diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index 4961af63de8..764714ba8a5 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -1514,6 +1514,9 @@ c_finish_options (void)
       cb_file_change (parse_in, cmd_map);
       linemap_line_start (line_table, 0, 1);
 
+      bool fortify_seen_p = false;
+      bool cxx_assert_seen_p = false;
+
       /* All command line defines must have the same location.  */
       cpp_force_token_locations (parse_in, line_table->highest_line);
       for (size_t i = 0; i < deferred_count; i++)
@@ -1531,6 +1534,28 @@ c_finish_options (void)
 	      else
 		cpp_assert (parse_in, opt->arg);
 	    }
+
+	  if (UNLIKELY (flag_hardened)
+	      && (opt->code == OPT_D || opt->code == OPT_U))
+	    {
+	      if (!fortify_seen_p)
+		fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
+	      if (!cxx_assert_seen_p)
+		cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
+	    }
+	}
+
+      if (flag_hardened)
+	{
+	  if (!fortify_seen_p && optimize > 0)
+	    {
+	      if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
+		cpp_define (parse_in, "_FORTIFY_SOURCE=3");
+	      else
+		cpp_define (parse_in, "_FORTIFY_SOURCE=2");
+	    }
+	  if (!cxx_assert_seen_p)
+	    cpp_define (parse_in, "_GLIBCXX_ASSERTIONS");
 	}
 
       cpp_stop_forcing_token_locations (parse_in);
diff --git a/gcc/common.opt b/gcc/common.opt
index 0888c15b88f..d1273df006e 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1823,6 +1823,10 @@ fharden-conditional-branches
 Common Var(flag_harden_conditional_branches) Optimization
 Harden conditional branches by checking reversed conditions.
 
+fhardened
+Common Driver Var(flag_hardened)
+Enable various security-relevant flags.
+
 ; Nonzero means ignore `#ident' directives.  0 means handle them.
 ; Generate position-independent code for executables if possible
 ; On SVR4 targets, it also controls whether or not to emit a
diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
index e47f9ed5d5f..963593bcd27 100644
--- a/gcc/config/i386/i386-options.cc
+++ b/gcc/config/i386/i386-options.cc
@@ -3024,10 +3024,19 @@ ix86_option_override_internal (bool main_args_p,
         = build_target_option_node (opts, opts_set);
     }
 
+  const bool cf_okay_p = (TARGET_64BIT || TARGET_CMOV);
+  /* When -fhardened, enable -fcf-protection=full, but only when it's
+     compatible with this target, and when it wasn't already specified
+     on the command line.  */
+  if (opts->x_flag_hardened
+      && cf_okay_p
+      && opts->x_flag_cf_protection == CF_NONE)
+    opts->x_flag_cf_protection = CF_FULL;
+
   if (opts->x_flag_cf_protection != CF_NONE)
     {
       if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
-	  && !TARGET_64BIT && !TARGET_CMOV)
+	  && !cf_okay_p)
 	error ("%<-fcf-protection%> is not compatible with this target");
 
       opts->x_flag_cf_protection
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 16aa92b5e86..326c64eb4d4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -639,7 +639,7 @@ Objective-C and Objective-C++ Dialects}.
 -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},...
 -fsanitize-undefined-trap-on-error  -fbounds-check
 -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]}
--fharden-compares -fharden-conditional-branches
+-fharden-compares -fharden-conditional-branches -fhardened
 -fstack-protector  -fstack-protector-all  -fstack-protector-strong
 -fstack-protector-explicit  -fstack-check
 -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym}
@@ -17342,6 +17342,33 @@ condition, and to call @code{__builtin_trap} if the result is
 unexpected.  Use with @samp{-fharden-compares} to cover all
 conditionals.
 
+@opindex fhardened
+@item -fhardened
+Enable a set of flags for C and C++ that improve the security of the
+generated code without affecting its ABI.  The precise flags enabled
+may change between major releases of GCC, but are currently:
+
+@gccoptlist{
+-D_FORTIFY_SOURCE=3
+-D_GLIBCXX_ASSERTIONS
+-ftrivial-auto-var-init=zero
+-fPIE  -pie  -Wl,-z,relro,-z,now
+-fstack-protector-strong
+-fstack-clash-protection
+-fcf-protection=full @r{(x86 GNU/Linux only)}
+}
+
+The list of options enabled by @option{-fhardened} can be generated using
+the @option{--help=hardened} option.
+
+When the system glibc is older than 2.35, @option{-D_FORTIFY_SOURCE=2}
+is used instead.
+
+@option{-fhardened} only enables a particular option if it wasn't
+already specified anywhere on the command line.  For instance,
+@option{-fhardened} @option{-fstack-protector} will only enable
+@option{-fstack-protector}, but not @option{-fstack-protector-strong}.
+
 @opindex fstack-protector
 @item -fstack-protector
 Emit extra code to check for buffer overflows, such as stack smashing
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index a9dd0eb655c..a1205c972d8 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -302,6 +302,13 @@ static size_t dumpdir_length = 0;
    driver added to dumpdir after dumpbase or linker output name.  */
 static bool dumpdir_trailing_dash_added = false;
 
+/* True if -r, -shared, -pie, or -no-pie were specified on the command
+   line.  */
+static bool any_link_options_p;
+
+/* True if -static was specified on the command line.  */
+static bool static_p;
+
 /* Basename of dump and aux outputs, computed from dumpbase (given or
    derived from output name), to override input_basename in non-%w %b
    et al.  */
@@ -4597,10 +4604,20 @@ driver_handle_option (struct gcc_options *opts,
       save_switch ("-o", 1, &arg, validated, true);
       return true;
 
-#ifdef ENABLE_DEFAULT_PIE
     case OPT_pie:
+#ifdef ENABLE_DEFAULT_PIE
       /* -pie is turned on by default.  */
+      validated = true;
 #endif
+    case OPT_r:
+    case OPT_shared:
+    case OPT_no_pie:
+      any_link_options_p = true;
+      break;
+
+    case OPT_static:
+      static_p = true;
+      break;
 
     case OPT_static_libgcc:
     case OPT_shared_libgcc:
@@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
 #endif
     }
 
+  /* TODO: check if -static -pie works and maybe use it.  */
+  if (flag_hardened && !any_link_options_p && !static_p)
+    {
+      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
+      /* TODO: check if BIND_NOW/RELRO is supported.  */
+      if (true)
+	{
+	  /* These are passed straight down to collect2 so we have to break
+	     it up like this.  */
+	  add_infile ("-z", "*");
+	  add_infile ("now", "*");
+	  add_infile ("-z", "*");
+	  add_infile ("relro", "*");
+	}
+    }
+
   /* Handle -gtoggle as it would later in toplev.cc:process_options to
      make the debug-level-gt spec function work as expected.  */
   if (flag_gtoggle)
diff --git a/gcc/opts.cc b/gcc/opts.cc
index ac81d4e4294..f6c3b960b96 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -1093,6 +1093,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
       opts->x_flag_section_anchors = 0;
     }
 
+  if (!opts_set->x_flag_auto_var_init && opts->x_flag_hardened)
+    opts->x_flag_auto_var_init = AUTO_INIT_ZERO;
+
   if (!opts->x_flag_opts_finished)
     {
       /* We initialize opts->x_flag_pie to -1 so that targets can set a
@@ -1102,7 +1105,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
 	  /* We initialize opts->x_flag_pic to -1 so that we can tell if
 	     -fpic, -fPIC, -fno-pic or -fno-PIC is used.  */
 	  if (opts->x_flag_pic == -1)
-	    opts->x_flag_pie = DEFAULT_FLAG_PIE;
+	    opts->x_flag_pie = (opts->x_flag_hardened
+				? /*-fPIE*/ 2 : DEFAULT_FLAG_PIE);
 	  else
 	    opts->x_flag_pie = 0;
 	}
@@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
     }
 
   /* We initialize opts->x_flag_stack_protect to -1 so that targets
-     can set a default value.  */
+     can set a default value.  With --enable-default-ssp or -fhardened
+     the default is -fstack-protector-strong.  */
   if (opts->x_flag_stack_protect == -1)
-    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
+    opts->x_flag_stack_protect = (opts->x_flag_hardened
+				  ? SPCT_FLAG_STRONG
+				  : DEFAULT_FLAG_SSP);
 
   if (opts->x_optimize == 0)
     {
@@ -2586,6 +2593,8 @@ print_help (struct gcc_options *opts, unsigned int lang_mask,
       a = comma + 1;
     }
 
+  /* TODO --help=hardened someplace here.  */
+
   /* We started using PerFunction/Optimization for parameters and
      a warning.  We should exclude these from optimization options.  */
   if (include_flags & CL_OPTIMIZATION)
diff --git a/gcc/testsuite/c-c++-common/fhardened-1.S b/gcc/testsuite/c-c++-common/fhardened-1.S
new file mode 100644
index 00000000000..5bdc7f0fa3f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-1.S
@@ -0,0 +1,6 @@
+/* { dg-do preprocess } */
+/* { dg-options "-fhardened" } */
+
+#if __PIE__ != 2
+# error "-fPIE not enabled"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-1.c b/gcc/testsuite/c-c++-common/fhardened-1.c
new file mode 100644
index 00000000000..872c0750b81
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -O" } */
+
+#ifndef __SSP_STRONG__
+# error "-fstack-protector-strong not enabled"
+#endif
+
+#if __PIE__ != 2
+# error "-fPIE not enabled"
+#endif
+
+#if _FORTIFY_SOURCE < 2
+# error "_FORTIFY_SOURCE not enabled"
+#endif
+
+#ifndef _GLIBCXX_ASSERTIONS
+# error "_GLIBCXX_ASSERTIONS not enabled"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-10.c b/gcc/testsuite/c-c++-common/fhardened-10.c
new file mode 100644
index 00000000000..431bd5d3a80
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-10.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -D_FORTIFY_SOURCE=1" } */
+
+#if _FORTIFY_SOURCE != 1
+# error "_FORTIFY_SOURCE != 1"
+#endif
+
+#ifndef _GLIBCXX_ASSERTIONS
+# error "_GLIBCXX_ASSERTIONS disabled when it should not be"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-2.c b/gcc/testsuite/c-c++-common/fhardened-2.c
new file mode 100644
index 00000000000..63aeda011c5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -fstack-protector -fno-PIE" } */
+
+#ifdef __SSP_STRONG__
+# error "-fstack-protector-strong enabled when it should not be"
+#endif
+#ifndef __SSP__
+# error "-fstack-protector not enabled"
+#endif
+#ifdef __PIE__
+# error "PIE enabled when it should not be"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-3.c b/gcc/testsuite/c-c++-common/fhardened-3.c
new file mode 100644
index 00000000000..105e013d734
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -O0" } */
+/* Test that we don't get any diagnostic coming from libc headers.  */
+
+#include <stdio.h>
+
+/* The most useful C program known to man.  */
+
+int
+main ()
+{
+}
diff --git a/gcc/testsuite/c-c++-common/fhardened-5.c b/gcc/testsuite/c-c++-common/fhardened-5.c
new file mode 100644
index 00000000000..340a7bdad6d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-5.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -fdump-tree-gimple" } */
+
+int
+foo ()
+{
+  int i;
+  return i;
+}
+
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/fhardened-6.c b/gcc/testsuite/c-c++-common/fhardened-6.c
new file mode 100644
index 00000000000..478caf9895b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-6.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -ftrivial-auto-var-init=uninitialized -fdump-tree-gimple" } */
+
+int
+foo ()
+{
+  int i;
+  return i;
+}
+
+/* { dg-final { scan-tree-dump-not ".DEFERRED_INIT" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/fhardened-7.c b/gcc/testsuite/c-c++-common/fhardened-7.c
new file mode 100644
index 00000000000..60c48cde0f2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-7.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -fpie" } */
+
+/* -fpie takes precedence over -fhardened */
+#if __PIE__ != 1
+# error "__PIE__ != 1"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-8.c b/gcc/testsuite/c-c++-common/fhardened-8.c
new file mode 100644
index 00000000000..a4f01159a6f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-8.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -fPIC" } */
+
+/* -fPIC takes precedence over -fhardened */
+#ifdef __PIE__
+# error "PIE enabled when it should not be"
+#endif
diff --git a/gcc/testsuite/c-c++-common/fhardened-9.c b/gcc/testsuite/c-c++-common/fhardened-9.c
new file mode 100644
index 00000000000..f64e7eba56c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fhardened-9.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-fhardened -U_FORTIFY_SOURCE -U_GLIBCXX_ASSERTIONS" } */
+
+#if defined(_FORTIFY_SOURCE) || defined(_GLIBCXX_ASSERTIONS)
+# error "hardening enabled when it should not be"
+#endif
diff --git a/gcc/testsuite/gcc.misc-tests/help.exp b/gcc/testsuite/gcc.misc-tests/help.exp
index 52b9cb0ab90..7786a465406 100644
--- a/gcc/testsuite/gcc.misc-tests/help.exp
+++ b/gcc/testsuite/gcc.misc-tests/help.exp
@@ -151,6 +151,8 @@ foreach cls { "ada" "c" "c++" "d" "fortran" "go" \
 # Listing only excludes gives empty results.
 check_for_options c "--help=^joined,^separate" "" "" ""
 
+# TODO -fhardened tests
+
 if [ info exists prev_columns ] {
     # Reset the enviroment variable to its oriuginal value.
     set env(COLUMNS) $prev_columns
diff --git a/gcc/testsuite/gcc.target/i386/cf_check-6.c b/gcc/testsuite/gcc.target/i386/cf_check-6.c
new file mode 100644
index 00000000000..73b78dce889
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cf_check-6.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fhardened -mno-manual-endbr" } */
+/* { dg-final { scan-assembler-times {\mendbr} 1 } } */
+/* Test that -fhardened enables CET.  */
+
+extern void bar (void) __attribute__((__cf_check__));
+
+void
+foo (void)
+{
+  bar ();
+}
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 6c1a6f443c1..01b6caba203 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -1575,6 +1575,12 @@ process_options (bool no_backend)
 		  "where the stack grows from lower to higher addresses");
       flag_stack_clash_protection = 0;
     }
+  else if (flag_hardened
+	   && !flag_stack_clash_protection
+	   /* Don't enable -fstack-clash-protection when -fstack-check=
+	      is used: it would result in confusing errors.  */
+	   && flag_stack_check == NO_STACK_CHECK)
+    flag_stack_clash_protection = 1;
 
   /* We cannot support -fstack-check= and -fstack-clash-protection at
      the same time.  */

base-commit: fce74ce2535aa3b7648ba82e7e61eb77d0175546
-- 
2.41.0

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
@ 2023-08-29 20:11 ` Sam James
  2023-08-29 22:07   ` Marek Polacek
  2023-08-30  8:46 ` Martin Uecker
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Sam James @ 2023-08-29 20:11 UTC (permalink / raw)
  To: Marek Polacek; +Cc: gcc-patches


Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:

> Improving the security of software has been a major trend in the recent
> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> hardening.  These flags aren't enabled by default, though.  And since
> there are a lot of hardening flags, with more to come, it's been difficult
> to keep on top of them; more so for the users of GCC who ought not to be
> expected to keep track of all the new options.
>
> To alleviate some of the problems I mentioned, we thought it would
> be useful to provide a new umbrella option that enables a reasonable set
> of hardening flags.  What's "reasonable" in this context is not easy to
> pin down.  Surely, there must be no ABI impact, the option cannot cause
> severe performance issues, and, I suspect, it should not cause build
> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> -Wint-conversion).  Including a controversial option in -fhardened
> would likely cause that users would not use -fhardened at all.  It's
> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> options, and evolve over time, and are not kept in sync with other
> compilers.
>
> Currently, -fhardened enables:

Right now, we patch the compiler in Gentoo to default to these
(some always, some only if the user requests hardening).

It's a bit invasive (trivial, but just a bit messy) and it gets
pretty tedious to rebase it. I'd find it really helpful to be able
to instead default on -fhardened from a maintenance perspective.

>
>   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>   -D_GLIBCXX_ASSERTIONS
>   -ftrivial-auto-var-init=zero
>   -fPIE  -pie  -Wl,-z,relro,-z,now
>   -fstack-protector-strong
>   -fstack-clash-protection
>   -fcf-protection=full (x86 GNU/Linux only)
>

... and I also think it's going to be useful for people when
debugging/developing. We can tell them to simply use -fhardened and
then they'll know the compiler will give them the equivalent of -Wall
in terms of sanity checks to help find problems.

It should be useful for folks who just want to slap it in their CI as
well without keeping up with the various new developments and compiler
features.

> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> also liable to break a lot of code so I didn't include it.
>
> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> yet.  A fairly crucial point is that -fhardened will not override options
> that were specified on the command line (before or after -fhardened).  For
> example,
>      
>      -D_FORTIFY_SOURCE=1 -fhardened
>
> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
>
>       -fhardened -fstack-protector
>
> will not enable -fstack-protector-strong.
>
> Thoughts?
>
> ---
>  gcc/c-family/c-opts.cc                     | 25 ++++++++++++++++
>  gcc/common.opt                             |  4 +++
>  gcc/config/i386/i386-options.cc            | 11 ++++++-
>  gcc/doc/invoke.texi                        | 29 +++++++++++++++++-
>  gcc/gcc.cc                                 | 35 +++++++++++++++++++++-
>  gcc/opts.cc                                | 15 ++++++++--
>  gcc/testsuite/c-c++-common/fhardened-1.S   |  6 ++++
>  gcc/testsuite/c-c++-common/fhardened-1.c   | 18 +++++++++++
>  gcc/testsuite/c-c++-common/fhardened-10.c  | 10 +++++++
>  gcc/testsuite/c-c++-common/fhardened-2.c   | 12 ++++++++
>  gcc/testsuite/c-c++-common/fhardened-3.c   | 12 ++++++++
>  gcc/testsuite/c-c++-common/fhardened-5.c   | 11 +++++++
>  gcc/testsuite/c-c++-common/fhardened-6.c   | 11 +++++++
>  gcc/testsuite/c-c++-common/fhardened-7.c   |  7 +++++
>  gcc/testsuite/c-c++-common/fhardened-8.c   |  7 +++++
>  gcc/testsuite/c-c++-common/fhardened-9.c   |  6 ++++
>  gcc/testsuite/gcc.misc-tests/help.exp      |  2 ++
>  gcc/testsuite/gcc.target/i386/cf_check-6.c | 12 ++++++++
>  gcc/toplev.cc                              |  6 ++++
>  19 files changed, 233 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.S
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-10.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-2.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-3.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-5.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-6.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-7.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-8.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-9.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c
>
> diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
> index 4961af63de8..764714ba8a5 100644
> --- a/gcc/c-family/c-opts.cc
> +++ b/gcc/c-family/c-opts.cc
> @@ -1514,6 +1514,9 @@ c_finish_options (void)
>        cb_file_change (parse_in, cmd_map);
>        linemap_line_start (line_table, 0, 1);
>  
> +      bool fortify_seen_p = false;
> +      bool cxx_assert_seen_p = false;
> +
>        /* All command line defines must have the same location.  */
>        cpp_force_token_locations (parse_in, line_table->highest_line);
>        for (size_t i = 0; i < deferred_count; i++)
> @@ -1531,6 +1534,28 @@ c_finish_options (void)
>  	      else
>  		cpp_assert (parse_in, opt->arg);
>  	    }
> +
> +	  if (UNLIKELY (flag_hardened)
> +	      && (opt->code == OPT_D || opt->code == OPT_U))
> +	    {
> +	      if (!fortify_seen_p)
> +		fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> +	      if (!cxx_assert_seen_p)
> +		cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
> +	    }
> +	}
> +
> +      if (flag_hardened)
> +	{
> +	  if (!fortify_seen_p && optimize > 0)
> +	    {
> +	      if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> +		cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> +	      else
> +		cpp_define (parse_in, "_FORTIFY_SOURCE=2");
> +	    }
> +	  if (!cxx_assert_seen_p)
> +	    cpp_define (parse_in, "_GLIBCXX_ASSERTIONS");
>  	}
>  
>        cpp_stop_forcing_token_locations (parse_in);
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 0888c15b88f..d1273df006e 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1823,6 +1823,10 @@ fharden-conditional-branches
>  Common Var(flag_harden_conditional_branches) Optimization
>  Harden conditional branches by checking reversed conditions.
>  
> +fhardened
> +Common Driver Var(flag_hardened)
> +Enable various security-relevant flags.
> +
>  ; Nonzero means ignore `#ident' directives.  0 means handle them.
>  ; Generate position-independent code for executables if possible
>  ; On SVR4 targets, it also controls whether or not to emit a
> diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> index e47f9ed5d5f..963593bcd27 100644
> --- a/gcc/config/i386/i386-options.cc
> +++ b/gcc/config/i386/i386-options.cc
> @@ -3024,10 +3024,19 @@ ix86_option_override_internal (bool main_args_p,
>          = build_target_option_node (opts, opts_set);
>      }
>  
> +  const bool cf_okay_p = (TARGET_64BIT || TARGET_CMOV);
> +  /* When -fhardened, enable -fcf-protection=full, but only when it's
> +     compatible with this target, and when it wasn't already specified
> +     on the command line.  */
> +  if (opts->x_flag_hardened
> +      && cf_okay_p
> +      && opts->x_flag_cf_protection == CF_NONE)
> +    opts->x_flag_cf_protection = CF_FULL;
> +
>    if (opts->x_flag_cf_protection != CF_NONE)
>      {
>        if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
> -	  && !TARGET_64BIT && !TARGET_CMOV)
> +	  && !cf_okay_p)
>  	error ("%<-fcf-protection%> is not compatible with this target");
>  
>        opts->x_flag_cf_protection
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 16aa92b5e86..326c64eb4d4 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -639,7 +639,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},...
>  -fsanitize-undefined-trap-on-error  -fbounds-check
>  -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]}
> --fharden-compares -fharden-conditional-branches
> +-fharden-compares -fharden-conditional-branches -fhardened
>  -fstack-protector  -fstack-protector-all  -fstack-protector-strong
>  -fstack-protector-explicit  -fstack-check
>  -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym}
> @@ -17342,6 +17342,33 @@ condition, and to call @code{__builtin_trap} if the result is
>  unexpected.  Use with @samp{-fharden-compares} to cover all
>  conditionals.
>  
> +@opindex fhardened
> +@item -fhardened
> +Enable a set of flags for C and C++ that improve the security of the
> +generated code without affecting its ABI.  The precise flags enabled
> +may change between major releases of GCC, but are currently:
> +
> +@gccoptlist{
> +-D_FORTIFY_SOURCE=3
> +-D_GLIBCXX_ASSERTIONS
> +-ftrivial-auto-var-init=zero
> +-fPIE  -pie  -Wl,-z,relro,-z,now
> +-fstack-protector-strong
> +-fstack-clash-protection
> +-fcf-protection=full @r{(x86 GNU/Linux only)}
> +}
> +
> +The list of options enabled by @option{-fhardened} can be generated using
> +the @option{--help=hardened} option.
> +
> +When the system glibc is older than 2.35, @option{-D_FORTIFY_SOURCE=2}
> +is used instead.
> +
> +@option{-fhardened} only enables a particular option if it wasn't
> +already specified anywhere on the command line.  For instance,
> +@option{-fhardened} @option{-fstack-protector} will only enable
> +@option{-fstack-protector}, but not @option{-fstack-protector-strong}.
> +
>  @opindex fstack-protector
>  @item -fstack-protector
>  Emit extra code to check for buffer overflows, such as stack smashing
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index a9dd0eb655c..a1205c972d8 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -302,6 +302,13 @@ static size_t dumpdir_length = 0;
>     driver added to dumpdir after dumpbase or linker output name.  */
>  static bool dumpdir_trailing_dash_added = false;
>  
> +/* True if -r, -shared, -pie, or -no-pie were specified on the command
> +   line.  */
> +static bool any_link_options_p;
> +
> +/* True if -static was specified on the command line.  */
> +static bool static_p;
> +
>  /* Basename of dump and aux outputs, computed from dumpbase (given or
>     derived from output name), to override input_basename in non-%w %b
>     et al.  */
> @@ -4597,10 +4604,20 @@ driver_handle_option (struct gcc_options *opts,
>        save_switch ("-o", 1, &arg, validated, true);
>        return true;
>  
> -#ifdef ENABLE_DEFAULT_PIE
>      case OPT_pie:
> +#ifdef ENABLE_DEFAULT_PIE
>        /* -pie is turned on by default.  */
> +      validated = true;
>  #endif
> +    case OPT_r:
> +    case OPT_shared:
> +    case OPT_no_pie:
> +      any_link_options_p = true;
> +      break;
> +
> +    case OPT_static:
> +      static_p = true;
> +      break;
>  
>      case OPT_static_libgcc:
>      case OPT_shared_libgcc:
> @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
>  #endif
>      }
>  
> +  /* TODO: check if -static -pie works and maybe use it.  */
> +  if (flag_hardened && !any_link_options_p && !static_p)
> +    {
> +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> +      if (true)
> +	{
> +	  /* These are passed straight down to collect2 so we have to break
> +	     it up like this.  */
> +	  add_infile ("-z", "*");
> +	  add_infile ("now", "*");
> +	  add_infile ("-z", "*");
> +	  add_infile ("relro", "*");
> +	}
> +    }
> +
>    /* Handle -gtoggle as it would later in toplev.cc:process_options to
>       make the debug-level-gt spec function work as expected.  */
>    if (flag_gtoggle)
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index ac81d4e4294..f6c3b960b96 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -1093,6 +1093,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>        opts->x_flag_section_anchors = 0;
>      }
>  
> +  if (!opts_set->x_flag_auto_var_init && opts->x_flag_hardened)
> +    opts->x_flag_auto_var_init = AUTO_INIT_ZERO;
> +
>    if (!opts->x_flag_opts_finished)
>      {
>        /* We initialize opts->x_flag_pie to -1 so that targets can set a
> @@ -1102,7 +1105,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>  	  /* We initialize opts->x_flag_pic to -1 so that we can tell if
>  	     -fpic, -fPIC, -fno-pic or -fno-PIC is used.  */
>  	  if (opts->x_flag_pic == -1)
> -	    opts->x_flag_pie = DEFAULT_FLAG_PIE;
> +	    opts->x_flag_pie = (opts->x_flag_hardened
> +				? /*-fPIE*/ 2 : DEFAULT_FLAG_PIE);
>  	  else
>  	    opts->x_flag_pie = 0;
>  	}
> @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>      }
>  
>    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> -     can set a default value.  */
> +     can set a default value.  With --enable-default-ssp or -fhardened
> +     the default is -fstack-protector-strong.  */
>    if (opts->x_flag_stack_protect == -1)
> -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> +				  ? SPCT_FLAG_STRONG
> +				  : DEFAULT_FLAG_SSP);
>  
>    if (opts->x_optimize == 0)
>      {
> @@ -2586,6 +2593,8 @@ print_help (struct gcc_options *opts, unsigned int lang_mask,
>        a = comma + 1;
>      }
>  
> +  /* TODO --help=hardened someplace here.  */
> +
>    /* We started using PerFunction/Optimization for parameters and
>       a warning.  We should exclude these from optimization options.  */
>    if (include_flags & CL_OPTIMIZATION)
> diff --git a/gcc/testsuite/c-c++-common/fhardened-1.S b/gcc/testsuite/c-c++-common/fhardened-1.S
> new file mode 100644
> index 00000000000..5bdc7f0fa3f
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-1.S
> @@ -0,0 +1,6 @@
> +/* { dg-do preprocess } */
> +/* { dg-options "-fhardened" } */
> +
> +#if __PIE__ != 2
> +# error "-fPIE not enabled"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-1.c b/gcc/testsuite/c-c++-common/fhardened-1.c
> new file mode 100644
> index 00000000000..872c0750b81
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -O" } */
> +
> +#ifndef __SSP_STRONG__
> +# error "-fstack-protector-strong not enabled"
> +#endif
> +
> +#if __PIE__ != 2
> +# error "-fPIE not enabled"
> +#endif
> +
> +#if _FORTIFY_SOURCE < 2
> +# error "_FORTIFY_SOURCE not enabled"
> +#endif
> +
> +#ifndef _GLIBCXX_ASSERTIONS
> +# error "_GLIBCXX_ASSERTIONS not enabled"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-10.c b/gcc/testsuite/c-c++-common/fhardened-10.c
> new file mode 100644
> index 00000000000..431bd5d3a80
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-10.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -D_FORTIFY_SOURCE=1" } */
> +
> +#if _FORTIFY_SOURCE != 1
> +# error "_FORTIFY_SOURCE != 1"
> +#endif
> +
> +#ifndef _GLIBCXX_ASSERTIONS
> +# error "_GLIBCXX_ASSERTIONS disabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-2.c b/gcc/testsuite/c-c++-common/fhardened-2.c
> new file mode 100644
> index 00000000000..63aeda011c5
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fstack-protector -fno-PIE" } */
> +
> +#ifdef __SSP_STRONG__
> +# error "-fstack-protector-strong enabled when it should not be"
> +#endif
> +#ifndef __SSP__
> +# error "-fstack-protector not enabled"
> +#endif
> +#ifdef __PIE__
> +# error "PIE enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-3.c b/gcc/testsuite/c-c++-common/fhardened-3.c
> new file mode 100644
> index 00000000000..105e013d734
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -O0" } */
> +/* Test that we don't get any diagnostic coming from libc headers.  */
> +
> +#include <stdio.h>
> +
> +/* The most useful C program known to man.  */
> +
> +int
> +main ()
> +{
> +}
> diff --git a/gcc/testsuite/c-c++-common/fhardened-5.c b/gcc/testsuite/c-c++-common/fhardened-5.c
> new file mode 100644
> index 00000000000..340a7bdad6d
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-5.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fdump-tree-gimple" } */
> +
> +int
> +foo ()
> +{
> +  int i;
> +  return i;
> +}
> +
> +/* { dg-final { scan-tree-dump ".DEFERRED_INIT" "gimple" } } */
> diff --git a/gcc/testsuite/c-c++-common/fhardened-6.c b/gcc/testsuite/c-c++-common/fhardened-6.c
> new file mode 100644
> index 00000000000..478caf9895b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-6.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -ftrivial-auto-var-init=uninitialized -fdump-tree-gimple" } */
> +
> +int
> +foo ()
> +{
> +  int i;
> +  return i;
> +}
> +
> +/* { dg-final { scan-tree-dump-not ".DEFERRED_INIT" "gimple" } } */
> diff --git a/gcc/testsuite/c-c++-common/fhardened-7.c b/gcc/testsuite/c-c++-common/fhardened-7.c
> new file mode 100644
> index 00000000000..60c48cde0f2
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-7.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fpie" } */
> +
> +/* -fpie takes precedence over -fhardened */
> +#if __PIE__ != 1
> +# error "__PIE__ != 1"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-8.c b/gcc/testsuite/c-c++-common/fhardened-8.c
> new file mode 100644
> index 00000000000..a4f01159a6f
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-8.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fPIC" } */
> +
> +/* -fPIC takes precedence over -fhardened */
> +#ifdef __PIE__
> +# error "PIE enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-9.c b/gcc/testsuite/c-c++-common/fhardened-9.c
> new file mode 100644
> index 00000000000..f64e7eba56c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-9.c
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -U_FORTIFY_SOURCE -U_GLIBCXX_ASSERTIONS" } */
> +
> +#if defined(_FORTIFY_SOURCE) || defined(_GLIBCXX_ASSERTIONS)
> +# error "hardening enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/gcc.misc-tests/help.exp b/gcc/testsuite/gcc.misc-tests/help.exp
> index 52b9cb0ab90..7786a465406 100644
> --- a/gcc/testsuite/gcc.misc-tests/help.exp
> +++ b/gcc/testsuite/gcc.misc-tests/help.exp
> @@ -151,6 +151,8 @@ foreach cls { "ada" "c" "c++" "d" "fortran" "go" \
>  # Listing only excludes gives empty results.
>  check_for_options c "--help=^joined,^separate" "" "" ""
>  
> +# TODO -fhardened tests
> +
>  if [ info exists prev_columns ] {
>      # Reset the enviroment variable to its oriuginal value.
>      set env(COLUMNS) $prev_columns
> diff --git a/gcc/testsuite/gcc.target/i386/cf_check-6.c b/gcc/testsuite/gcc.target/i386/cf_check-6.c
> new file mode 100644
> index 00000000000..73b78dce889
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/cf_check-6.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fhardened -mno-manual-endbr" } */
> +/* { dg-final { scan-assembler-times {\mendbr} 1 } } */
> +/* Test that -fhardened enables CET.  */
> +
> +extern void bar (void) __attribute__((__cf_check__));
> +
> +void
> +foo (void)
> +{
> +  bar ();
> +}
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 6c1a6f443c1..01b6caba203 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -1575,6 +1575,12 @@ process_options (bool no_backend)
>  		  "where the stack grows from lower to higher addresses");
>        flag_stack_clash_protection = 0;
>      }
> +  else if (flag_hardened
> +	   && !flag_stack_clash_protection
> +	   /* Don't enable -fstack-clash-protection when -fstack-check=
> +	      is used: it would result in confusing errors.  */
> +	   && flag_stack_check == NO_STACK_CHECK)
> +    flag_stack_clash_protection = 1;
>  
>    /* We cannot support -fstack-check= and -fstack-clash-protection at
>       the same time.  */
>
> base-commit: fce74ce2535aa3b7648ba82e7e61eb77d0175546


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 20:11 ` Sam James
@ 2023-08-29 22:07   ` Marek Polacek
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-08-29 22:07 UTC (permalink / raw)
  To: Sam James; +Cc: gcc-patches

On Tue, Aug 29, 2023 at 09:11:35PM +0100, Sam James via Gcc-patches wrote:
> 
> Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> 
> > Improving the security of software has been a major trend in the recent
> > years.  Fortunately, GCC offers a wide variety of flags that enable extra
> > hardening.  These flags aren't enabled by default, though.  And since
> > there are a lot of hardening flags, with more to come, it's been difficult
> > to keep on top of them; more so for the users of GCC who ought not to be
> > expected to keep track of all the new options.
> >
> > To alleviate some of the problems I mentioned, we thought it would
> > be useful to provide a new umbrella option that enables a reasonable set
> > of hardening flags.  What's "reasonable" in this context is not easy to
> > pin down.  Surely, there must be no ABI impact, the option cannot cause
> > severe performance issues, and, I suspect, it should not cause build
> > errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> > -Wint-conversion).  Including a controversial option in -fhardened
> > would likely cause that users would not use -fhardened at all.  It's
> > roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> > options, and evolve over time, and are not kept in sync with other
> > compilers.
> >
> > Currently, -fhardened enables:
> 
> Right now, we patch the compiler in Gentoo to default to these
> (some always, some only if the user requests hardening).
> 
> It's a bit invasive (trivial, but just a bit messy) and it gets
> pretty tedious to rebase it.

Yeah, I bet.

> I'd find it really helpful to be able
> to instead default on -fhardened from a maintenance perspective.

That's good feedback.
 
> >
> >   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> >   -D_GLIBCXX_ASSERTIONS
> >   -ftrivial-auto-var-init=zero
> >   -fPIE  -pie  -Wl,-z,relro,-z,now
> >   -fstack-protector-strong
> >   -fstack-clash-protection
> >   -fcf-protection=full (x86 GNU/Linux only)
> >
> 
> ... and I also think it's going to be useful for people when
> debugging/developing. We can tell them to simply use -fhardened and
> then they'll know the compiler will give them the equivalent of -Wall
> in terms of sanity checks to help find problems.
> 
> It should be useful for folks who just want to slap it in their CI as
> well without keeping up with the various new developments and compiler
> features.

Yup, pretty much the intended usage.

Thanks!

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
  2023-08-29 20:11 ` Sam James
@ 2023-08-30  8:46 ` Martin Uecker
  2023-09-15 15:11   ` Marek Polacek
  2023-08-30  9:06 ` Xi Ruoyao
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Martin Uecker @ 2023-08-30  8:46 UTC (permalink / raw)
  To: polacek; +Cc: gcc-patches

> Improving the security of software has been a major trend in the recent
> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> hardening.  These flags aren't enabled by default, though.  And since
> there are a lot of hardening flags, with more to come, it's been difficult
> to keep on top of them; more so for the users of GCC who ought not to be
> expected to keep track of all the new options.
> 
> To alleviate some of the problems I mentioned, we thought it would
> be useful to provide a new umbrella option that enables a reasonable set
> of hardening flags.  What's "reasonable" in this context is not easy to
> pin down.  Surely, there must be no ABI impact, the option cannot cause
> severe performance issues, and, I suspect, it should not cause build
> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> -Wint-conversion).  Including a controversial option in -fhardened
> would likely cause that users would not use -fhardened at all.  It's
> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> options, and evolve over time, and are not kept in sync with other
> compilers.
> 
> Currently, -fhardened enables:
> 
>   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>   -D_GLIBCXX_ASSERTIONS
>   -ftrivial-auto-var-init=zero
>   -fPIE  -pie  -Wl,-z,relro,-z,now
>   -fstack-protector-strong
>   -fstack-clash-protection
>   -fcf-protection=full (x86 GNU/Linux only)
> 
> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> also liable to break a lot of code so I didn't include it.
> 
> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> yet.  A fairly crucial point is that -fhardened will not override options
> that were specified on the command line (before or after -fhardened).  For
> example,
>      
>      -D_FORTIFY_SOURCE=1 -fhardened
> 
> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> 
>       -fhardened -fstack-protector
> 
> will not enable -fstack-protector-strong.
> 
> Thoughts?

I think this is a great idea!  Considering that it is difficult to
decide what shoud be activated and what not and the baseline should
not cause compile errors,  I wonder whether there should be higher
levels  similar to -O1,2,3 ? 

Although it would be nice to have a one-letter or very short
option similar to -O2 or -Wall, but maybe this is not possible 
because all short ones are already taken. Of course, 
"-fhardening" would  already a huge  improvement to the 
current situation.

Martin



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
  2023-08-29 20:11 ` Sam James
  2023-08-30  8:46 ` Martin Uecker
@ 2023-08-30  9:06 ` Xi Ruoyao
  2023-09-15 15:12   ` Marek Polacek
  2023-08-30 10:50 ` Jakub Jelinek
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Xi Ruoyao @ 2023-08-30  9:06 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On Tue, 2023-08-29 at 15:42 -0400, Marek Polacek via Gcc-patches wrote:
> +         if (UNLIKELY (flag_hardened)
> +             && (opt->code == OPT_D || opt->code == OPT_U))
> +           {
> +             if (!fortify_seen_p)
> +               fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> +             if (!cxx_assert_seen_p)
> +               cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");

It looks like there is some minor logic issue here: the first strncmp
will mistakenly match "-D_FORTIFY_SOURCE_FAKE", and the second strcmp
will not match "-D_GLIBCXX_ASSERTIONS=1".

> +           }

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
                   ` (2 preceding siblings ...)
  2023-08-30  9:06 ` Xi Ruoyao
@ 2023-08-30 10:50 ` Jakub Jelinek
  2023-08-30 13:08   ` Richard Biener
  2023-09-15 15:17   ` Marek Polacek
  2023-09-01 22:09 ` Qing Zhao
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Jakub Jelinek @ 2023-08-30 10:50 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote:
> +	  if (UNLIKELY (flag_hardened)
> +	      && (opt->code == OPT_D || opt->code == OPT_U))
> +	    {
> +	      if (!fortify_seen_p)
> +		fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);

Perhaps this should check that the char after it is either '\0' or '=', we
shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro.

> +	      if (!cxx_assert_seen_p)
> +		cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");

Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42

> +	    }
> +	}
> +
> +      if (flag_hardened)
> +	{
> +	  if (!fortify_seen_p && optimize > 0)
> +	    {
> +	      if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> +		cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> +	      else
> +		cpp_define (parse_in, "_FORTIFY_SOURCE=2");

I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for
-fhardened only for targets which actually have such a support in the C
library.  There is some poor man's _FORTIFY_SOURCE support in libssp,
but e.g. one has to link with -lssp in that case and compile with
-isystem `gcc -print-include-filename=include`/ssp .
For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source
mentions NetBSD support since 2006, newlib since 2017, some Darwin libc,
bionic (but seems they have only some clang support and dropped GCC
support) and some third party reimplementation of libssp.
Or do we just enable it and hope that either it works well or isn't
supported at all quietly?  E.g. it would certainly break the ssp case
where -isystem finds ssp headers but -lssp isn't linked in.

> @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
>  #endif
>      }
>  
> +  /* TODO: check if -static -pie works and maybe use it.  */
> +  if (flag_hardened && !any_link_options_p && !static_p)
> +    {
> +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> +      if (true)
> +	{
> +	  /* These are passed straight down to collect2 so we have to break
> +	     it up like this.  */
> +	  add_infile ("-z", "*");
> +	  add_infile ("now", "*");
> +	  add_infile ("-z", "*");
> +	  add_infile ("relro", "*");

As the TODO comment says, to do that we need to check at configure time that
linker supports -z now and -z relro options.

> @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>      }
>  
>    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> -     can set a default value.  */
> +     can set a default value.  With --enable-default-ssp or -fhardened
> +     the default is -fstack-protector-strong.  */
>    if (opts->x_flag_stack_protect == -1)
> -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> +				  ? SPCT_FLAG_STRONG
> +				  : DEFAULT_FLAG_SSP);

This needs to be careful, -fstack-protector isn't supported on all targets
(e.g. ia64) and we don't want toplev.cc warning:
  /* Targets must be able to place spill slots at lower addresses.  If the
     target already uses a soft frame pointer, the transition is trivial.  */
  if (!FRAME_GROWS_DOWNWARD && flag_stack_protect)
    {
      warning_at (UNKNOWN_LOCATION, 0,
                  "%<-fstack-protector%> not supported for this target");
      flag_stack_protect = 0;
    }
to be emitted whenever using -fhardened, it should not be enabled there
silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it
work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was
enabled and otherwise keep it !FRAME_GROWS_DOWNWARD).

	Jakub


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-30 10:50 ` Jakub Jelinek
@ 2023-08-30 13:08   ` Richard Biener
  2023-09-15 15:21     ` Marek Polacek
  2023-09-15 15:17   ` Marek Polacek
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Biener @ 2023-08-30 13:08 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Marek Polacek, GCC Patches

On Wed, Aug 30, 2023 at 12:51 PM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote:
> > +       if (UNLIKELY (flag_hardened)
> > +           && (opt->code == OPT_D || opt->code == OPT_U))
> > +         {
> > +           if (!fortify_seen_p)
> > +             fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
>
> Perhaps this should check that the char after it is either '\0' or '=', we
> shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro.
>
> > +           if (!cxx_assert_seen_p)
> > +             cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
>
> Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42
>
> > +         }
> > +     }
> > +
> > +      if (flag_hardened)
> > +     {
> > +       if (!fortify_seen_p && optimize > 0)
> > +         {
> > +           if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> > +             cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> > +           else
> > +             cpp_define (parse_in, "_FORTIFY_SOURCE=2");
>
> I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for
> -fhardened only for targets which actually have such a support in the C
> library.  There is some poor man's _FORTIFY_SOURCE support in libssp,
> but e.g. one has to link with -lssp in that case and compile with
> -isystem `gcc -print-include-filename=include`/ssp .
> For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source
> mentions NetBSD support since 2006, newlib since 2017, some Darwin libc,
> bionic (but seems they have only some clang support and dropped GCC
> support) and some third party reimplementation of libssp.
> Or do we just enable it and hope that either it works well or isn't
> supported at all quietly?  E.g. it would certainly break the ssp case
> where -isystem finds ssp headers but -lssp isn't linked in.
>
> > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
> >  #endif
> >      }
> >
> > +  /* TODO: check if -static -pie works and maybe use it.  */
> > +  if (flag_hardened && !any_link_options_p && !static_p)
> > +    {
> > +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> > +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> > +      if (true)
> > +     {
> > +       /* These are passed straight down to collect2 so we have to break
> > +          it up like this.  */
> > +       add_infile ("-z", "*");
> > +       add_infile ("now", "*");
> > +       add_infile ("-z", "*");
> > +       add_infile ("relro", "*");
>
> As the TODO comment says, to do that we need to check at configure time that
> linker supports -z now and -z relro options.
>
> > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
> >      }
> >
> >    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> > -     can set a default value.  */
> > +     can set a default value.  With --enable-default-ssp or -fhardened
> > +     the default is -fstack-protector-strong.  */
> >    if (opts->x_flag_stack_protect == -1)
> > -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> > +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> > +                               ? SPCT_FLAG_STRONG
> > +                               : DEFAULT_FLAG_SSP);
>
> This needs to be careful, -fstack-protector isn't supported on all targets
> (e.g. ia64) and we don't want toplev.cc warning:
>   /* Targets must be able to place spill slots at lower addresses.  If the
>      target already uses a soft frame pointer, the transition is trivial.  */
>   if (!FRAME_GROWS_DOWNWARD && flag_stack_protect)
>     {
>       warning_at (UNKNOWN_LOCATION, 0,
>                   "%<-fstack-protector%> not supported for this target");
>       flag_stack_protect = 0;
>     }
> to be emitted whenever using -fhardened, it should not be enabled there
> silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it
> work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was
> enabled and otherwise keep it !FRAME_GROWS_DOWNWARD).

I'll note that with selectively enabling parts of -fhardening it can
also give a false
sensation of safety when under the hood we ignore half of the option due to
one or another reason ...

How does -fhardening reflect into -[gf]record-gcc-switches?  Is it at
least possible
to verify the actually enabled bits?

Richard.

>         Jakub
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
                   ` (3 preceding siblings ...)
  2023-08-30 10:50 ` Jakub Jelinek
@ 2023-09-01 22:09 ` Qing Zhao
  2023-09-04 22:40   ` Richard Sandiford
  2023-09-15 15:40   ` Marek Polacek
  2023-09-14 14:48 ` Hongtao Liu
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Qing Zhao @ 2023-09-01 22:09 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches



> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Improving the security of software has been a major trend in the recent
> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> hardening.  These flags aren't enabled by default, though.  And since
> there are a lot of hardening flags, with more to come, it's been difficult
> to keep on top of them; more so for the users of GCC who ought not to be
> expected to keep track of all the new options.
> 
> To alleviate some of the problems I mentioned, we thought it would
> be useful to provide a new umbrella option that enables a reasonable set
> of hardening flags.  What's "reasonable" in this context is not easy to
> pin down.  Surely, there must be no ABI impact, the option cannot cause
> severe performance issues, and, I suspect, it should not cause build
> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> -Wint-conversion).  Including a controversial option in -fhardened
> would likely cause that users would not use -fhardened at all.  It's
> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> options, and evolve over time, and are not kept in sync with other
> compilers.
> 
> Currently, -fhardened enables:
> 
>  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>  -D_GLIBCXX_ASSERTIONS
>  -ftrivial-auto-var-init=zero
>  -fPIE  -pie  -Wl,-z,relro,-z,now
>  -fstack-protector-strong
>  -fstack-clash-protection
>  -fcf-protection=full (x86 GNU/Linux only)
> 
> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> also liable to break a lot of code so I didn't include it.
> 
> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> yet.  A fairly crucial point is that -fhardened will not override options
> that were specified on the command line (before or after -fhardened).  For
> example,
> 
>     -D_FORTIFY_SOURCE=1 -fhardened
> 
> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> 
>      -fhardened -fstack-protector
> 
> will not enable -fstack-protector-strong.
> 
> Thoughts?

In general, I think that it is a very good idea to provide umbrella options
 for software security purpose.  Thanks a lot for this work!

1. I do agree with Martin, multiple-level control for this purpose might be needed,
similar as multiple levels for warnings, and multiple levels for optimizations.

Similar as optimization options, can we organize all the security options together 
In our manual, then the user will have a good central place to get more and complete
Information of the security features our compiler provides? 

2. What’s the major criteria to decide which security feature should go into this list?
Later, when we have new security features, how to decide whether to add them to
This list or not?
I am wondering why -fzero-call-used-regs is not included in the list and also 
Why chose -ftrivial-auto-var-init=zero instead of -ftrivial-auto-var-init=pattern? 

3. Looks like currently, -fhardened excludes all compilation-time Warning options for security purpose,
(For example, -Warray-bounds, --Wstringop-overflow…)
And also excludes all sanitizer options for security purpose (-fsanitizer=undifined)

So, shall we also provide an umbrella option for compilation-time warning options for security purpose
And a umbrella option for sanitizer options (is the -fsanitizer=undefined this one)?

Just some thoughts. -:).

Qing








^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-01 22:09 ` Qing Zhao
@ 2023-09-04 22:40   ` Richard Sandiford
  2023-09-15 15:23     ` Marek Polacek
  2023-09-15 15:40   ` Marek Polacek
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Sandiford @ 2023-09-04 22:40 UTC (permalink / raw)
  To: Qing Zhao via Gcc-patches; +Cc: Marek Polacek, Qing Zhao

Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> 
>> Improving the security of software has been a major trend in the recent
>> years.  Fortunately, GCC offers a wide variety of flags that enable extra
>> hardening.  These flags aren't enabled by default, though.  And since
>> there are a lot of hardening flags, with more to come, it's been difficult
>> to keep on top of them; more so for the users of GCC who ought not to be
>> expected to keep track of all the new options.
>> 
>> To alleviate some of the problems I mentioned, we thought it would
>> be useful to provide a new umbrella option that enables a reasonable set
>> of hardening flags.  What's "reasonable" in this context is not easy to
>> pin down.  Surely, there must be no ABI impact, the option cannot cause
>> severe performance issues, and, I suspect, it should not cause build
>> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
>> -Wint-conversion).  Including a controversial option in -fhardened
>> would likely cause that users would not use -fhardened at all.  It's
>> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
>> options, and evolve over time, and are not kept in sync with other
>> compilers.
>> 
>> Currently, -fhardened enables:
>> 
>>  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>>  -D_GLIBCXX_ASSERTIONS
>>  -ftrivial-auto-var-init=zero
>>  -fPIE  -pie  -Wl,-z,relro,-z,now
>>  -fstack-protector-strong
>>  -fstack-clash-protection
>>  -fcf-protection=full (x86 GNU/Linux only)
>> 
>> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
>> also liable to break a lot of code so I didn't include it.
>> 
>> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
>> yet.  A fairly crucial point is that -fhardened will not override options
>> that were specified on the command line (before or after -fhardened).  For
>> example,
>> 
>>     -D_FORTIFY_SOURCE=1 -fhardened
>> 
>> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
>> 
>>      -fhardened -fstack-protector
>> 
>> will not enable -fstack-protector-strong.
>> 
>> Thoughts?
>
> In general, I think that it is a very good idea to provide umbrella options
>  for software security purpose.  Thanks a lot for this work!
>
> 1. I do agree with Martin, multiple-level control for this purpose might be needed,
> similar as multiple levels for warnings, and multiple levels for optimizations.
>
> Similar as optimization options, can we organize all the security options together 
> In our manual, then the user will have a good central place to get more and complete
> Information of the security features our compiler provides? 
>
> 2. What’s the major criteria to decide which security feature should go into this list?
> Later, when we have new security features, how to decide whether to add them to
> This list or not?
> I am wondering why -fzero-call-used-regs is not included in the list and also

FWIW, I wondered the same thing.  Not a strong conviction that it should
be included -- maybe the code bloat is too much on some targets.  But it
might be acceptable for the -fhardened equivalent of -O3, at least if
restricted to GPRs.
 
> Why chose -ftrivial-auto-var-init=zero instead of -ftrivial-auto-var-init=pattern? 

Yeah, IIRC -ftrivial-auto-var-init=zero was controversial with some
Clang maintainers because it effectively creates a language dialect.
-ftrivial-auto-var-init=pattern wasn't controversial in the same way.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
                   ` (4 preceding siblings ...)
  2023-09-01 22:09 ` Qing Zhao
@ 2023-09-14 14:48 ` Hongtao Liu
  2023-09-17  3:16 ` Hans-Peter Nilsson
  2023-09-20 11:33 ` jvoisin
  7 siblings, 0 replies; 25+ messages in thread
From: Hongtao Liu @ 2023-09-14 14:48 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On Wed, Aug 30, 2023 at 3:42 AM Marek Polacek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Improving the security of software has been a major trend in the recent
> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> hardening.  These flags aren't enabled by default, though.  And since
> there are a lot of hardening flags, with more to come, it's been difficult
> to keep on top of them; more so for the users of GCC who ought not to be
> expected to keep track of all the new options.
>
> To alleviate some of the problems I mentioned, we thought it would
> be useful to provide a new umbrella option that enables a reasonable set
> of hardening flags.  What's "reasonable" in this context is not easy to
> pin down.  Surely, there must be no ABI impact, the option cannot cause
> severe performance issues, and, I suspect, it should not cause build
> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> -Wint-conversion).  Including a controversial option in -fhardened
> would likely cause that users would not use -fhardened at all.  It's
> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> options, and evolve over time, and are not kept in sync with other
> compilers.
>
> Currently, -fhardened enables:
>
>   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>   -D_GLIBCXX_ASSERTIONS
>   -ftrivial-auto-var-init=zero
>   -fPIE  -pie  -Wl,-z,relro,-z,now
>   -fstack-protector-strong
>   -fstack-clash-protection
>   -fcf-protection=full (x86 GNU/Linux only)
>
> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> also liable to break a lot of code so I didn't include it.
>
> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> yet.  A fairly crucial point is that -fhardened will not override options
> that were specified on the command line (before or after -fhardened).  For
> example,
>
>      -D_FORTIFY_SOURCE=1 -fhardened
>
> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
>
>       -fhardened -fstack-protector
>
> will not enable -fstack-protector-strong.
>
> Thoughts?
>
> ---
>  gcc/c-family/c-opts.cc                     | 25 ++++++++++++++++
>  gcc/common.opt                             |  4 +++
>  gcc/config/i386/i386-options.cc            | 11 ++++++-
>  gcc/doc/invoke.texi                        | 29 +++++++++++++++++-
>  gcc/gcc.cc                                 | 35 +++++++++++++++++++++-
>  gcc/opts.cc                                | 15 ++++++++--
>  gcc/testsuite/c-c++-common/fhardened-1.S   |  6 ++++
>  gcc/testsuite/c-c++-common/fhardened-1.c   | 18 +++++++++++
>  gcc/testsuite/c-c++-common/fhardened-10.c  | 10 +++++++
>  gcc/testsuite/c-c++-common/fhardened-2.c   | 12 ++++++++
>  gcc/testsuite/c-c++-common/fhardened-3.c   | 12 ++++++++
>  gcc/testsuite/c-c++-common/fhardened-5.c   | 11 +++++++
>  gcc/testsuite/c-c++-common/fhardened-6.c   | 11 +++++++
>  gcc/testsuite/c-c++-common/fhardened-7.c   |  7 +++++
>  gcc/testsuite/c-c++-common/fhardened-8.c   |  7 +++++
>  gcc/testsuite/c-c++-common/fhardened-9.c   |  6 ++++
>  gcc/testsuite/gcc.misc-tests/help.exp      |  2 ++
>  gcc/testsuite/gcc.target/i386/cf_check-6.c | 12 ++++++++
>  gcc/toplev.cc                              |  6 ++++
>  19 files changed, 233 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.S
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-10.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-2.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-3.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-5.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-6.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-7.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-8.c
>  create mode 100644 gcc/testsuite/c-c++-common/fhardened-9.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c
>
> diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
> index 4961af63de8..764714ba8a5 100644
> --- a/gcc/c-family/c-opts.cc
> +++ b/gcc/c-family/c-opts.cc
> @@ -1514,6 +1514,9 @@ c_finish_options (void)
>        cb_file_change (parse_in, cmd_map);
>        linemap_line_start (line_table, 0, 1);
>
> +      bool fortify_seen_p = false;
> +      bool cxx_assert_seen_p = false;
> +
>        /* All command line defines must have the same location.  */
>        cpp_force_token_locations (parse_in, line_table->highest_line);
>        for (size_t i = 0; i < deferred_count; i++)
> @@ -1531,6 +1534,28 @@ c_finish_options (void)
>               else
>                 cpp_assert (parse_in, opt->arg);
>             }
> +
> +         if (UNLIKELY (flag_hardened)
> +             && (opt->code == OPT_D || opt->code == OPT_U))
> +           {
> +             if (!fortify_seen_p)
> +               fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> +             if (!cxx_assert_seen_p)
> +               cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
> +           }
> +       }
> +
> +      if (flag_hardened)
> +       {
> +         if (!fortify_seen_p && optimize > 0)
> +           {
> +             if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> +               cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> +             else
> +               cpp_define (parse_in, "_FORTIFY_SOURCE=2");
> +           }
> +         if (!cxx_assert_seen_p)
> +           cpp_define (parse_in, "_GLIBCXX_ASSERTIONS");
>         }
>
>        cpp_stop_forcing_token_locations (parse_in);
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 0888c15b88f..d1273df006e 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1823,6 +1823,10 @@ fharden-conditional-branches
>  Common Var(flag_harden_conditional_branches) Optimization
>  Harden conditional branches by checking reversed conditions.
>
> +fhardened
> +Common Driver Var(flag_hardened)
> +Enable various security-relevant flags.
> +
>  ; Nonzero means ignore `#ident' directives.  0 means handle them.
>  ; Generate position-independent code for executables if possible
>  ; On SVR4 targets, it also controls whether or not to emit a
> diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> index e47f9ed5d5f..963593bcd27 100644
> --- a/gcc/config/i386/i386-options.cc
> +++ b/gcc/config/i386/i386-options.cc
> @@ -3024,10 +3024,19 @@ ix86_option_override_internal (bool main_args_p,
>          = build_target_option_node (opts, opts_set);
>      }
>
> +  const bool cf_okay_p = (TARGET_64BIT || TARGET_CMOV);
> +  /* When -fhardened, enable -fcf-protection=full, but only when it's
> +     compatible with this target, and when it wasn't already specified
> +     on the command line.  */
> +  if (opts->x_flag_hardened
> +      && cf_okay_p
> +      && opts->x_flag_cf_protection == CF_NONE)
> +    opts->x_flag_cf_protection = CF_FULL;
> +
>    if (opts->x_flag_cf_protection != CF_NONE)
>      {
>        if ((opts->x_flag_cf_protection & CF_BRANCH) == CF_BRANCH
> -         && !TARGET_64BIT && !TARGET_CMOV)
> +         && !cf_okay_p)
>         error ("%<-fcf-protection%> is not compatible with this target");
>
LGTM for x86 part.
>        opts->x_flag_cf_protection
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 16aa92b5e86..326c64eb4d4 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -639,7 +639,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},...
>  -fsanitize-undefined-trap-on-error  -fbounds-check
>  -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]}
> --fharden-compares -fharden-conditional-branches
> +-fharden-compares -fharden-conditional-branches -fhardened
>  -fstack-protector  -fstack-protector-all  -fstack-protector-strong
>  -fstack-protector-explicit  -fstack-check
>  -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym}
> @@ -17342,6 +17342,33 @@ condition, and to call @code{__builtin_trap} if the result is
>  unexpected.  Use with @samp{-fharden-compares} to cover all
>  conditionals.
>
> +@opindex fhardened
> +@item -fhardened
> +Enable a set of flags for C and C++ that improve the security of the
> +generated code without affecting its ABI.  The precise flags enabled
> +may change between major releases of GCC, but are currently:
> +
> +@gccoptlist{
> +-D_FORTIFY_SOURCE=3
> +-D_GLIBCXX_ASSERTIONS
> +-ftrivial-auto-var-init=zero
> +-fPIE  -pie  -Wl,-z,relro,-z,now
> +-fstack-protector-strong
> +-fstack-clash-protection
> +-fcf-protection=full @r{(x86 GNU/Linux only)}
> +}
> +
> +The list of options enabled by @option{-fhardened} can be generated using
> +the @option{--help=hardened} option.
> +
> +When the system glibc is older than 2.35, @option{-D_FORTIFY_SOURCE=2}
> +is used instead.
> +
> +@option{-fhardened} only enables a particular option if it wasn't
> +already specified anywhere on the command line.  For instance,
> +@option{-fhardened} @option{-fstack-protector} will only enable
> +@option{-fstack-protector}, but not @option{-fstack-protector-strong}.
> +
>  @opindex fstack-protector
>  @item -fstack-protector
>  Emit extra code to check for buffer overflows, such as stack smashing
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index a9dd0eb655c..a1205c972d8 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -302,6 +302,13 @@ static size_t dumpdir_length = 0;
>     driver added to dumpdir after dumpbase or linker output name.  */
>  static bool dumpdir_trailing_dash_added = false;
>
> +/* True if -r, -shared, -pie, or -no-pie were specified on the command
> +   line.  */
> +static bool any_link_options_p;
> +
> +/* True if -static was specified on the command line.  */
> +static bool static_p;
> +
>  /* Basename of dump and aux outputs, computed from dumpbase (given or
>     derived from output name), to override input_basename in non-%w %b
>     et al.  */
> @@ -4597,10 +4604,20 @@ driver_handle_option (struct gcc_options *opts,
>        save_switch ("-o", 1, &arg, validated, true);
>        return true;
>
> -#ifdef ENABLE_DEFAULT_PIE
>      case OPT_pie:
> +#ifdef ENABLE_DEFAULT_PIE
>        /* -pie is turned on by default.  */
> +      validated = true;
>  #endif
> +    case OPT_r:
> +    case OPT_shared:
> +    case OPT_no_pie:
> +      any_link_options_p = true;
> +      break;
> +
> +    case OPT_static:
> +      static_p = true;
> +      break;
>
>      case OPT_static_libgcc:
>      case OPT_shared_libgcc:
> @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
>  #endif
>      }
>
> +  /* TODO: check if -static -pie works and maybe use it.  */
> +  if (flag_hardened && !any_link_options_p && !static_p)
> +    {
> +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> +      if (true)
> +       {
> +         /* These are passed straight down to collect2 so we have to break
> +            it up like this.  */
> +         add_infile ("-z", "*");
> +         add_infile ("now", "*");
> +         add_infile ("-z", "*");
> +         add_infile ("relro", "*");
> +       }
> +    }
> +
>    /* Handle -gtoggle as it would later in toplev.cc:process_options to
>       make the debug-level-gt spec function work as expected.  */
>    if (flag_gtoggle)
> diff --git a/gcc/opts.cc b/gcc/opts.cc
> index ac81d4e4294..f6c3b960b96 100644
> --- a/gcc/opts.cc
> +++ b/gcc/opts.cc
> @@ -1093,6 +1093,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>        opts->x_flag_section_anchors = 0;
>      }
>
> +  if (!opts_set->x_flag_auto_var_init && opts->x_flag_hardened)
> +    opts->x_flag_auto_var_init = AUTO_INIT_ZERO;
> +
>    if (!opts->x_flag_opts_finished)
>      {
>        /* We initialize opts->x_flag_pie to -1 so that targets can set a
> @@ -1102,7 +1105,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>           /* We initialize opts->x_flag_pic to -1 so that we can tell if
>              -fpic, -fPIC, -fno-pic or -fno-PIC is used.  */
>           if (opts->x_flag_pic == -1)
> -           opts->x_flag_pie = DEFAULT_FLAG_PIE;
> +           opts->x_flag_pie = (opts->x_flag_hardened
> +                               ? /*-fPIE*/ 2 : DEFAULT_FLAG_PIE);
>           else
>             opts->x_flag_pie = 0;
>         }
> @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
>      }
>
>    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> -     can set a default value.  */
> +     can set a default value.  With --enable-default-ssp or -fhardened
> +     the default is -fstack-protector-strong.  */
>    if (opts->x_flag_stack_protect == -1)
> -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> +                                 ? SPCT_FLAG_STRONG
> +                                 : DEFAULT_FLAG_SSP);
>
>    if (opts->x_optimize == 0)
>      {
> @@ -2586,6 +2593,8 @@ print_help (struct gcc_options *opts, unsigned int lang_mask,
>        a = comma + 1;
>      }
>
> +  /* TODO --help=hardened someplace here.  */
> +
>    /* We started using PerFunction/Optimization for parameters and
>       a warning.  We should exclude these from optimization options.  */
>    if (include_flags & CL_OPTIMIZATION)
> diff --git a/gcc/testsuite/c-c++-common/fhardened-1.S b/gcc/testsuite/c-c++-common/fhardened-1.S
> new file mode 100644
> index 00000000000..5bdc7f0fa3f
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-1.S
> @@ -0,0 +1,6 @@
> +/* { dg-do preprocess } */
> +/* { dg-options "-fhardened" } */
> +
> +#if __PIE__ != 2
> +# error "-fPIE not enabled"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-1.c b/gcc/testsuite/c-c++-common/fhardened-1.c
> new file mode 100644
> index 00000000000..872c0750b81
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -O" } */
> +
> +#ifndef __SSP_STRONG__
> +# error "-fstack-protector-strong not enabled"
> +#endif
> +
> +#if __PIE__ != 2
> +# error "-fPIE not enabled"
> +#endif
> +
> +#if _FORTIFY_SOURCE < 2
> +# error "_FORTIFY_SOURCE not enabled"
> +#endif
> +
> +#ifndef _GLIBCXX_ASSERTIONS
> +# error "_GLIBCXX_ASSERTIONS not enabled"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-10.c b/gcc/testsuite/c-c++-common/fhardened-10.c
> new file mode 100644
> index 00000000000..431bd5d3a80
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-10.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -D_FORTIFY_SOURCE=1" } */
> +
> +#if _FORTIFY_SOURCE != 1
> +# error "_FORTIFY_SOURCE != 1"
> +#endif
> +
> +#ifndef _GLIBCXX_ASSERTIONS
> +# error "_GLIBCXX_ASSERTIONS disabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-2.c b/gcc/testsuite/c-c++-common/fhardened-2.c
> new file mode 100644
> index 00000000000..63aeda011c5
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fstack-protector -fno-PIE" } */
> +
> +#ifdef __SSP_STRONG__
> +# error "-fstack-protector-strong enabled when it should not be"
> +#endif
> +#ifndef __SSP__
> +# error "-fstack-protector not enabled"
> +#endif
> +#ifdef __PIE__
> +# error "PIE enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-3.c b/gcc/testsuite/c-c++-common/fhardened-3.c
> new file mode 100644
> index 00000000000..105e013d734
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -O0" } */
> +/* Test that we don't get any diagnostic coming from libc headers.  */
> +
> +#include <stdio.h>
> +
> +/* The most useful C program known to man.  */
> +
> +int
> +main ()
> +{
> +}
> diff --git a/gcc/testsuite/c-c++-common/fhardened-5.c b/gcc/testsuite/c-c++-common/fhardened-5.c
> new file mode 100644
> index 00000000000..340a7bdad6d
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-5.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fdump-tree-gimple" } */
> +
> +int
> +foo ()
> +{
> +  int i;
> +  return i;
> +}
> +
> +/* { dg-final { scan-tree-dump ".DEFERRED_INIT" "gimple" } } */
> diff --git a/gcc/testsuite/c-c++-common/fhardened-6.c b/gcc/testsuite/c-c++-common/fhardened-6.c
> new file mode 100644
> index 00000000000..478caf9895b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-6.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -ftrivial-auto-var-init=uninitialized -fdump-tree-gimple" } */
> +
> +int
> +foo ()
> +{
> +  int i;
> +  return i;
> +}
> +
> +/* { dg-final { scan-tree-dump-not ".DEFERRED_INIT" "gimple" } } */
> diff --git a/gcc/testsuite/c-c++-common/fhardened-7.c b/gcc/testsuite/c-c++-common/fhardened-7.c
> new file mode 100644
> index 00000000000..60c48cde0f2
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-7.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fpie" } */
> +
> +/* -fpie takes precedence over -fhardened */
> +#if __PIE__ != 1
> +# error "__PIE__ != 1"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-8.c b/gcc/testsuite/c-c++-common/fhardened-8.c
> new file mode 100644
> index 00000000000..a4f01159a6f
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-8.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -fPIC" } */
> +
> +/* -fPIC takes precedence over -fhardened */
> +#ifdef __PIE__
> +# error "PIE enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/c-c++-common/fhardened-9.c b/gcc/testsuite/c-c++-common/fhardened-9.c
> new file mode 100644
> index 00000000000..f64e7eba56c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fhardened-9.c
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fhardened -U_FORTIFY_SOURCE -U_GLIBCXX_ASSERTIONS" } */
> +
> +#if defined(_FORTIFY_SOURCE) || defined(_GLIBCXX_ASSERTIONS)
> +# error "hardening enabled when it should not be"
> +#endif
> diff --git a/gcc/testsuite/gcc.misc-tests/help.exp b/gcc/testsuite/gcc.misc-tests/help.exp
> index 52b9cb0ab90..7786a465406 100644
> --- a/gcc/testsuite/gcc.misc-tests/help.exp
> +++ b/gcc/testsuite/gcc.misc-tests/help.exp
> @@ -151,6 +151,8 @@ foreach cls { "ada" "c" "c++" "d" "fortran" "go" \
>  # Listing only excludes gives empty results.
>  check_for_options c "--help=^joined,^separate" "" "" ""
>
> +# TODO -fhardened tests
> +
>  if [ info exists prev_columns ] {
>      # Reset the enviroment variable to its oriuginal value.
>      set env(COLUMNS) $prev_columns
> diff --git a/gcc/testsuite/gcc.target/i386/cf_check-6.c b/gcc/testsuite/gcc.target/i386/cf_check-6.c
> new file mode 100644
> index 00000000000..73b78dce889
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/cf_check-6.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fhardened -mno-manual-endbr" } */
> +/* { dg-final { scan-assembler-times {\mendbr} 1 } } */
> +/* Test that -fhardened enables CET.  */
> +
> +extern void bar (void) __attribute__((__cf_check__));
> +
> +void
> +foo (void)
> +{
> +  bar ();
> +}
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 6c1a6f443c1..01b6caba203 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -1575,6 +1575,12 @@ process_options (bool no_backend)
>                   "where the stack grows from lower to higher addresses");
>        flag_stack_clash_protection = 0;
>      }
> +  else if (flag_hardened
> +          && !flag_stack_clash_protection
> +          /* Don't enable -fstack-clash-protection when -fstack-check=
> +             is used: it would result in confusing errors.  */
> +          && flag_stack_check == NO_STACK_CHECK)
> +    flag_stack_clash_protection = 1;
>
>    /* We cannot support -fstack-check= and -fstack-clash-protection at
>       the same time.  */
>
> base-commit: fce74ce2535aa3b7648ba82e7e61eb77d0175546
> --
> 2.41.0
>
> Marek
>


-- 
BR,
Hongtao

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-30  8:46 ` Martin Uecker
@ 2023-09-15 15:11   ` Marek Polacek
  2023-09-16  7:40     ` Martin Uecker
  0 siblings, 1 reply; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:11 UTC (permalink / raw)
  To: Martin Uecker; +Cc: gcc-patches

On Wed, Aug 30, 2023 at 10:46:14AM +0200, Martin Uecker wrote:
> > Improving the security of software has been a major trend in the recent
> > years.  Fortunately, GCC offers a wide variety of flags that enable extra
> > hardening.  These flags aren't enabled by default, though.  And since
> > there are a lot of hardening flags, with more to come, it's been difficult
> > to keep on top of them; more so for the users of GCC who ought not to be
> > expected to keep track of all the new options.
> > 
> > To alleviate some of the problems I mentioned, we thought it would
> > be useful to provide a new umbrella option that enables a reasonable set
> > of hardening flags.  What's "reasonable" in this context is not easy to
> > pin down.  Surely, there must be no ABI impact, the option cannot cause
> > severe performance issues, and, I suspect, it should not cause build
> > errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> > -Wint-conversion).  Including a controversial option in -fhardened
> > would likely cause that users would not use -fhardened at all.  It's
> > roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> > options, and evolve over time, and are not kept in sync with other
> > compilers.
> > 
> > Currently, -fhardened enables:
> > 
> >   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> >   -D_GLIBCXX_ASSERTIONS
> >   -ftrivial-auto-var-init=zero
> >   -fPIE  -pie  -Wl,-z,relro,-z,now
> >   -fstack-protector-strong
> >   -fstack-clash-protection
> >   -fcf-protection=full (x86 GNU/Linux only)
> > 
> > -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> > also liable to break a lot of code so I didn't include it.
> > 
> > Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> > yet.  A fairly crucial point is that -fhardened will not override options
> > that were specified on the command line (before or after -fhardened).  For
> > example,
> >      
> >      -D_FORTIFY_SOURCE=1 -fhardened
> > 
> > means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> > 
> >       -fhardened -fstack-protector
> > 
> > will not enable -fstack-protector-strong.
> > 
> > Thoughts?
> 
> I think this is a great idea!  Considering that it is difficult to
> decide what shoud be activated and what not and the baseline should
> not cause compile errors,  I wonder whether there should be higher
> levels  similar to -O1,2,3 ? 
 
Thanks.  I would like to avoid any levels if at all possible; I think
they would be confusing.

> Although it would be nice to have a one-letter or very short
> option similar to -O2 or -Wall, but maybe this is not possible 
> because all short ones are already taken. Of course, 
> "-fhardening" would  already a huge  improvement to the 
> current situation.

There are some free ones, like -Z, but I'm not confident I could take
it :).

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-30  9:06 ` Xi Ruoyao
@ 2023-09-15 15:12   ` Marek Polacek
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:12 UTC (permalink / raw)
  To: Xi Ruoyao; +Cc: GCC Patches

On Wed, Aug 30, 2023 at 05:06:57PM +0800, Xi Ruoyao via Gcc-patches wrote:
> On Tue, 2023-08-29 at 15:42 -0400, Marek Polacek via Gcc-patches wrote:
> > +         if (UNLIKELY (flag_hardened)
> > +             && (opt->code == OPT_D || opt->code == OPT_U))
> > +           {
> > +             if (!fortify_seen_p)
> > +               fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> > +             if (!cxx_assert_seen_p)
> > +               cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
> 
> It looks like there is some minor logic issue here: the first strncmp
> will mistakenly match "-D_FORTIFY_SOURCE_FAKE", and the second strcmp
> will not match "-D_GLIBCXX_ASSERTIONS=1".

Thanks, you're right.  Should be fixed in
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-30 10:50 ` Jakub Jelinek
  2023-08-30 13:08   ` Richard Biener
@ 2023-09-15 15:17   ` Marek Polacek
  1 sibling, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:17 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches

On Wed, Aug 30, 2023 at 12:50:40PM +0200, Jakub Jelinek wrote:
> On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote:
> > +	  if (UNLIKELY (flag_hardened)
> > +	      && (opt->code == OPT_D || opt->code == OPT_U))
> > +	    {
> > +	      if (!fortify_seen_p)
> > +		fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> 
> Perhaps this should check that the char after it is either '\0' or '=', we
> shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro.

Right, should be fixed in
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html
 
> > +	      if (!cxx_assert_seen_p)
> > +		cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
> 
> Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42

This too.
 
> > +	    }
> > +	}
> > +
> > +      if (flag_hardened)
> > +	{
> > +	  if (!fortify_seen_p && optimize > 0)
> > +	    {
> > +	      if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> > +		cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> > +	      else
> > +		cpp_define (parse_in, "_FORTIFY_SOURCE=2");
> 
> I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for
> -fhardened only for targets which actually have such a support in the C
> library.  There is some poor man's _FORTIFY_SOURCE support in libssp,
> but e.g. one has to link with -lssp in that case and compile with
> -isystem `gcc -print-include-filename=include`/ssp .
> For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source
> mentions NetBSD support since 2006, newlib since 2017, some Darwin libc,
> bionic (but seems they have only some clang support and dropped GCC
> support) and some third party reimplementation of libssp.
> Or do we just enable it and hope that either it works well or isn't
> supported at all quietly?  E.g. it would certainly break the ssp case
> where -isystem finds ssp headers but -lssp isn't linked in.

I don't really have an idea how this should be handled.  I left it as
it was.
 
> > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
> >  #endif
> >      }
> >  
> > +  /* TODO: check if -static -pie works and maybe use it.  */
> > +  if (flag_hardened && !any_link_options_p && !static_p)
> > +    {
> > +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> > +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> > +      if (true)
> > +	{
> > +	  /* These are passed straight down to collect2 so we have to break
> > +	     it up like this.  */
> > +	  add_infile ("-z", "*");
> > +	  add_infile ("now", "*");
> > +	  add_infile ("-z", "*");
> > +	  add_infile ("relro", "*");
> 
> As the TODO comment says, to do that we need to check at configure time that
> linker supports -z now and -z relro options.

I've added configure checks in
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html

> > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
> >      }
> >  
> >    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> > -     can set a default value.  */
> > +     can set a default value.  With --enable-default-ssp or -fhardened
> > +     the default is -fstack-protector-strong.  */
> >    if (opts->x_flag_stack_protect == -1)
> > -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> > +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> > +				  ? SPCT_FLAG_STRONG
> > +				  : DEFAULT_FLAG_SSP);
> 
> This needs to be careful, -fstack-protector isn't supported on all targets
> (e.g. ia64) and we don't want toplev.cc warning:
>   /* Targets must be able to place spill slots at lower addresses.  If the
>      target already uses a soft frame pointer, the transition is trivial.  */
>   if (!FRAME_GROWS_DOWNWARD && flag_stack_protect)
>     {
>       warning_at (UNKNOWN_LOCATION, 0,
>                   "%<-fstack-protector%> not supported for this target");
>       flag_stack_protect = 0;
>     }
> to be emitted whenever using -fhardened, it should not be enabled there
> silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it
> work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was
> enabled and otherwise keep it !FRAME_GROWS_DOWNWARD).

Unfortunately, I found out that I can't just check FRAME_GROWS_DOWNWARD.
Some targets like rs6000 and mips use flag_stack_protect in the definition
of FRAME_GROWS_DOWNWARD, and that is not usable in finish_options.  There,
you have to use opts->x_flag_*.  Besides, some targets like BPF don't
support -fstack-protector at all, but in finish_options we can't know that
yet (AFAIK).

I fixed it, but in a very ugly way, using a new global.  :/

Thanks,

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-30 13:08   ` Richard Biener
@ 2023-09-15 15:21     ` Marek Polacek
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:21 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches

On Wed, Aug 30, 2023 at 03:08:46PM +0200, Richard Biener wrote:
> On Wed, Aug 30, 2023 at 12:51 PM Jakub Jelinek via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Tue, Aug 29, 2023 at 03:42:27PM -0400, Marek Polacek via Gcc-patches wrote:
> > > +       if (UNLIKELY (flag_hardened)
> > > +           && (opt->code == OPT_D || opt->code == OPT_U))
> > > +         {
> > > +           if (!fortify_seen_p)
> > > +             fortify_seen_p = !strncmp (opt->arg, "_FORTIFY_SOURCE", 15);
> >
> > Perhaps this should check that the char after it is either '\0' or '=', we
> > shouldn't care if user defines or undefines _FORTIFY_SOURCE_WHATEVER macro.
> >
> > > +           if (!cxx_assert_seen_p)
> > > +             cxx_assert_seen_p = !strcmp (opt->arg, "_GLIBCXX_ASSERTIONS");
> >
> > Like we don't care in this case about -D_GLIBCXX_ASSERTIONS42
> >
> > > +         }
> > > +     }
> > > +
> > > +      if (flag_hardened)
> > > +     {
> > > +       if (!fortify_seen_p && optimize > 0)
> > > +         {
> > > +           if (TARGET_GLIBC_MAJOR == 2 && TARGET_GLIBC_MINOR >= 35)
> > > +             cpp_define (parse_in, "_FORTIFY_SOURCE=3");
> > > +           else
> > > +             cpp_define (parse_in, "_FORTIFY_SOURCE=2");
> >
> > I wonder if it wouldn't be better to enable _FORTIFY_SOURCE=2 by default for
> > -fhardened only for targets which actually have such a support in the C
> > library.  There is some poor man's _FORTIFY_SOURCE support in libssp,
> > but e.g. one has to link with -lssp in that case and compile with
> > -isystem `gcc -print-include-filename=include`/ssp .
> > For glibc that is >= 2.3.4, https://maskray.me/blog/2022-11-06-fortify-source
> > mentions NetBSD support since 2006, newlib since 2017, some Darwin libc,
> > bionic (but seems they have only some clang support and dropped GCC
> > support) and some third party reimplementation of libssp.
> > Or do we just enable it and hope that either it works well or isn't
> > supported at all quietly?  E.g. it would certainly break the ssp case
> > where -isystem finds ssp headers but -lssp isn't linked in.
> >
> > > @@ -4976,6 +4993,22 @@ process_command (unsigned int decoded_options_count,
> > >  #endif
> > >      }
> > >
> > > +  /* TODO: check if -static -pie works and maybe use it.  */
> > > +  if (flag_hardened && !any_link_options_p && !static_p)
> > > +    {
> > > +      save_switch ("-pie", 0, NULL, /*validated=*/true, /*known=*/false);
> > > +      /* TODO: check if BIND_NOW/RELRO is supported.  */
> > > +      if (true)
> > > +     {
> > > +       /* These are passed straight down to collect2 so we have to break
> > > +          it up like this.  */
> > > +       add_infile ("-z", "*");
> > > +       add_infile ("now", "*");
> > > +       add_infile ("-z", "*");
> > > +       add_infile ("relro", "*");
> >
> > As the TODO comment says, to do that we need to check at configure time that
> > linker supports -z now and -z relro options.
> >
> > > @@ -1117,9 +1121,12 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
> > >      }
> > >
> > >    /* We initialize opts->x_flag_stack_protect to -1 so that targets
> > > -     can set a default value.  */
> > > +     can set a default value.  With --enable-default-ssp or -fhardened
> > > +     the default is -fstack-protector-strong.  */
> > >    if (opts->x_flag_stack_protect == -1)
> > > -    opts->x_flag_stack_protect = DEFAULT_FLAG_SSP;
> > > +    opts->x_flag_stack_protect = (opts->x_flag_hardened
> > > +                               ? SPCT_FLAG_STRONG
> > > +                               : DEFAULT_FLAG_SSP);
> >
> > This needs to be careful, -fstack-protector isn't supported on all targets
> > (e.g. ia64) and we don't want toplev.cc warning:
> >   /* Targets must be able to place spill slots at lower addresses.  If the
> >      target already uses a soft frame pointer, the transition is trivial.  */
> >   if (!FRAME_GROWS_DOWNWARD && flag_stack_protect)
> >     {
> >       warning_at (UNKNOWN_LOCATION, 0,
> >                   "%<-fstack-protector%> not supported for this target");
> >       flag_stack_protect = 0;
> >     }
> > to be emitted whenever using -fhardened, it should not be enabled there
> > silently (for ia64 Fedora/RHEL gcc actually had a short patch to make it
> > work, turn the target into FRAME_GROWS_DOWNWARD one if -fstack-protect* was
> > enabled and otherwise keep it !FRAME_GROWS_DOWNWARD).
> 
> I'll note that with selectively enabling parts of -fhardening it can
> also give a false
> sensation of safety when under the hood we ignore half of the option due to
> one or another reason ...

I suppose you're right :/.
 
> How does -fhardening reflect into -[gf]record-gcc-switches?  Is it at
> least possible
> to verify the actually enabled bits?

In DW_AT_producer it simply shows "-fhardened".  It doesn't expand to what
it actually enabled.  I imagine gen_producer_string could be tweaked to
print the options enabled by -fhardened.  But, DW_AT_producer doesn't seem
to show -D options, or linked options like -Wl,-z,now.  So I think we need
something better.  Maybe add a line in gcc -v?  I would not like to add
a new option just for this.

(It would have to be careful not to show options that the target doesn't
support, like -fstack-protector on BPF.)

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-04 22:40   ` Richard Sandiford
@ 2023-09-15 15:23     ` Marek Polacek
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:23 UTC (permalink / raw)
  To: Qing Zhao via Gcc-patches, Qing Zhao, richard.sandiford

On Mon, Sep 04, 2023 at 11:40:34PM +0100, Richard Sandiford wrote:
> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >> 
> >> Improving the security of software has been a major trend in the recent
> >> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> >> hardening.  These flags aren't enabled by default, though.  And since
> >> there are a lot of hardening flags, with more to come, it's been difficult
> >> to keep on top of them; more so for the users of GCC who ought not to be
> >> expected to keep track of all the new options.
> >> 
> >> To alleviate some of the problems I mentioned, we thought it would
> >> be useful to provide a new umbrella option that enables a reasonable set
> >> of hardening flags.  What's "reasonable" in this context is not easy to
> >> pin down.  Surely, there must be no ABI impact, the option cannot cause
> >> severe performance issues, and, I suspect, it should not cause build
> >> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> >> -Wint-conversion).  Including a controversial option in -fhardened
> >> would likely cause that users would not use -fhardened at all.  It's
> >> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> >> options, and evolve over time, and are not kept in sync with other
> >> compilers.
> >> 
> >> Currently, -fhardened enables:
> >> 
> >>  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> >>  -D_GLIBCXX_ASSERTIONS
> >>  -ftrivial-auto-var-init=zero
> >>  -fPIE  -pie  -Wl,-z,relro,-z,now
> >>  -fstack-protector-strong
> >>  -fstack-clash-protection
> >>  -fcf-protection=full (x86 GNU/Linux only)
> >> 
> >> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> >> also liable to break a lot of code so I didn't include it.
> >> 
> >> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> >> yet.  A fairly crucial point is that -fhardened will not override options
> >> that were specified on the command line (before or after -fhardened).  For
> >> example,
> >> 
> >>     -D_FORTIFY_SOURCE=1 -fhardened
> >> 
> >> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> >> 
> >>      -fhardened -fstack-protector
> >> 
> >> will not enable -fstack-protector-strong.
> >> 
> >> Thoughts?
> >
> > In general, I think that it is a very good idea to provide umbrella options
> >  for software security purpose.  Thanks a lot for this work!
> >
> > 1. I do agree with Martin, multiple-level control for this purpose might be needed,
> > similar as multiple levels for warnings, and multiple levels for optimizations.
> >
> > Similar as optimization options, can we organize all the security options together 
> > In our manual, then the user will have a good central place to get more and complete
> > Information of the security features our compiler provides? 
> >
> > 2. What’s the major criteria to decide which security feature should go into this list?
> > Later, when we have new security features, how to decide whether to add them to
> > This list or not?
> > I am wondering why -fzero-call-used-regs is not included in the list and also
> 
> FWIW, I wondered the same thing.  Not a strong conviction that it should
> be included -- maybe the code bloat is too much on some targets.  But it
> might be acceptable for the -fhardened equivalent of -O3, at least if
> restricted to GPRs.

I have no opinion here.  I trust you so if you think it'd make sense, I will
add it.  :)

The patch I posted today:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630550.html
doesn't include that option yet.

> > Why chose -ftrivial-auto-var-init=zero instead of -ftrivial-auto-var-init=pattern? 
> 
> Yeah, IIRC -ftrivial-auto-var-init=zero was controversial with some
> Clang maintainers because it effectively creates a language dialect.
> -ftrivial-auto-var-init=pattern wasn't controversial in the same way.

Thanks.  I've adjusted the patch to enable -ftrivial-auto-var-init=pattern
rather than -ftrivial-auto-var-init=zero.

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-01 22:09 ` Qing Zhao
  2023-09-04 22:40   ` Richard Sandiford
@ 2023-09-15 15:40   ` Marek Polacek
  1 sibling, 0 replies; 25+ messages in thread
From: Marek Polacek @ 2023-09-15 15:40 UTC (permalink / raw)
  To: Qing Zhao; +Cc: GCC Patches

On Fri, Sep 01, 2023 at 10:09:28PM +0000, Qing Zhao via Gcc-patches wrote:
> 
> 
> > On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > 
> > Improving the security of software has been a major trend in the recent
> > years.  Fortunately, GCC offers a wide variety of flags that enable extra
> > hardening.  These flags aren't enabled by default, though.  And since
> > there are a lot of hardening flags, with more to come, it's been difficult
> > to keep on top of them; more so for the users of GCC who ought not to be
> > expected to keep track of all the new options.
> > 
> > To alleviate some of the problems I mentioned, we thought it would
> > be useful to provide a new umbrella option that enables a reasonable set
> > of hardening flags.  What's "reasonable" in this context is not easy to
> > pin down.  Surely, there must be no ABI impact, the option cannot cause
> > severe performance issues, and, I suspect, it should not cause build
> > errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> > -Wint-conversion).  Including a controversial option in -fhardened
> > would likely cause that users would not use -fhardened at all.  It's
> > roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> > options, and evolve over time, and are not kept in sync with other
> > compilers.
> > 
> > Currently, -fhardened enables:
> > 
> >  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> >  -D_GLIBCXX_ASSERTIONS
> >  -ftrivial-auto-var-init=zero
> >  -fPIE  -pie  -Wl,-z,relro,-z,now
> >  -fstack-protector-strong
> >  -fstack-clash-protection
> >  -fcf-protection=full (x86 GNU/Linux only)
> > 
> > -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> > also liable to break a lot of code so I didn't include it.
> > 
> > Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> > yet.  A fairly crucial point is that -fhardened will not override options
> > that were specified on the command line (before or after -fhardened).  For
> > example,
> > 
> >     -D_FORTIFY_SOURCE=1 -fhardened
> > 
> > means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> > 
> >      -fhardened -fstack-protector
> > 
> > will not enable -fstack-protector-strong.
> > 
> > Thoughts?
> 
> In general, I think that it is a very good idea to provide umbrella options
>  for software security purpose.  Thanks a lot for this work!

And thank you for taking a look!

> 1. I do agree with Martin, multiple-level control for this purpose might be needed,
> similar as multiple levels for warnings, and multiple levels for optimizations.

As I just mentioned in the other email, I'm reluctant to add
more levels, like -fhardened=2, at this time.  I think it's confusing,
because if there are multiple choices, then as a user you have to
figure out which one you want and this option is supposed to simplify
things, not to puzzle people with one more decision.
 
> Similar as optimization options, can we organize all the security options together 
> In our manual, then the user will have a good central place to get more and complete
> Information of the security features our compiler provides? 

Maybe.  But going through all the options and deciding whether
it's a security options may be too big a task, especially if we
are not exactly sure how we define security.
 
> 2. What’s the major criteria to decide which security feature should go into this list?
> Later, when we have new security features, how to decide whether to add them to
> This list or not?

From my original post:

What's "reasonable" in this context is not easy to
pin down.  Surely, there must be no ABI impact, the option cannot cause
severe performance issues, and, I suspect, it should not cause build
errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
-Wint-conversion).  Including a controversial option in -fhardened
would likely cause that users would not use -fhardened at all.

> I am wondering why -fzero-call-used-regs is not included in the list and also 
> Why chose -ftrivial-auto-var-init=zero instead of -ftrivial-auto-var-init=pattern? 

I can't readily evaluate the effect of -fzero-call-used-regs.  But I did
change =zero to =pattern.

> 3. Looks like currently, -fhardened excludes all compilation-time Warning options for security purpose,
> (For example, -Warray-bounds, --Wstringop-overflow…)

Correct.

> And also excludes all sanitizer options for security purpose (-fsanitizer=undifined)

Correct.

> So, shall we also provide an umbrella option for compilation-time warning options for security purpose

I don't think so... we already have -Wall and -Wextra.

> And a umbrella option for sanitizer options (is the -fsanitizer=undefined this one)?

Yes, -fsanitizer=undefined is already an umbrella option.

Thanks,

Marek


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-15 15:11   ` Marek Polacek
@ 2023-09-16  7:40     ` Martin Uecker
  0 siblings, 0 replies; 25+ messages in thread
From: Martin Uecker @ 2023-09-16  7:40 UTC (permalink / raw)
  To: Marek Polacek; +Cc: gcc-patches

Am Freitag, dem 15.09.2023 um 11:11 -0400 schrieb Marek Polacek:
> On Wed, Aug 30, 2023 at 10:46:14AM +0200, Martin Uecker wrote:
> > > Improving the security of software has been a major trend in the recent
> > > years.  Fortunately, GCC offers a wide variety of flags that enable extra
> > > hardening.  These flags aren't enabled by default, though.  And since
> > > there are a lot of hardening flags, with more to come, it's been difficult
> > > to keep on top of them; more so for the users of GCC who ought not to be
> > > expected to keep track of all the new options.
> > > 
> > > To alleviate some of the problems I mentioned, we thought it would
> > > be useful to provide a new umbrella option that enables a reasonable set
> > > of hardening flags.  What's "reasonable" in this context is not easy to
> > > pin down.  Surely, there must be no ABI impact, the option cannot cause
> > > severe performance issues, and, I suspect, it should not cause build
> > > errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> > > -Wint-conversion).  Including a controversial option in -fhardened
> > > would likely cause that users would not use -fhardened at all.  It's
> > > roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> > > options, and evolve over time, and are not kept in sync with other
> > > compilers.
> > > 
> > > Currently, -fhardened enables:
> > > 
> > >   -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
> > >   -D_GLIBCXX_ASSERTIONS
> > >   -ftrivial-auto-var-init=zero
> > >   -fPIE  -pie  -Wl,-z,relro,-z,now
> > >   -fstack-protector-strong
> > >   -fstack-clash-protection
> > >   -fcf-protection=full (x86 GNU/Linux only)
> > > 
> > > -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> > > also liable to break a lot of code so I didn't include it.
> > > 
> > > Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> > > yet.  A fairly crucial point is that -fhardened will not override options
> > > that were specified on the command line (before or after -fhardened).  For
> > > example,
> > >      
> > >      -D_FORTIFY_SOURCE=1 -fhardened
> > > 
> > > means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> > > 
> > >       -fhardened -fstack-protector
> > > 
> > > will not enable -fstack-protector-strong.
> > > 
> > > Thoughts?
> > 
> > I think this is a great idea!  Considering that it is difficult to
> > decide what shoud be activated and what not and the baseline should
> > not cause compile errors,  I wonder whether there should be higher
> > levels  similar to -O1,2,3 ? 
>  
> Thanks.  I would like to avoid any levels if at all possible; I think
> they would be confusing.
> 
> > Although it would be nice to have a one-letter or very short
> > option similar to -O2 or -Wall, but maybe this is not possible 
> > because all short ones are already taken. Of course, 
> > "-fhardening" would  already a huge  improvement to the 
> > current situation.
> 
> There are some free ones, like -Z, but I'm not confident I could take
> it :).
> 

It would send a message.

Today I can get crazy optimizations with 

-O3 

but for (somewhat) decent security, I need something
like:

 -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
  -D_GLIBCXX_ASSERTIONS
  -ftrivial-auto-var-init=pattern
  -fPIE  -pie  -Wl,-z,relro,-z,now
  -fstack-protector-strong
  -fstack-clash-protection
  -fcf-protection=full 
  -fsanitize=undefined
  -fsanitize-undefined-trap-on-error
  -Wall
  -Wextra

which also sends a message.

Martin




^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
                   ` (5 preceding siblings ...)
  2023-09-14 14:48 ` Hongtao Liu
@ 2023-09-17  3:16 ` Hans-Peter Nilsson
  2023-09-17  4:00   ` Sam James
  2023-09-20 11:33 ` jvoisin
  7 siblings, 1 reply; 25+ messages in thread
From: Hans-Peter Nilsson @ 2023-09-17  3:16 UTC (permalink / raw)
  To: Marek Polacek; +Cc: gcc-patches

> Date: Tue, 29 Aug 2023 15:42:27 -0400
> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org>

> Surely, there must be no ABI impact, the option cannot cause
> severe performance issues,

> Currently, -fhardened enables:
...
>   -ftrivial-auto-var-init=zero

> Thoughts?

Regarding -ftrivial-auto-var-init=zero, I was consulted when
colleagues investigating a performance regression
pint-pointed it as *causing severe performance issues*;
cf. https://github.com/systemd/systemd.git commit 1a4e392760
(TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd
build).

The situation was described as "we noticed that some test
suites takes 35% percent longer time to finish.  After
further investigation it was noticed that running systemctl
unmask x takes around 5s more time on [version including
patch vs. before that patch]" (timing out some tests).
Reverting that patch fixed the drop in performance.

Just a data point, but I believe also exactly your intended
use.  IMO including -ftrivial-auto-var-init is worth extra
consideration.

Alternatively, strike the while "cannot cause severe
performance issues".

brgds, H-P

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-17  3:16 ` Hans-Peter Nilsson
@ 2023-09-17  4:00   ` Sam James
  2023-09-17 16:36     ` Hans-Peter Nilsson
  0 siblings, 1 reply; 25+ messages in thread
From: Sam James @ 2023-09-17  4:00 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Marek Polacek, gcc-patches


Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:

>> Date: Tue, 29 Aug 2023 15:42:27 -0400
>> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org>
>
>> Surely, there must be no ABI impact, the option cannot cause
>> severe performance issues,
>
>> Currently, -fhardened enables:
> ...
>>   -ftrivial-auto-var-init=zero
>
>> Thoughts?
>
> Regarding -ftrivial-auto-var-init=zero, I was consulted when
> colleagues investigating a performance regression
> pint-pointed it as *causing severe performance issues*;
> cf. https://github.com/systemd/systemd.git commit 1a4e392760
> (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd
> build).
>
> The situation was described as "we noticed that some test
> suites takes 35% percent longer time to finish.  After
> further investigation it was noticed that running systemctl
> unmask x takes around 5s more time on [version including
> patch vs. before that patch]" (timing out some tests).
> Reverting that patch fixed the drop in performance.

Did some bug ever get filed for this to see if we can do a bit
better here?

Some slowdown doesn't mean it's of the expected magnitude.

>
> Just a data point, but I believe also exactly your intended
> use.  IMO including -ftrivial-auto-var-init is worth extra
> consideration.
>
> Alternatively, strike the while "cannot cause severe
> performance issues".
>
> brgds, H-P


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-17  4:00   ` Sam James
@ 2023-09-17 16:36     ` Hans-Peter Nilsson
  2023-09-18  7:21       ` Sam James
  2023-09-19 14:19       ` Qing Zhao
  0 siblings, 2 replies; 25+ messages in thread
From: Hans-Peter Nilsson @ 2023-09-17 16:36 UTC (permalink / raw)
  To: Sam James; +Cc: polacek, gcc-patches

> From: Sam James <sam@gentoo.org>
> Date: Sun, 17 Sep 2023 05:00:37 +0100

> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> 
> >> Date: Tue, 29 Aug 2023 15:42:27 -0400
> >> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org>
> >
> >> Surely, there must be no ABI impact, the option cannot cause
> >> severe performance issues,
> >
> >> Currently, -fhardened enables:
> > ...
> >>   -ftrivial-auto-var-init=zero
> >
> >> Thoughts?
> >
> > Regarding -ftrivial-auto-var-init=zero, I was consulted when
> > colleagues investigating a performance regression
> > pint-pointed it as *causing severe performance issues*;
> > cf. https://github.com/systemd/systemd.git commit 1a4e392760
> > (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd
> > build).
> >
> > The situation was described as "we noticed that some test
> > suites takes 35% percent longer time to finish.  After
> > further investigation it was noticed that running systemctl
> > unmask x takes around 5s more time on [version including
> > patch vs. before that patch]" (timing out some tests).
> > Reverting that patch fixed the drop in performance.
> 
> Did some bug ever get filed for this to see if we can do a bit
> better here?

Not that I know of; neither for systemd nor gcc.

> Some slowdown doesn't mean it's of the expected magnitude.

Can you please rephrase that?

> > Just a data point, but I believe also exactly your intended
> > use.  IMO including -ftrivial-auto-var-init is worth extra
> > consideration.
> >
> > Alternatively, strike the while "cannot cause severe
> > performance issues".
> >
> > brgds, H-P
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-17 16:36     ` Hans-Peter Nilsson
@ 2023-09-18  7:21       ` Sam James
  2023-09-18 14:34         ` Hans-Peter Nilsson
  2023-09-19 14:19       ` Qing Zhao
  1 sibling, 1 reply; 25+ messages in thread
From: Sam James @ 2023-09-18  7:21 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Sam James, polacek, gcc-patches


Hans-Peter Nilsson <hp@axis.com> writes:

>> From: Sam James <sam@gentoo.org>
>> Date: Sun, 17 Sep 2023 05:00:37 +0100
>
>> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> 
>> >> Date: Tue, 29 Aug 2023 15:42:27 -0400
>> >> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org>
>> >
>> >> Surely, there must be no ABI impact, the option cannot cause
>> >> severe performance issues,
>> >
>> >> Currently, -fhardened enables:
>> > ...
>> >>   -ftrivial-auto-var-init=zero
>> >
>> >> Thoughts?
>> >
>> > Regarding -ftrivial-auto-var-init=zero, I was consulted when
>> > colleagues investigating a performance regression
>> > pint-pointed it as *causing severe performance issues*;
>> > cf. https://github.com/systemd/systemd.git commit 1a4e392760
>> > (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd
>> > build).
>> >
>> > The situation was described as "we noticed that some test
>> > suites takes 35% percent longer time to finish.  After
>> > further investigation it was noticed that running systemctl
>> > unmask x takes around 5s more time on [version including
>> > patch vs. before that patch]" (timing out some tests).
>> > Reverting that patch fixed the drop in performance.
>> 
>> Did some bug ever get filed for this to see if we can do a bit
>> better here?
>
> Not that I know of; neither for systemd nor gcc.
>
>> Some slowdown doesn't mean it's of the expected magnitude.
>
> Can you please rephrase that?

Sorry, what I meant was: yes, I'd expect a bit of a slowdown
with this option that's unavoidable, but what you're describing
sounds far beyond the normal case.

(to the extent that it might be
we're either missing an optimisation in GCC for the relevant bits,
or the systemd parts being exercised here are pathological.)



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-18  7:21       ` Sam James
@ 2023-09-18 14:34         ` Hans-Peter Nilsson
  0 siblings, 0 replies; 25+ messages in thread
From: Hans-Peter Nilsson @ 2023-09-18 14:34 UTC (permalink / raw)
  To: Sam James; +Cc: sam, polacek, gcc-patches

> From: Sam James <sam@gentoo.org>
> Date: Mon, 18 Sep 2023 08:21:45 +0100

> Hans-Peter Nilsson <hp@axis.com> writes:
> 
> >> From: Sam James <sam@gentoo.org>
> >> Date: Sun, 17 Sep 2023 05:00:37 +0100
> >
> >> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >> > The situation was described as "we noticed that some test
> >> > suites takes 35% percent longer time to finish.  After
> >> > further investigation it was noticed that running systemctl
> >> > unmask x takes around 5s more time on [version including
> >> > patch vs. before that patch]" (timing out some tests).
> >> > Reverting that patch fixed the drop in performance.
> >> 
> >> Did some bug ever get filed for this to see if we can do a bit
> >> better here?
> >
> > Not that I know of; neither for systemd nor gcc.
> >
> >> Some slowdown doesn't mean it's of the expected magnitude.
> >
> > Can you please rephrase that?
> 
> Sorry, what I meant was: yes, I'd expect a bit of a slowdown
> with this option that's unavoidable, but what you're describing
> sounds far beyond the normal case.

Thanks.  My take is that it's something I more or less
expect when indiscriminately using that option.  IOW, for
code consciously using that option, a step is necessary
where the code is vetted and adjusted, for example to remove
large structures and arrays allocated on the stack (just a
guess).

> (to the extent that it might be
> we're either missing an optimisation in GCC for the relevant bits,
> or the systemd parts being exercised here are pathological.)

Sorry, I don't have anything more in terms of local
observations from that investigation; no perf and
flamegraphs.  Again as above, the issue should be observable
as a noticeable slowdown for "systemctl unmask x" for a
systemd compiled with/without 1a4e392760.

brgds, H-P

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-17 16:36     ` Hans-Peter Nilsson
  2023-09-18  7:21       ` Sam James
@ 2023-09-19 14:19       ` Qing Zhao
  2023-09-21 16:53         ` Hans-Peter Nilsson
  1 sibling, 1 reply; 25+ messages in thread
From: Qing Zhao @ 2023-09-19 14:19 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Sam James, polacek, gcc-patches



> On Sep 17, 2023, at 12:36 PM, Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
>> From: Sam James <sam@gentoo.org>
>> Date: Sun, 17 Sep 2023 05:00:37 +0100
> 
>> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> 
>>>> Date: Tue, 29 Aug 2023 15:42:27 -0400
>>>> From: Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org>
>>> 
>>>> Surely, there must be no ABI impact, the option cannot cause
>>>> severe performance issues,
>>> 
>>>> Currently, -fhardened enables:
>>> ...
>>>>  -ftrivial-auto-var-init=zero
>>> 
>>>> Thoughts?
>>> 
>>> Regarding -ftrivial-auto-var-init=zero, I was consulted when
>>> colleagues investigating a performance regression
>>> pint-pointed it as *causing severe performance issues*;
>>> cf. https://github.com/systemd/systemd.git commit 1a4e392760
>>> (TL;DR: adds "-ftrivial-auto-var-init=zero" to the systemd
>>> build).
>>> 
>>> The situation was described as "we noticed that some test
>>> suites takes 35% percent longer time to finish.  After
>>> further investigation it was noticed that running systemctl
>>> unmask x takes around 5s more time on [version including
>>> patch vs. before that patch]" (timing out some tests).
>>> Reverting that patch fixed the drop in performance.
>> 
>> Did some bug ever get filed for this to see if we can do a bit
>> better here?
> 
> Not that I know of; neither for systemd nor gcc.

Then, is it convenient to file a bug on this?  That will be very helpful for us to locate the issue and fix it.

Before I committing the -ftrivial-auto-var-init patch, I have done some performance testing on CPU2017 for x86 and aarch64,
The runtime overhead was quite limited. 

Which platform the 35% performance slowdown was on?

Thanks.

Qing
> 
>> Some slowdown doesn't mean it's of the expected magnitude.
> 
> Can you please rephrase that?
> 
>>> Just a data point, but I believe also exactly your intended
>>> use.  IMO including -ftrivial-auto-var-init is worth extra
>>> consideration.
>>> 
>>> Alternatively, strike the while "cannot cause severe
>>> performance issues".
>>> 
>>> brgds, H-P


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
                   ` (6 preceding siblings ...)
  2023-09-17  3:16 ` Hans-Peter Nilsson
@ 2023-09-20 11:33 ` jvoisin
  7 siblings, 0 replies; 25+ messages in thread
From: jvoisin @ 2023-09-20 11:33 UTC (permalink / raw)
  To: polacek; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

I'd like to provide some data-points on hardening-related flags, as I've
spent some time with Sam documenting their usage across various
distributions here[1]. I also attached the relevant file to this email
for archiving purposes.

tl'dr: the suggested flag selection for `-fhardened` is not only sound
but great. The only issue I see is `-fcf-protection=full`, since musl
libc doesn't support it for now, so it might cause runtime issues.

1. https://github.com/jvoisin/compiler-flags-distro/blob/main/README.md

-- 
Julien (jvoisin) Voisin
GPG: 04D041E8171901CC
dustri.org

[-- Attachment #2: README.md --]
[-- Type: text/markdown, Size: 10252 bytes --]

# Usage of enabled-by-default hardening-related compiler flags across Linux distributions

|.                                | Alpine | Debian | Fedora    | Gentoo Hardened | Ubuntu | OpenSUSE | ArchLinux | OpenBSD | Chimera Linux | Android |
|---------------------------------|--------|--------|-----------|-----------------|--------|----------|-----------|---------|---------------|---------|
|`-D_FORTIFY_SOURCE=2`            |[yes](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[2011](https://github.com/guillemj/dpkg/commit/f3bb7d4939ae95cf44c89e8f599e7ed5da431e57)|[2007](https://listman.redhat.com/archives/fedora-devel-announce/2007-September/msg00015.html)|superseded|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags#A-D_FORTIFY_SOURCE.3D2)|[2005](https://en.opensuse.org/openSUSE:Security_Features)|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|?|[yes](https://github.com/chimera-linux/cports/commit/9b78e55067f024b8dbf9fbceb472e8705f84ed5d)|[2017](https://android-developers.googleblog.com/2019/10/introducing-ndk-r21-our-first-long-term.html)|
|`-D_FORTIFY_SOURCE=3`            |no      |[no](https://wiki.debian.org/Hardening)|[2023](https://fedoraproject.org/wiki/Changes/Add_FORTIFY_SOURCE%3D3_to_distribution_build_flags)|[2022](https://bugs.gentoo.org/876893)|[no](https://bugs.launchpad.net/ubuntu/+source/gcc-12/+bug/2012440)|[2023](https://en.opensuse.org/openSUSE:Security_Features)|[not](https://gitlab.archlinux.org/archlinux/rfcs/-/merge_requests/17) [yet](https://gitlab.archlinux.org/archlinux/devtools/-/merge_requests/191)|?|no|[no](https://android.googlesource.com/platform/bionic.git/+/HEAD/docs/status.md#fortify)|
|`-D_GLIBCXX_ASSERTIONS`          |[2023](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/44c933da5d8e364d6cd755071f629c05444191df)|no|[2018](https://fedoraproject.org/wiki/Changes/HardeningFlags28)|[2022](https://bugs.gentoo.org/876895)|[no](https://bugs.launchpad.net/ubuntu/+source/gcc-12/+bug/2016042)|yes|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|no|no|no|
|`-D_LIBCPP_ENABLE_HARDENED_MODE` (llvm17) |[not yet](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/65b5d578b2d9e3f170bc9d31dcd23f0014cfc36e)[^1]|no|no|[2023](https://bugs.gentoo.org/851111)|no|no|no|?|?|no|
|`-D_LIBCXX_ENABLE_ASSERTIONS` (llvm16) |no|no|no|superseded|no|no|no|?|[yes](https://github.com/search?q=repo%3Achimera-linux%2Fcports+DLIBCXX_ENABLE_ASSERTIONS&type=code)|?|
|`-Wformat -Wformat-security`/`-Wformat=2` |[2023](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/ca8375f0e9d1715e38c14c918c675d6774f1eabc)|[2011](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2013](https://fedoraproject.org/wiki/Changes/FormatSecurity)|[2009](https://bugs.gentoo.org/259417)|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|yes|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|?|[2023](https://github.com/chimera-linux/cports/commit/ad898a6b645b11dee989f4504e89577f5395ba24)|[2010](https://source.android.com/docs/security/enhancements/enhancements41)|
|`-Wl,-z,noexecstack`             |yes|yes|yes|yes|yes|yes|yes|yes|yes|yes|
|`-Wl,-z,relro`/`-Wl,-z,now`      |[yes](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[yes](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2015](https://fedoraproject.org/wiki/Security_Features_Matrix#Built_as_PIE)|[yes](https://wiki.gentoo.org/wiki/Hardened/Toolchain)|[2008](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|[2006](https://en.opensuse.org/openSUSE:Security_Features)|[2017](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/b4b2bb56174493ea2e60b1eecc0085db421908cc)|?|[yes](https://github.com/chimera-linux/cports/commit/9b78e55067f024b8dbf9fbceb472e8705f84ed5d)|[2013](https://source.android.com/docs/security/enhancements/enhancements43)|
|`-fPIE`/`-fPIC`/…                |[2008](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/fdc478bde8a2a0d76d33fcc89fa313c9f31bb79c)|[2011](https://github.com/guillemj/dpkg/commit/f3bb7d4939ae95cf44c89e8f599e7ed5da431e57)|[2015](https://fedoraproject.org/wiki/Changes/Harden_All_Packages)|[yes](https://wiki.gentoo.org/wiki/Hardened/Toolchain)|[2016](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|[2017](https://bugzilla.suse.com/show_bug.cgi?id=912298)|[2017](https://github.com/archlinux/svntogit-packages/commit/5936710c764016ce306f9cb975056e5b7605a65b)|[yes](https://man.openbsd.org/clang-local)|[yes](https://github.com/chimera-linux/cports/blob/master/Packaging.md#hardening_options)|[2012](https://source.android.com/docs/security/enhancements/enhancements41)|
|`-fcf-protection`/`-mcet`[^2]    |[no](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[yes](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2018](https://fedoraproject.org/wiki/Changes/HardeningFlags28)|[2021](https://bugs.gentoo.org/822036)|[2019](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|yes|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|[2023](https://github.com/openbsd/src/commit/bba006a81846d90e529167c689ea0d456b4599bc)|[no](https://github.com/chimera-linux/cports/blob/master/src/cbuild/core/profile.py)|no|
|`-fsanitize=bounds`              |no|no|no|no|no|no|no|no|no|[2019](https://source.android.com/docs/security/enhancements/enhancements10), partial|
|`-fsanitize=cfi`[^2]             |no|no|no|no|no|no|no|no|[partial](https://github.com/search?q=repo%3Achimera-linux%2Fcports+%22cfi%22&type=code)|[2018](https://source.android.com/docs/security/test/cfi), partial|
|`-fsanitize=safe-stack`[^2]      |no|no|no|no|no|no|no|no|[no](https://github.com/chimera-linux/cports/blob/master/Packaging.md#hardening_options)|?|
|`-fsanitize=shadow-call-stack`[^2] |no|no|no|no|no|no|no|no|no|[2019](https://security.googleblog.com/2019/05/queue-hardening-enhancements.html), partial|
|`-fsanitize=signed-integer-overflow`/`-ftrapv`|no|no|no|no|no|no|no|[no](https://man.openbsd.org/clang-local)|[yes](https://github.com/chimera-linux/cports/blob/master/Packaging.md#hardening_options)|[2018](https://android-developers.googleblog.com/2018/06/compiler-based-security-mitigations-in.html), partial|
|`-fsanitize=undefined`|no|no|no|no|no|no|no|?|no|?|
|`-fstack-clash-protection`       |[2023](https://gitlab.alpinelinux.org/alpine/abuild/-/commit/4f7a2aff7b87cec7dd2783f95b5d6f744244c6c7)|[yes](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[2018](https://fedoraproject.org/wiki/Changes/HardeningFlags28)|[2018](https://bugs.gentoo.org/675050)|[2019](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|[2018](https://en.opensuse.org/openSUSE:Security_Features)|[2021](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/f409a72342bf37017f190021970efaaeac1bb619)|?|[yes](https://github.com/chimera-linux/cports/blob/master/Packaging.md#hardening_options)|?|
|`-fstack-protector-strong`       |[yes](https://gitlab.alpinelinux.org/alpine/tsc/-/issues/64)|[yes](https://salsa.debian.org/toolchain-team/gcc/-/blob/master/debian/patches/gcc-distro-specs.diff)|[yes](https://src.fedoraproject.org/rpms/redhat-rpm-config//blob/rawhide/f/buildflags.md)|[yes](https://wiki.gentoo.org/wiki/Hardened/Toolchain)|[2014](https://wiki.ubuntu.com/ToolChain/CompilerFlags)|[2006](https://en.opensuse.org/openSUSE:Security_Features)|[2014](https://gitlab.archlinux.org/archlinux/packaging/packages/pacman/-/commit/2ae260d290234c5fc4e5a2bd792d2d1b9e54f227)|[yes](https://man.openbsd.org/clang-local)|[yes](https://github.com/chimera-linux/cports/blob/master/Packaging.md#hardening_options)|[2015](https://android.googlesource.com/platform/build/+/8765b1035f813be2c26988a73cf3e9815aa5adf6)|
|`-fstack-protector`              |superseded|superseded|superseded|superseded|superseded|superseded|superseded|superseded|superseded|[2009](https://source.android.com/docs/security/enhancements/enhancements41)|
|`-ftrivial-auto-var-init=zero`   |no|[no](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1010685)|no|[no](https://bugs.gentoo.org/913339)|[no](https://bugs.launchpad.net/ubuntu/+source/gcc-12/+bug/1972043)|no|no|?|[2023](https://github.com/chimera-linux/cports/commit/ad898a6b645b11dee989f4504e89577f5395ba24)|[2020](https://cs.android.com/android/_/android/platform/build/soong/+/59759dff24ffddca43a1940ed8615f96ee1e875f)|
|`-mbranch-protection=standard`/`-mbranch-target-enforce`|no|no|[2020](https://fedoraproject.org/wiki/Changes/Aarch64_PointerAuthentication)|no|no|no|no|[2023](https://github.com/openbsd/src/commit/990129f49dcc7205208dec5e29b252be8659896d)|[no](https://github.com/chimera-linux/cports/blob/master/src/cbuild/core/profile.py)|?|
|`-mshstk`                        |no|no|no|no|no|no|no|no|no|?|
|`-msign-return-address=[all/non-leaf]`|no|no|superseded|no|no|no|no|superseded|superseded|?|

Note that:
- some flags are incompatible between each other
- some flags are more useful than others
- some flags are superseding some others
- some libc are incompatible with some flags
- "partial" means "enabled in a lot of places, but not everywhere, with substantial caveats"


Sources:
- https://src.fedoraproject.org/rpms/redhat-rpm-config//blob/rawhide/f/buildflags.md
- https://en.opensuse.org/openSUSE:Security_Features
- https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628748.html
- https://wiki.gentoo.org/wiki/Hardened/Toolchain#Changes
- https://gitlab.archlinux.org/archlinux/rfcs/-/blob/master/rfcs/0003-buildflags.rst?ref_type=heads
- https://man.openbsd.org/clang-local
- https://sergesanspaille.fedorapeople.org/lpc2020.pdf
- https://wiki.ubuntu.com/Security/Features
- https://wiki.ubuntu.com/ToolChain/CompilerFlags
- https://fedoraproject.org/wiki/Security_Features_Matrix

[^1]: As `-D_LIBCPP_ENABLE_HARDENED_MODE` only works for llvm17, which isn't in Alpine yet.
[^2]: Not supported by [musl libc](https://musl.libc.org)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: RFC: Introduce -fhardened to enable security-related flags
  2023-09-19 14:19       ` Qing Zhao
@ 2023-09-21 16:53         ` Hans-Peter Nilsson
  0 siblings, 0 replies; 25+ messages in thread
From: Hans-Peter Nilsson @ 2023-09-21 16:53 UTC (permalink / raw)
  To: Qing Zhao; +Cc: sam, polacek, gcc-patches

> From: Qing Zhao <qing.zhao@oracle.com>
> Date: Tue, 19 Sep 2023 14:19:09 +0000
> > On Sep 17, 2023, at 12:36 PM, Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >> From: Sam James <sam@gentoo.org>
> >> Date: Sun, 17 Sep 2023 05:00:37 +0100
> >> Did some bug ever get filed for this to see if we can do a bit
> >> better here?
> > 
> > Not that I know of; neither for systemd nor gcc.
> 
> Then, is it convenient to file a bug on this?

A fair request, but I can't commit to analyze it myself to
the usual level, producing a self-contained test-case.

I see Sam James was super fast and has already added you to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111523
(thanks!)

> That will
> be very helpful for us to locate the issue and fix it.
> 
> Before I committing the -ftrivial-auto-var-init patch, I
> have done some performance testing on CPU2017 for x86 and
> aarch64,
> The runtime overhead was quite limited. 

Perhaps it would also make sense to performance-test on
network-facing software and system software such as systemd?

> Which platform the 35% performance slowdown was on?

arm-linux-eabi on ARM Cortex-A9. 

brgds, H-P

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2023-09-21 17:05 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-29 19:42 RFC: Introduce -fhardened to enable security-related flags Marek Polacek
2023-08-29 20:11 ` Sam James
2023-08-29 22:07   ` Marek Polacek
2023-08-30  8:46 ` Martin Uecker
2023-09-15 15:11   ` Marek Polacek
2023-09-16  7:40     ` Martin Uecker
2023-08-30  9:06 ` Xi Ruoyao
2023-09-15 15:12   ` Marek Polacek
2023-08-30 10:50 ` Jakub Jelinek
2023-08-30 13:08   ` Richard Biener
2023-09-15 15:21     ` Marek Polacek
2023-09-15 15:17   ` Marek Polacek
2023-09-01 22:09 ` Qing Zhao
2023-09-04 22:40   ` Richard Sandiford
2023-09-15 15:23     ` Marek Polacek
2023-09-15 15:40   ` Marek Polacek
2023-09-14 14:48 ` Hongtao Liu
2023-09-17  3:16 ` Hans-Peter Nilsson
2023-09-17  4:00   ` Sam James
2023-09-17 16:36     ` Hans-Peter Nilsson
2023-09-18  7:21       ` Sam James
2023-09-18 14:34         ` Hans-Peter Nilsson
2023-09-19 14:19       ` Qing Zhao
2023-09-21 16:53         ` Hans-Peter Nilsson
2023-09-20 11:33 ` jvoisin

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).