public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alexandre Oliva <oliva@adacore.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org,
	Jeremy Bennett <jeremy.bennett@embecosm.com>,
	Craig Blackmore <craig.blackmore@embecosm.com>,
	Graham Markall <graham.markall@embecosm.com>,
	Martin Jambor <mjambor@suse.cz>, Jan Hubicka <hubicka@ucw.cz>,
	Jim Wilson <wilson@tuliptree.org>,
	Jeff Law <jeffreyalaw@gmail.com>,
	Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH v4] Introduce strub: machine-independent stack scrubbing
Date: Wed, 29 Nov 2023 05:53:07 -0300	[thread overview]
Message-ID: <or1qc9lz4c.fsf@lxoliva.fsfla.org> (raw)
In-Reply-To: <CAFiYyc01uyz4eRVZ=4QpsQax38Jq0HzwSBYL8ihW2SrscGqhtQ@mail.gmail.com> (Richard Biener's message of "Thu, 23 Nov 2023 13:05:27 +0100")

On Nov 23, 2023, Richard Biener <richard.guenther@gmail.com> wrote:

> Conceptually it shouldn't be much different from what IPA-SRA does
> which is cloning a function but with different arguments, the function
> signature transform described in terms of ipa-param-manipulation bits.
> I've talked with Martin and at least there's currently no
> by-value-to-by-reference
> "transform", but IPA-SRA can pass two registers instead of one aggregate
> for example.  There's IPA_PARAM_OP_NEW already to add a new param.
> In principle the whole function rewriting (apart of recovering
> from inlining) should be doable within this framework.

I agree, but my attempts to do so have been unfruitful, and hit various
very obscure issues that made it unworkable.  Maybe someone smarter than
I am, or with more time than I had, can make the conversion.  The uses
of IPA_PARAM_OP_NEW for the cloning are there, and the more conservative
choices I made got it to work reliably, unlike the more adventurous
alternatives I tried.  One of the obscure issues I recall hitting was
this one.  I can probably still locate the early strub patch that hit
the described issue back then, if there's interest.
https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575044.html

>> The
>> wrapper already needs a custom ABI, because of the added watermark
>> pointer, and va_list and whatnot when needed, so a little further
>> customization to avoid a quite significant overhead seemed desirable.

> I understood the purpose of this, but I also saw it's only ever needed for
> non-strict operation

Erhm...  I suppose you're not talking about -fstrub=strict, because I
can't see how this relates with the by-reference argument passing from
wrapper to wrapped in internal strubbing.

> and I think that if you care about security you'd never
> want to operate in a way that you don't absolutely know the function
> you call isn't going to scrub your secrets ...

The ABI between wrapper and wrapped function splitting in internal strub
doesn't change this.

> But yes, I also wondered why you even run into re-gimplification issues.

Because &arg_#(D)[n_#] is good gimple, but &(*byref_arg_#(D))[n_#] isn't.

Maybe instead of going through regimplify, we could explicitly create an
SSA name for the indirection load, so that the purpose is made more
explicit.

> replacing the PARM_DECL there with a MEM_REF (new-PARM_DECL) should
> work without re-gimplification.

FWIW, I thought so as well ;-)

> I didn't remember seeing that you do sth like wrapping
> each "strubbed call" inside a try { call(); } finally { do-strub } to
> ensure this.
> Guess it was well hidden in the large patch ;)

Indeed, gsi_insert_finally_seq_after_call is where this is taken care of.

>> > As it's only for optimization, how much of the code would go away when
>> > you avoid changing the parameters of the wrapped function?
>> 
>> We can't avoid changing them entirely, and IIRC some of the
>> infrastructure for regimplification was also used for va_list and
>> apply_args handling, so it wouldn't save all that much.

> Ah, var-args ... indeed for simple forwarding it shouldn't be too bad.
> I wonder if this were a way to remove the restriction on function
> splitting of var-args functions - there's a recent bugreport about that
> (before any va_arg () has been called, of course).

If running va_start unconditionally in varargs functions is generally
acceptable, then the method I've used for wrapping varargs functions
should work generally, yeah.  The trick was to turn:

foo (...)
{
  va_list ap;

  ...
  va_start (&ap, <ignored>);
  ...
}

into

wrapped_foo (va_list &wrap)
{
  va_list ap;

  ...
  va_copy (ap, wrap);
  ...
}

foo (...)
{
  va_list wrap;

  va_start (wrap, <also ignored>);
  wrapped_foo (wrap);
  va_end (wrap);
}


Here's the opening comment I added to ipa-strub.cc:

