From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x130.google.com (mail-lf1-x130.google.com [IPv6:2a00:1450:4864:20::130]) by sourceware.org (Postfix) with ESMTPS id 9F06E3858C31 for ; Wed, 29 Nov 2023 12:48:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9F06E3858C31 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9F06E3858C31 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701262139; cv=none; b=jfsLN8SpYJs5qoC/MUAH1G01tlxjmirjGS5IRKFQkRut7wOmDWNWGdjru70ixe3+WSoFTTqmA/x6gzIrq98GvIgjvYCT4f4tesk8A72H6/bF066gJtJgNLSbkAOJ0MCgdujChz1YIKxN1+MXi/fpVUHbyww5wPoDLjPicw4rN8c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701262139; c=relaxed/simple; bh=MAfn+lBU/cc2Wc1+JtUbGMUKBbo6HmmzqZpyV3Gz6Vs=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=DNFG4PK447WvHyJb/aYTT6A6rgsZI5+p7/JyyD3R5fwOvXhUBCXCNJTQuw/FG9Pz9EFDU7+wejXqu/MBCvVq8c+IFSsE5JPM6jc7ZSCNTfky95W5/WlqilLNuViXm6Eit45rISzy4PcGlvUIy4Ql5RM3B60aZnVYgWpjRs0pVJY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x130.google.com with SMTP id 2adb3069b0e04-50abb83866bso9032448e87.3 for ; Wed, 29 Nov 2023 04:48:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701262135; x=1701866935; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=4VU9tR/kFRErO6Phin8w5eKo5xKUjazkDqu3tivklkE=; b=Krjv4T/SP2H3CUWaO6SN19+LpSsmjSf78b5TszSzlrXUewSmn7lQEM7vw0LFuWcl8K dSv5v+C6gHiZ49+f+12GxZjTk2VpTlJZVGz0gFdWsYHnYMszUCfE+mRgFHkFi1+XdZT9 Hr38Q/2BD82vFc8c/OuDwvojLE0KeWfUjhEKXQ5AhpF11c0GPcioUboYHViIstfjsGjM LtXP+cpHR5NbXU6zn9KSlRWcvXH/tbP/2P968bmAoG5BVUUGG2irSAkd0PCw2iIzt3if whrZR/e9PFqwPZ3+O3hjje34lpszECKI4J4NS/XJwBfL+dAn+ZKdNP5caWLTZoibNjsR ahew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701262135; x=1701866935; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4VU9tR/kFRErO6Phin8w5eKo5xKUjazkDqu3tivklkE=; b=ezfzZGXfocA8hkSDzUNEtvBzGa88ZJpQN9mci4KClIjn+hRxHBDT6qVozufYeiz3k4 8qgu+obJEyc8I2nJKdQIVjxuecD9swifvH0Zd56enp34neMOGLAtkBPbvnHbT+JhGUEU pua1taRmII/dXwANhDAxvqb141yyrmbSI14pl09o3PMWKiKoZk3C/W5H/hfWvJdq4rXh XL62OcuAB23l5cq1XoaK1xhAYWOikZy6YLzVMkMkIue221T+GILB8W7iuMbRQO3TN//2 7OBy6NAOoSn7XsSDrTsxVeE32vCziUpI5lUiDjtU6hBLxwuS9ZDqtM3uIQArSmIAlChO dWmQ== X-Gm-Message-State: AOJu0Yxzk9ywr+YEHGzyyuGF849uJLgHRm/e7vJr6Y/7YaMrMiLxXWJQ l79gQrhShKHk4I0JmcgJS1NZKaYEBgXoS3/DSrY= X-Google-Smtp-Source: AGHT+IEfozK+2QKTgTv6/yfFRziA6bf6EN7+orBssdkiKIm7js98rQveCY73OveTSIpxV+jDZq0fyhQvpPV2V8k03mI= X-Received: by 2002:a05:6512:3b9c:b0:50b:c699:b4f7 with SMTP id g28-20020a0565123b9c00b0050bc699b4f7mr1158754lfv.38.1701262134637; Wed, 29 Nov 2023 04:48:54 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Wed, 29 Nov 2023 13:48:41 +0100 Message-ID: Subject: Re: [PATCH v4] Introduce strub: machine-independent stack scrubbing To: Alexandre Oliva Cc: gcc-patches@gcc.gnu.org, Jeremy Bennett , Craig Blackmore , Graham Markall , Martin Jambor , Jan Hubicka , Jim Wilson , Jeff Law , Jakub Jelinek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_QUOTING autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, Nov 29, 2023 at 9:53=E2=80=AFAM Alexandre Oliva = wrote: > > On Nov 23, 2023, Richard Biener 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 aggregat= e > > 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=3Dstrict, 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 wh= en > >> > 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, ); > ... > } > > into > > wrapped_foo (va_list &wrap) > { > va_list ap; > > ... > va_copy (ap, wrap); > ... > } > > foo (...) > { > va_list wrap; > > va_start (wrap, ); > 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=3Drelaxed mode > is in effect (that's the default). In -fstrub=3Dstrict 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=3Dstrict 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 > Profiling and Test Coverage in Freestanding Environments. > > +
  • > + New options -fharden-compares > + and -fharden-conditional-branches to verify compares > + and conditional branches, to detect some power-deprivation > + hardware attacks, using reversed conditions. > +
    • > > > > 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.

    > of hardening flags. The options it enables can be displayed using t= he > --help=3Dhardened option. > > +
  • > + New option -fharden-control-flow-redundancy, 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. > +
    • > +
  • > + New type attribute hardbool, for C and Ada. Hardened > + booleans take user-specified representations for true > + and false, presumably with higher hamming distance > + than standard booleans, and get verified at every use, detecting > + memory corruption and some malicious attacks. > +
    • > +
  • > + New type attribute strub 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. > +
    • > +
  • > + New option -finline-stringops, to force inline > + expansion of memcmp, memcpy, > + memmove and memset, even when that is > + not an optimization, to avoid relying on library > + implementations. > +
    • > > >

    New Languages and Language specific improvements 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