Re: [PATCH] gcc: Disallow trampolines when -fhardened

[Iain Sandoe <idsandoe@googlemail.com>]

> On 2 Dec 2023, at 09:42, Martin Uecker <ma.uecker@gmail.com> wrote:
> 
> 
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>> 
>> -- >8 --
>> It came up that a good hardening strategy is to disable trampolines
>> which may require executable stack.  Therefore the following patch
>> adds -Werror=trampolines to -fhardened.
> 
> This would add a warning about specific code (where it is then
> unclear whether rewriting it is feasible or even an improvement),
> which seems different to all the other flags -fhardening has
> now.
> 
> GCC now has an option to allocate trampolines on the heap,
> which would seem to be a better fit.

Indeed, I was thinking of mentioning this.

>  On the other hand,
> it does not work with longjmp which may be a limitation.

I suspect that we can make this work using handlers and forced unwind,
but unfortunately do not have time to work on it at the moment.

Iain

> 
> Martin
> 
> 
>> 
>> gcc/ChangeLog:
>> 
>> 	* common.opt (Wtrampolines): Enable by -fhardened.
>> 	* doc/invoke.texi: Reflect that -fhardened enables -Werror=trampolines.
>> 	* opts.cc (print_help_hardened): Add -Werror=trampolines.
>> 	* toplev.cc (process_options): Enable -Werror=trampolines for
>> 	-fhardened.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.dg/fhardened-1.c: New test.
>> 	* gcc.dg/fhardened-2.c: New test.
>> 	* gcc.dg/fhardened-3.c: New test.
>> 	* gcc.dg/fhardened-4.c: New test.
>> 	* gcc.dg/fhardened-5.c: New test.
>> ---
>> gcc/common.opt                     |  2 +-
>> gcc/doc/invoke.texi                |  1 +
>> gcc/opts.cc                        |  1 +
>> gcc/testsuite/gcc.dg/fhardened-1.c | 27 +++++++++++++++++++++++++++
>> gcc/testsuite/gcc.dg/fhardened-2.c | 25 +++++++++++++++++++++++++
>> gcc/testsuite/gcc.dg/fhardened-3.c | 25 +++++++++++++++++++++++++
>> gcc/testsuite/gcc.dg/fhardened-4.c | 25 +++++++++++++++++++++++++
>> gcc/testsuite/gcc.dg/fhardened-5.c | 27 +++++++++++++++++++++++++++
>> gcc/toplev.cc                      |  8 +++++++-
>> 9 files changed, 139 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/fhardened-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/fhardened-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/fhardened-3.c
>> create mode 100644 gcc/testsuite/gcc.dg/fhardened-4.c
>> create mode 100644 gcc/testsuite/gcc.dg/fhardened-5.c
>> 
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index 161a035d736..9b09c7cb3df 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -807,7 +807,7 @@ Common Var(warn_system_headers) Warning
>> Do not suppress warnings from system headers.
>> 
>> Wtrampolines
>> -Common Var(warn_trampolines) Warning
>> +Common Var(warn_trampolines) Warning EnabledBy(fhardened)
>> Warn whenever a trampoline is generated.
>> 
>> Wtrivial-auto-var-init
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 2fab4c5d71f..c1664a1a0f1 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -17745,6 +17745,7 @@ may change between major releases of GCC, but are currently:
>> -fstack-protector-strong
>> -fstack-clash-protection
>> -fcf-protection=full @r{(x86 GNU/Linux only)}
>> +-Werror=trampolines
>> }
>> 
>> The list of options enabled by @option{-fhardened} can be generated using
>> diff --git a/gcc/opts.cc b/gcc/opts.cc
>> index 5d5efaf1b9e..aa062b87cef 100644
>> --- a/gcc/opts.cc
>> +++ b/gcc/opts.cc
>> @@ -2517,6 +2517,7 @@ print_help_hardened ()
>>   printf ("  %s\n", "-fstack-protector-strong");
>>   printf ("  %s\n", "-fstack-clash-protection");
>>   printf ("  %s\n", "-fcf-protection=full");
>> +  printf ("  %s\n", "-Werror=trampolines");
>>   putchar ('\n');
>> }
>> 
>> diff --git a/gcc/testsuite/gcc.dg/fhardened-1.c b/gcc/testsuite/gcc.dg/fhardened-1.c
>> new file mode 100644
>> index 00000000000..8710959b6f1
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/fhardened-1.c
>> @@ -0,0 +1,27 @@
>> +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
>> +/* { dg-require-effective-target trampolines } */
>> +/* { dg-options "-fhardened -O" } */
>> +
>> +static void
>> +baz (int (*bar) (void))
>> +{
>> +  bar ();
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a = 6;
>> +
>> +  int
>> +  bar (void)	// { dg-error "trampoline" }
>> +  {
>> +    return a;
>> +  }
>> +
>> +  baz (bar);
>> +
>> +  return 0;
>> +}
>> +
>> +/* { dg-prune-output "some warnings being treated as errors" } */
>> diff --git a/gcc/testsuite/gcc.dg/fhardened-2.c b/gcc/testsuite/gcc.dg/fhardened-2.c
>> new file mode 100644
>> index 00000000000..d47512aa47f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/fhardened-2.c
>> @@ -0,0 +1,25 @@
>> +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
>> +/* { dg-require-effective-target trampolines } */
>> +/* { dg-options "-fhardened -O -Wno-trampolines" } */
>> +
>> +static void
>> +baz (int (*bar) (void))
>> +{
>> +  bar ();
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a = 6;
>> +
>> +  int
>> +  bar (void)	// { dg-bogus "trampoline" }
>> +  {
>> +    return a;
>> +  }
>> +
>> +  baz (bar);
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/fhardened-3.c b/gcc/testsuite/gcc.dg/fhardened-3.c
>> new file mode 100644
>> index 00000000000..cebae13d8be
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/fhardened-3.c
>> @@ -0,0 +1,25 @@
>> +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
>> +/* { dg-require-effective-target trampolines } */
>> +/* { dg-options "-fhardened -O -Wno-error" } */
>> +
>> +static void
>> +baz (int (*bar) (void))
>> +{
>> +  bar ();
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a = 6;
>> +
>> +  int
>> +  bar (void)	// { dg-warning "trampoline" }
>> +  {
>> +    return a;
>> +  }
>> +
>> +  baz (bar);
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/fhardened-4.c b/gcc/testsuite/gcc.dg/fhardened-4.c
>> new file mode 100644
>> index 00000000000..7e62ed3385d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/fhardened-4.c
>> @@ -0,0 +1,25 @@
>> +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
>> +/* { dg-require-effective-target trampolines } */
>> +/* { dg-options "-fhardened -O -Wno-error=trampolines" } */
>> +
>> +static void
>> +baz (int (*bar) (void))
>> +{
>> +  bar ();
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a = 6;
>> +
>> +  int
>> +  bar (void)	// { dg-warning "trampoline" }
>> +  {
>> +    return a;
>> +  }
>> +
>> +  baz (bar);
>> +
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/fhardened-5.c b/gcc/testsuite/gcc.dg/fhardened-5.c
>> new file mode 100644
>> index 00000000000..5d3f0dcae8e
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/fhardened-5.c
>> @@ -0,0 +1,27 @@
>> +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */
>> +/* { dg-require-effective-target trampolines } */
>> +/* { dg-options "-fhardened -O -Wtrampolines" } */
>> +
>> +static void
>> +baz (int (*bar) (void))
>> +{
>> +  bar ();
>> +}
>> +
>> +int
>> +main (void)
>> +{
>> +  int a = 6;
>> +
>> +  int
>> +  bar (void)	// { dg-error "trampoline" }
>> +  {
>> +    return a;
>> +  }
>> +
>> +  baz (bar);
>> +
>> +  return 0;
>> +}
>> +
>> +/* { dg-prune-output "some warnings being treated as errors" } */
>> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
>> index 85450d97a1a..2f0ac74dee0 100644
>> --- a/gcc/toplev.cc
>> +++ b/gcc/toplev.cc
>> @@ -1682,7 +1682,7 @@ process_options ()
>>     flag_ipa_ra = 0;
>> 
>>   /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error
>> -     have not been set.  */
>> +     have not been set.  Also enable -Werror=trampolines for -fhardened.  */
>>   if (!OPTION_SET_P (warnings_are_errors))
>>     {
>>       if (warn_coverage_mismatch
>> @@ -1693,6 +1693,12 @@ process_options ()
>> 	  && option_unspecified_p (OPT_Wcoverage_invalid_line_number))
>> 	diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_invalid_line_number,
>> 					DK_ERROR, UNKNOWN_LOCATION);
>> +
>> +      if (flag_hardened
>> +	  && warn_trampolines
>> +	  && option_unspecified_p (OPT_Wtrampolines))
>> +	diagnostic_classify_diagnostic (global_dc, OPT_Wtrampolines,
>> +					DK_ERROR, UNKNOWN_LOCATION);
>>     }
>> 
>>   /* Save the current optimization options.  */
>> 
>> base-commit: b8edb812ff4934c609fdfafe2e1c7f932bc18305
>> -- 
>> 2.42.0
>> 
>