/* This file introduces two passes that, together, implement
   machine-independent stack scrubbing, strub for short.  It arranges
   for stack frames that have strub enabled to be zeroed-out after
   relinquishing control to a caller, whether by returning or by
   propagating an exception.  This admittedly unusual design decision
   was driven by exception support (one needs a stack frame to be
   active to propagate exceptions out of it), and it enabled an
   implementation that is entirely machine-independent (no custom
   epilogue code is required).

   Strub modes can be selected for stack frames by attaching attribute
   strub to functions or to variables (to their types, actually).
   Different strub modes, with different implementation details, are
   available, and they can be selected by an argument to the strub
   attribute.  When enabled by strub-enabled variables, whether by
   accessing (as in reading from) statically-allocated ones, or by
   introducing (as in declaring) automatically-allocated ones, a
   suitable mode is selected automatically.

   At-calls mode modifies the interface of a function, adding a stack
   watermark argument, that callers use to clean up the stack frame of
   the called function.  Because of the interface change, it can only
   be used when explicitly selected, or when a function is internal to
   a translation unit.  Strub-at-calls function types are distinct
   from their original types (they're not modified in-place), and they
   are not interchangeable with other function types.

   Internal mode, in turn, does not modify the type or the interface
   of a function.  It is currently implemented by turning the function
   into a wrapper, moving the function body to a separate wrapped
   function, and scrubbing the wrapped body's stack in the wrapper.
   Internal-strub function types are mostly interface-compatible with
   other strub modes, namely callable (from strub functions, though
   not strub-enabled) and disabled (not callable from strub
   functions).

   Always_inline functions can be strub functions, but they can only
   be called from other strub functions, because strub functions must
   never be inlined into non-strub functions.  Internal and at-calls
   modes are indistinguishable when it comes to always_inline
   functions: they will necessarily be inlined into another strub
   function, and will thus be integrated into the caller's stack
   frame, whatever the mode.  (Contrast with non-always_inline strub
   functions: an at-calls function can be called from other strub
   functions, ensuring no discontinuity in stack erasing, whereas an
   internal-strub function can only be called from other strub
   functions if it happens to be inlined, or if -fstrub=relaxed mode
   is in effect (that's the default).  In -fstrub=strict mode,
   internal-strub functions are not callable from strub functions,
   because the wrapper itself is not strubbed.

   The implementation involves two simple-IPA passes.  The earliest
   one, strub-mode, assigns strub modes to functions.  It needs to run
   before any inlining, so that we can prevent inlining of strub
   functions into non-strub functions.  It notes explicit strub mode
   requests, enables strub in response to strub variables and testing
   options, and flags unsatisfiable requests.

   Three possibilities of unsatisfiable requests come to mind: (a)
   when a strub mode is explicitly selected, but the function uses
   features that make it ineligible for that mode (e.g. at-calls rules
   out calling __builtin_apply_args, because of the interface changes,
   and internal mode rules out noclone or otherwise non-versionable
   functions, non-default varargs, non-local or forced labels, and
   functions with far too many arguments); (b) when some strub mode
   must be enabled because of a strub variable, but the function is
   not eligible or not viable for any mode; and (c) when
   -fstrub=strict is enabled, and calls are found in strub functions
   to functions that are not callable from strub contexts.
   compute_strub_mode implements (a) and (b), and verify_strub
   implements (c).

   The second IPA pass modifies interfaces of at-calls-strub functions
   and types, introduces strub calls in and around them. and splits
   internal-strub functions.  It is placed after early inlining, so
   that even internal-strub functions get a chance of being inlined
   into other strub functions, but before non-early inlining, so that
   internal-strub wrapper functions still get a chance of inlining
   after splitting.

   Wrappers avoid duplicating the copying of large arguments again by
   passing them by reference to the wrapped bodies.  This involves
   occasional SSA rewriting of address computations, because of the
   additional indirection.  Besides these changes, and the
   introduction of the stack watermark parameter, wrappers and wrapped
   functions cooperate to handle variable argument lists (performing
   va_start in the wrapper, passing the list as an argument, and
   replacing va_start calls in the wrapped body with va_copy), and
   __builtin_apply_args (also called in the wrapper and passed to the
   wrapped body as an argument).

   Strub bodies (both internal-mode wrapped bodies, and at-calls
   functions) always start by adjusting the watermark parameter, by
   calling __builtin___strub_update.  The compiler inserts them in the
   main strub pass.  Allocations of additional stack space for the
   frame (__builtin_alloca) are also followed by watermark updates.
   Stack space temporarily allocated to pass arguments to other
   functions, released right after the call, is not regarded as part
   of the frame.  Around calls to them, i.e., in internal-mode
   wrappers and at-calls callers (even calls through pointers), calls
   to __builtin___strub_enter and __builtin___strub_leave are
   inserted, the latter as a __finally block, so that it runs at
   regular and exceptional exit paths.  strub_enter only initializes
   the stack watermark, and strub_leave is where the scrubbing takes
   place, overwriting with zeros the stack space from the top of the
   stack to the watermark.

   These calls can be optimized in various cases.  In
   pass_ipa_strub::adjust_at_calls_call, for example, we enable
   tail-calling and other optimized calls from one strub body to
   another by passing on the watermark parameter.  The builtins
   themselves may undergo inline substitution during expansion,
   dependign on optimization levels.  This involves dealing with stack
   red zones (when the builtins are called out-of-line, the red zone
   cannot be used) and other ugly details related with inlining strub
   bodies into other strub bodies (see expand_builtin_strub_update).
   expand_builtin_strub_leave may even perform partial inline
   substitution.  */

