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
