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

Reply via email to