The type attribute description now starts like this:

@cindex @code{strub} type attribute
@item strub
This attribute defines stack-scrubbing properties of functions and
variables, so that functions that access sensitive data can have their
stack frames zeroed-out upon returning or propagating exceptions.  This
may be enabled explicitly, by selecting certain @code{strub} modes for
specific functions, or implicitly, by means of @code{strub} variables.

And I've fixed the symbol versioning for strub and hardcfr functions.

I've just pushed the base strub patch and the recent incremental changes
(and some commits used for testing) to refs/users/aoliva/heads/strub.


Here are changes.html entries for this and for the other newly-added
features:

new AdaCore-contributed hardening features in gcc 13 and 14

Mention hardening of conditionals (added in gcc 13), control flow
redundancy, hardened booleans, and stack scrubbing.

Also cover forced inlining of string operations while at that.
---
 htdocs/gcc-13/changes.html |    6 ++++++
 htdocs/gcc-14/changes.html |   29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/htdocs/gcc-13/changes.html b/htdocs/gcc-13/changes.html
index 8ef3d63952daf..c1dea18caf59b 100644
--- a/htdocs/gcc-13/changes.html
+++ b/htdocs/gcc-13/changes.html
@@ -168,6 +168,12 @@ You may also want to check out our
     been added, see also
     <a href="https://gcc.gnu.org/onlinedocs/gcc/Freestanding-Environments.html">Profiling and Test Coverage in Freestanding Environments</a>.
   </li>
+  <li>
+    New options <code>-fharden-compares</code>
+    and <code>-fharden-conditional-branches</code> to verify compares
+    and conditional branches, to detect some power-deprivation
+    hardware attacks, using reversed conditions.
+  </li>
 </ul>
 
 
diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
index 2088ee91a34e1..cb92f8d8095c3 100644
--- a/htdocs/gcc-14/changes.html
+++ b/htdocs/gcc-14/changes.html
@@ -109,6 +109,35 @@ a work-in-progress.</p>
     of hardening flags.  The options it enables can be displayed using the
     <code>--help=hardened</code> option.
   </li>
+  <li>
+    New option <code>-fharden-control-flow-redundancy</code>, to
+    verify, at the end of functions, that the visited basic blocks
+    correspond to a legitimate execution path, so as to detect and
+    prevent attacks that transfer control into the middle of
+    functions.
+  </li>
+  <li>
+    New type attribute <code>hardbool</code>, for C and Ada.  Hardened
+    booleans take user-specified representations for <code>true</code>
+    and <code>false</code>, presumably with higher hamming distance
+    than standard booleans, and get verified at every use, detecting
+    memory corruption and some malicious attacks.
+  </li>
+  <li>
+    New type attribute <code>strub</code> to control stack scrubbing
+    properties of functions and variables.  The stack frame used by
+    functions marked with the attribute gets zeroed-out upon returning
+    or exception escaping.  Scalar variables marked with the attribute
+    cause functions contaning or accessing them to get stack scrubbing
+    enabled implicitly.
+  </li>
+  <li>
+    New option <code>-finline-stringops</code>, to force inline
+    expansion of <code>memcmp</code>, <code>memcpy</code>,
+    <code>memmove</code> and <code>memset</code>, even when that is
+    not an optimization, to avoid relying on library
+    implementations.
+  </li>
 </ul>
 <!-- .................................................................. -->
 <h2 id="languages">New Languages and Language specific improvements</h2>


