On Wed, Nov 29, 2023 at 9:53 AM Alexandre Oliva <ol...@adacore.com> wrote:
>
> On Nov 23, 2023, Richard Biener <richard.guent...@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.
'arg_#(D)' looks like a SSA name, and no, taking the address doesn't work,
so I assume it was &MEM[arg_(D)][n_#] which is indeed OK. But you
shouldn't need to change a pointer argument to be passed by reference,
do you? As said, you want to restrict by-reference passing to arguments
that are !is_gimple_reg_type (). Everywhere where a plain PARM_DECL
was valid a *ptr indirection is as well.
> 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);
> }
Ah, nice trick indeed. I think that should work, it might not
be optimal (I don't think we will optimize the va_copy).
>
> 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. */
Thanks.
> 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>
LGTM.
Can you check on the pass-by-reference thing again please?
Let's see if Honza or Martin have any comments on the IPA bits, I just
mentioned what I think should be doable ...
Thanks,
Richard.
>
> --
> 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