-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

  reply	other threads:[~2023-11-29  8:53 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <ormtqpsbuc.fsf@lxoliva.fsfla.org>
2021-09-09  7:11 ` [PATCH] " Alexandre Oliva
2022-07-29  6:16   ` [PATCH v2 00/10] Introduce " Alexandre Oliva
2022-07-29  6:24     ` [PATCH v2 01/10] Introduce strub: documentation, and new command-line options Alexandre Oliva
2022-07-29  6:25     ` [PATCH v2 02/10] Introduce strub: torture tests for C and C++ Alexandre Oliva
2022-08-09 13:34       ` Alexandre Oliva
2022-07-29  6:25     ` [PATCH v2 03/10] Introduce strub: non-torture " Alexandre Oliva
2022-07-29  6:26     ` [PATCH v2 04/10] Introduce strub: tests for C++ and Ada Alexandre Oliva
2022-07-29  6:26     ` [PATCH v2 05/10] Introduce strub: builtins and runtime Alexandre Oliva
2022-07-29  6:27     ` [PATCH v2 06/10] Introduce strub: attributes Alexandre Oliva
2022-07-29  6:28     ` [PATCH v2 07/10] Introduce strub: infrastructure interfaces and adjustments Alexandre Oliva
2022-07-29  6:28     ` [PATCH v2 08/10] Introduce strub: strub modes Alexandre Oliva
2022-07-29  6:30     ` [PATCH v2 09/10] Introduce strub: strubm (mode assignment) pass Alexandre Oliva
2022-07-29  6:34     ` [PATCH v2 10/10] Introduce strub: strub pass Alexandre Oliva
2022-07-29  6:36     ` [PATCH v2 00/10] Introduce strub: machine-independent stack scrubbing Alexandre Oliva
2022-10-10  8:48       ` Richard Biener
2022-10-11 11:57         ` Alexandre Oliva
2022-10-11 11:59           ` Richard Biener
2022-10-11 13:33             ` Alexandre Oliva
2022-10-13 11:38               ` Richard Biener
2022-10-13 13:15                 ` Alexandre Oliva
2023-06-16  6:09     ` [PATCH v3] " Alexandre Oliva
2023-06-27 21:28       ` Qing Zhao
2023-06-28  8:20         ` Alexandre Oliva
2023-10-20  6:03       ` [PATCH v4] " Alexandre Oliva
2023-10-26  6:15         ` Alexandre Oliva
2023-11-20 12:40           ` Alexandre Oliva
2023-11-22 14:14             ` Richard Biener
2023-11-23 10:56               ` Alexandre Oliva
2023-11-23 12:05                 ` Richard Biener
2023-11-29  8:53                   ` Alexandre Oliva [this message]
2023-11-29 12:48                     ` Richard Biener
2023-11-30  4:13                       ` Alexandre Oliva
2023-11-30 12:00                         ` Richard Biener
2023-12-02 17:56                           ` [PATCH v5] " Alexandre Oliva
2023-12-05  6:25                             ` Alexandre Oliva
2023-12-06  1:04                               ` Alexandre Oliva
2023-12-05  9:01                             ` Richard Biener
2023-12-06  8:36                             ` Causes to nvptx bootstrap fail: " Tobias Burnus
2023-12-06 11:32                               ` Thomas Schwinge
2023-12-06 22:12                                 ` Alexandre Oliva
2023-12-07  3:33                                   ` [PATCH] strub: enable conditional support Alexandre Oliva
2023-12-07  7:24                                     ` Richard Biener
2023-12-07 16:44                                     ` Thomas Schwinge
2023-12-07 17:52                                       ` [PATCH] Alexandre Oliva
2023-12-08  6:46                                         ` [PATCH] Richard Biener
2023-12-08  9:33                                         ` [PATCH] strub: skip emutls after strubm errors Thomas Schwinge
2023-12-10  9:16                                           ` FX Coudert
2023-12-07  7:21                                   ` Causes to nvptx bootstrap fail: [PATCH v5] Introduce strub: machine-independent stack scrubbing Richard Biener
2023-12-06 10:22                             ` Jan Hubicka
2023-12-07 21:19                               ` Alexandre Oliva
2023-12-07 21:39                               ` Alexandre Oliva
2023-12-09  2:08                                 ` [PATCH] strub: add note on attribute access Alexandre Oliva
2023-12-11  7:26                                   ` Richard Biener
2023-12-12 14:21                                   ` Jan Hubicka
2023-12-11  8:40                             ` [PATCH] testsuite: Disable -fstack-protector* for some strub tests Jakub Jelinek
2023-12-11  8:59                               ` Richard Biener
2023-12-20  8:15                           ` [PATCH FYI] www: new AdaCore-contributed hardening features in gcc 13 and 14 Alexandre Oliva
2023-11-30  5:04                       ` [PATCH v4] Introduce strub: machine-independent stack scrubbing Alexandre Oliva
2023-11-30 11:56                         ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=or1qc9lz4c.fsf@lxoliva.fsfla.org \
    --to=oliva@adacore.com \
    --cc=craig.blackmore@embecosm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=graham.markall@embecosm.com \
    --cc=hubicka@ucw.cz \
    --cc=jakub@redhat.com \
    --cc=jeffreyalaw@gmail.com \
    --cc=jeremy.bennett@embecosm.com \
    --cc=mjambor@suse.cz \
    --cc=richard.guenther@gmail.com \
    --cc=wilson@tuliptree.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).