Message ID | oro87yho3n.fsf@lxoliva.fsfla.org |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 15B1F3857C52 for <patchwork@sourceware.org>; Sat, 9 Oct 2021 03:30:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 15B1F3857C52 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1633750246; bh=CZyi3HJMq0zcQG4JtqZdamqtKQYap8XN3NnLMmxHC/Q=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=cqNZ7eYXYg+Y4axyDfUsrXWJtERUMK7Obmul1DddwlVyJq8zJCSXAEPlVIB5U89gP NPG/Isivk8Mdgfk72cWh3tYaWx6QcL4xfW6x7Nrw5depZr5vrubHkf0CEUMArhfykR QJQp5/DCskzctJQ3PmvINKB930cew94A5Bi3k660= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTPS id 90B403858412 for <gcc-patches@gcc.gnu.org>; Sat, 9 Oct 2021 03:30:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 90B403858412 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 3D869117079; Fri, 8 Oct 2021 23:30:14 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ai8QdQnUglsX; Fri, 8 Oct 2021 23:30:14 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id CDE7211705B; Fri, 8 Oct 2021 23:30:13 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 1993U4kK225782 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 9 Oct 2021 00:30:04 -0300 To: gcc-patches@gcc.gnu.org Subject: [PATCH] hardened conditionals Organization: Free thinker, does not speak for AdaCore Date: Sat, 09 Oct 2021 00:30:04 -0300 Message-ID: <oro87yho3n.fsf@lxoliva.fsfla.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Alexandre Oliva <oliva@adacore.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
hardened conditionals
|
|
Commit Message
Alexandre Oliva
Oct. 9, 2021, 3:30 a.m. UTC
This patch introduces optional passes to harden conditionals used in branches, and in computing boolean expressions, by adding redundant tests of the reversed conditions, and trapping in case of unexpected results. Though in abstract machines the redundant tests should never fail, CPUs may be led to misbehave under certain kinds of attacks, such as of power deprivation, and these tests reduce the likelihood of going too far down an unexpected execution path. This patch was regstrapped on x86_64-linux-gnu. It was also bootstrapped along with an extra common.opt that enables both passes unconditionally. Ok to install? for gcc/ChangeLog * common.opt (fharden-compares): New. (fharden-conditional-branches): New. * doc/invoke.texi: Document new options. * gimple-harden-conditionals.cc: New. * passes.def: Add new passes. * tree-pass.h (make_pass_harden_compares): Declare. (make_pass_harden_conditional_branches): Declare. for gcc/ada/ChangeLog * doc/gnat_rm/security_hardening_features.rst (Hardened Conditionals): New. for gcc/testsuite/ChangeLog * c-c++-common/torture/harden-comp.c: New. * c-c++-common/torture/harden-cond.c: New. --- gcc/Makefile.in | 1 .../doc/gnat_rm/security_hardening_features.rst | 40 ++ gcc/common.opt | 8 gcc/doc/invoke.texi | 19 + gcc/gimple-harden-conditionals.cc | 379 ++++++++++++++++++++ gcc/passes.def | 2 gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + gcc/tree-pass.h | 3 9 files changed, 484 insertions(+) create mode 100644 gcc/gimple-harden-conditionals.cc create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c
Comments
On October 9, 2021 5:30:04 AM GMT+02:00, Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > >This patch introduces optional passes to harden conditionals used in >branches, and in computing boolean expressions, by adding redundant >tests of the reversed conditions, and trapping in case of unexpected >results. Though in abstract machines the redundant tests should never >fail, CPUs may be led to misbehave under certain kinds of attacks, >such as of power deprivation, and these tests reduce the likelihood of >going too far down an unexpected execution path. > >This patch was regstrapped on x86_64-linux-gnu. It was also >bootstrapped along with an extra common.opt that enables both passes >unconditionally. Ok to install? Why two passes (and two IL traverses?) How do you prevent RTL optimizers (jump threading) from removing the redundant tests? I'd have expected such hardening to occur very late in the RTL pipeline. Richard. > >for gcc/ChangeLog > > * common.opt (fharden-compares): New. > (fharden-conditional-branches): New. > * doc/invoke.texi: Document new options. > * gimple-harden-conditionals.cc: New. > * passes.def: Add new passes. > * tree-pass.h (make_pass_harden_compares): Declare. > (make_pass_harden_conditional_branches): Declare. > >for gcc/ada/ChangeLog > > * doc/gnat_rm/security_hardening_features.rst > (Hardened Conditionals): New. > >for gcc/testsuite/ChangeLog > > * c-c++-common/torture/harden-comp.c: New. > * c-c++-common/torture/harden-cond.c: New. >--- > gcc/Makefile.in | 1 > .../doc/gnat_rm/security_hardening_features.rst | 40 ++ > gcc/common.opt | 8 > gcc/doc/invoke.texi | 19 + > gcc/gimple-harden-conditionals.cc | 379 ++++++++++++++++++++ > gcc/passes.def | 2 > gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + > gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + > gcc/tree-pass.h | 3 > 9 files changed, 484 insertions(+) > create mode 100644 gcc/gimple-harden-conditionals.cc > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c > >diff --git a/gcc/Makefile.in b/gcc/Makefile.in >index 64252697573a7..7209ed117d09d 100644 >--- a/gcc/Makefile.in >+++ b/gcc/Makefile.in >@@ -1389,6 +1389,7 @@ OBJS = \ > gimple-if-to-switch.o \ > gimple-iterator.o \ > gimple-fold.o \ >+ gimple-harden-conditionals.o \ > gimple-laddress.o \ > gimple-loop-interchange.o \ > gimple-loop-jam.o \ >diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst >index 1c46e3a4c7b88..52240d7e3dd54 100644 >--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst >+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst >@@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, it is not > currently recommended to rely on any effects this pragma might be > expected to have when calling subprograms through access-to-subprogram > variables. >+ >+ >+.. Hardened Conditionals: >+ >+Hardened Conditionals >+===================== >+ >+GNAT can harden conditionals to protect against control flow attacks. >+ >+This is accomplished by two complementary transformations, each >+activated by a separate command-line option. >+ >+The option *-fharden-compares* enables hardening of compares that >+compute results stored in variables, adding verification that the >+reversed compare yields the opposite result. >+ >+The option *-fharden-conditional-branches* enables hardening of >+compares that guard conditional branches, adding verification of the >+reversed compare to both execution paths. >+ >+These transformations are introduced late in the compilation pipeline, >+long after boolean expressions are decomposed into separate compares, >+each one turned into either a conditional branch or a compare whose >+result is stored in a boolean variable or temporary. Compiler >+optimizations, if enabled, may also turn conditional branches into >+stored compares, and vice-versa. Conditionals may also be optimized >+out entirely, if their value can be determined at compile time, and >+occasionally multiple compares can be combined into one. >+ >+It is thus difficult to predict which of these two options will affect >+a specific compare operation expressed in source code. Using both >+options ensures that every compare that is not optimized out will be >+hardened. >+ >+The addition of reversed compares can be observed by enabling the dump >+files of the corresponding passes, through command line options >+*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. >+ >+They are separate options, however, because of the significantly >+different performance impact of the hardening transformations. >diff --git a/gcc/common.opt b/gcc/common.opt >index e867055fc000d..89f2e6da6e56e 100644 >--- a/gcc/common.opt >+++ b/gcc/common.opt >@@ -1719,6 +1719,14 @@ fguess-branch-probability > Common Var(flag_guess_branch_prob) Optimization > Enable guessing of branch probabilities. > >+fharden-compares >+Common Var(flag_harden_compares) Optimization >+Harden conditionals not used in branches, checking reversed conditions. >+ >+fharden-conditional-branches >+Common Var(flag_harden_conditional_branches) Optimization >+Harden conditional branches by checking reversed conditions. >+ > ; Nonzero means ignore `#ident' directives. 0 means handle them. > ; Generate position-independent code for executables if possible > ; On SVR4 targets, it also controls whether or not to emit a >diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >index 281dd10fff96c..5ed6a55f729e1 100644 >--- a/gcc/doc/invoke.texi >+++ b/gcc/doc/invoke.texi >@@ -595,6 +595,7 @@ Objective-C and Objective-C++ Dialects}. > -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol > -fsanitize-undefined-trap-on-error -fbounds-check @gol > -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol >+-fharden-compares -fharden-conditional-branches @gol > -fstack-protector -fstack-protector-all -fstack-protector-strong @gol > -fstack-protector-explicit -fstack-check @gol > -fstack-limit-register=@var{reg} -fstack-limit-symbol=@var{sym} @gol >@@ -15491,6 +15492,24 @@ which functions and calls should be skipped from instrumentation > Currently the x86 GNU/Linux target provides an implementation based > on Intel Control-flow Enforcement Technology (CET). > >+@item -fharden-compares >+@opindex fharden-compares >+For every logical test that survives gimple optimizations and is >+@emph{not} the condition in a conditional branch (for example, >+conditions tested for conditional moves, or to store in boolean >+variables), emit extra code to compute and verify the reversed >+condition, and to call @code{__builtin_trap} if the results do not >+match. Use with @samp{-fharden-conditional-branches} to cover all >+conditionals. >+ >+@item -fharden-conditional-branches >+@opindex fharden-conditional-branches >+For every non-vectorized conditional branch that survives gimple >+optimizations, emit extra code to compute and verify the reversed >+condition, and to call @code{__builtin_trap} if the result is >+unexpected. Use with @samp{-fharden-compares} to cover all >+conditionals. >+ > @item -fstack-protector > @opindex fstack-protector > Emit extra code to check for buffer overflows, such as stack smashing >diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc >new file mode 100644 >index 0000000000000..08187b21ebddf >--- /dev/null >+++ b/gcc/gimple-harden-conditionals.cc >@@ -0,0 +1,379 @@ >+/* Harden conditionals. >+ Copyright (C) 2021 Free Software Foundation, Inc. >+ Contributed by Alexandre Oliva <oliva@adacore.com>. >+ >+This file is part of GCC. >+ >+GCC is free software; you can redistribute it and/or modify it under >+the terms of the GNU General Public License as published by the Free >+Software Foundation; either version 3, or (at your option) any later >+version. >+ >+GCC is distributed in the hope that it will be useful, but WITHOUT ANY >+WARRANTY; without even the implied warranty of MERCHANTABILITY or >+FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License >+for more details. >+ >+You should have received a copy of the GNU General Public License >+along with GCC; see the file COPYING3. If not see >+<http://www.gnu.org/licenses/>. */ >+ >+#include "config.h" >+#include "system.h" >+#include "coretypes.h" >+#include "backend.h" >+#include "tree.h" >+#include "fold-const.h" >+#include "gimple.h" >+#include "gimplify.h" >+#include "tree-pass.h" >+#include "ssa.h" >+#include "gimple-iterator.h" >+#include "tree-cfg.h" >+#include "basic-block.h" >+#include "cfghooks.h" >+#include "cfgloop.h" >+#include "diagnostic.h" >+#include "intl.h" >+ >+namespace { >+ >+/* This pass introduces redundant, but reversed conditionals at every >+ compare, be it used for conditional branches, other conditional >+ operations, or for boolean computation. This doesn't make sense >+ for abstract CPUs, but this kind of hardening may avoid undesirable >+ execution paths on CPUs under such attacks as of power >+ deprivation. */ >+ >+/* Define a pass to harden conditionals other than branches. */ >+const pass_data pass_data_harden_compares = { >+ GIMPLE_PASS, >+ "hardcmp", >+ OPTGROUP_NONE, >+ TV_NONE, >+ PROP_cfg | PROP_ssa, // properties_required >+ 0, // properties_provided >+ 0, // properties_destroyed >+ 0, // properties_start >+ TODO_update_ssa >+ | TODO_cleanup_cfg >+ | TODO_verify_il, // properties_finish >+}; >+ >+class pass_harden_compares : public gimple_opt_pass >+{ >+public: >+ pass_harden_compares (gcc::context *ctxt) >+ : gimple_opt_pass (pass_data_harden_compares, ctxt) >+ {} >+ opt_pass *clone () { return new pass_harden_compares (m_ctxt); } >+ virtual bool gate (function *) { >+ return flag_harden_compares; >+ } >+ virtual unsigned int execute (function *); >+}; >+ >+/* This pass introduces redundant, but reversed conditionals at every >+ conditional branch. This doesn't make sense for abstract CPUs, but >+ this kind of hardening may avoid undesirable execution paths on >+ CPUs under such attacks as of power deprivation. This pass must >+ run after the above, otherwise it will re-harden the checks >+ introduced by the above. */ >+ >+/* Define a pass to harden conditionals in branches. */ >+const pass_data pass_data_harden_conditional_branches = { >+ GIMPLE_PASS, >+ "hardcbr", >+ OPTGROUP_NONE, >+ TV_NONE, >+ PROP_cfg | PROP_ssa, // properties_required >+ 0, // properties_provided >+ 0, // properties_destroyed >+ 0, // properties_start >+ TODO_update_ssa >+ | TODO_cleanup_cfg >+ | TODO_verify_il, // properties_finish >+}; >+ >+class pass_harden_conditional_branches : public gimple_opt_pass >+{ >+public: >+ pass_harden_conditional_branches (gcc::context *ctxt) >+ : gimple_opt_pass (pass_data_harden_conditional_branches, ctxt) >+ {} >+ opt_pass *clone () { return new pass_harden_conditional_branches (m_ctxt); } >+ virtual bool gate (function *) { >+ return flag_harden_conditional_branches; >+ } >+ virtual unsigned int execute (function *); >+}; >+ >+} >+ >+static inline tree >+detach_value (gimple_stmt_iterator *gsip, tree val) >+{ >+ if (TREE_CONSTANT (val) || TREE_CODE (val) != SSA_NAME) >+ { >+ gcc_checking_assert (is_gimple_min_invariant (val)); >+ return val; >+ } >+ >+ tree ret = copy_ssa_name (val); >+ >+ vec<tree, va_gc> *inputs = NULL; >+ vec<tree, va_gc> *outputs = NULL; >+ vec_safe_push (outputs, >+ build_tree_list >+ (build_tree_list >+ (NULL_TREE, build_string (2, "=g")), >+ ret)); >+ vec_safe_push (inputs, >+ build_tree_list >+ (build_tree_list >+ (NULL_TREE, build_string (1, "0")), >+ val)); >+ gasm *detach = gimple_build_asm_vec ("", inputs, outputs, >+ NULL, NULL); >+ gsi_insert_before (gsip, detach, GSI_SAME_STMT); >+ >+ SSA_NAME_DEF_STMT (ret) = detach; >+ >+ return ret; >+} >+ >+static inline void >+insert_check_and_trap (gimple_stmt_iterator *gsip, int flags, >+ enum tree_code cop, tree lhs, tree rhs) >+{ >+ basic_block chk = gsi_bb (*gsip); >+ >+ gcond *cond = gimple_build_cond (cop, lhs, rhs, NULL, NULL); >+ gsi_insert_before (gsip, cond, GSI_SAME_STMT); >+ >+ basic_block trp = create_empty_bb (chk); >+ >+ gimple_stmt_iterator gsit = gsi_after_labels (trp); >+ gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); >+ gsi_insert_before (&gsit, trap, GSI_SAME_STMT); >+ >+ if (dump_file) >+ fprintf (dump_file, >+ "Adding reversed compare to block %i, and trap to block %i\n", >+ chk->index, trp->index); >+ >+ if (BB_PARTITION (chk)) >+ BB_SET_PARTITION (trp, BB_COLD_PARTITION); >+ >+ int true_false_flag = flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >+ gcc_assert (true_false_flag); >+ int neg_true_false_flag = (~flags) & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); >+ >+ single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU; >+ single_succ_edge (chk)->flags |= neg_true_false_flag; >+ make_edge (chk, trp, true_false_flag); >+ >+ if (dom_info_available_p (CDI_DOMINATORS)) >+ set_immediate_dominator (CDI_DOMINATORS, trp, chk); >+ if (current_loops) >+ add_bb_to_loop (trp, current_loops->tree_root); >+} >+ >+static inline void >+insert_edge_check_and_trap (edge e, enum tree_code cop, tree lhs, tree rhs) >+{ >+ int flags = e->flags; >+ basic_block src = e->src; >+ basic_block dest = e->dest; >+ >+ basic_block chk = split_edge (e); >+ e = NULL; >+ >+ if (dump_file) >+ fprintf (dump_file, >+ "Splitting edge %i->%i into block %i\n", >+ src->index, dest->index, chk->index); >+ >+ gimple_stmt_iterator gsik = gsi_after_labels (chk); >+ >+ bool same_p = (lhs == rhs); >+ lhs = detach_value (&gsik, lhs); >+ rhs = same_p ? lhs : detach_value (&gsik, rhs); >+ >+ insert_check_and_trap (&gsik, flags, cop, lhs, rhs); >+} >+ >+unsigned int >+pass_harden_conditional_branches::execute (function *fun) >+{ >+ basic_block bb; >+ FOR_EACH_BB_REVERSE_FN (bb, fun) >+ { >+ gimple_stmt_iterator gsi = gsi_last_bb (bb); >+ >+ if (gsi_end_p (gsi)) >+ continue; >+ >+ gcond *cond = dyn_cast <gcond *> (gsi_stmt (gsi)); >+ if (!cond) >+ continue; >+ >+ /* Turn: >+ >+ if (x op y) goto l1; else goto l2; >+ >+ into: >+ >+ if (x op y) goto l1'; else goto l2'; >+ l1': if (x' cop y') goto l1'trap; else goto l1; >+ l1'trap: __builtin_trap (); >+ l2': if (x' cop y') goto l2; else goto l2'trap; >+ l2'trap: __builtin_trap (); >+ >+ where cop is a complementary boolean operation to op; l1', l1'trap, >+ l2' and l2'trap are newly-created labels; and x' and y' hold the same >+ value as x and y, but in a way that does not enable the compiler to >+ optimize the redundant compare away. >+ */ >+ >+ enum tree_code op = gimple_cond_code (cond); >+ tree lhs = gimple_cond_lhs (cond); >+ tree rhs = gimple_cond_rhs (cond); >+ >+ enum tree_code cop = invert_tree_comparison (op, HONOR_NANS (lhs)); >+ >+ if (cop == ERROR_MARK) >+ /* ??? Can we do better? */ >+ continue; >+ >+ insert_edge_check_and_trap (EDGE_SUCC (bb, 0), cop, lhs, rhs); >+ insert_edge_check_and_trap (EDGE_SUCC (bb, 1), cop, lhs, rhs); >+ } >+ >+ return 0; >+} >+ >+gimple_opt_pass * >+make_pass_harden_conditional_branches (gcc::context *ctxt) >+{ >+ return new pass_harden_conditional_branches (ctxt); >+} >+ >+unsigned int >+pass_harden_compares::execute (function *fun) >+{ >+ basic_block bb; >+ FOR_EACH_BB_REVERSE_FN (bb, fun) >+ for (gimple_stmt_iterator gsi = gsi_last_bb (bb); >+ !gsi_end_p (gsi); gsi_prev (&gsi)) >+ { >+ gassign *asgn = dyn_cast <gassign *> (gsi_stmt (gsi)); >+ if (!asgn) >+ continue; >+ >+ /* Turn: >+ >+ z = x op y; >+ >+ into: >+ >+ z = x op y; >+ z' = x' cop y'; >+ if (z == z') __builtin_trap (); >+ >+ where cop is a complementary boolean operation to op; and x' >+ and y' hold the same value as x and y, but in a way that does >+ not enable the compiler to optimize the redundant compare >+ away. >+ */ >+ >+ enum tree_code op = gimple_assign_rhs_code (asgn); >+ >+ enum tree_code cop; >+ >+ switch (op) >+ { >+ case EQ_EXPR: >+ case NE_EXPR: >+ case GT_EXPR: >+ case GE_EXPR: >+ case LT_EXPR: >+ case LE_EXPR: >+ case LTGT_EXPR: >+ case UNEQ_EXPR: >+ case UNGT_EXPR: >+ case UNGE_EXPR: >+ case UNLT_EXPR: >+ case UNLE_EXPR: >+ case ORDERED_EXPR: >+ case UNORDERED_EXPR: >+ cop = invert_tree_comparison (op, >+ HONOR_NANS >+ (gimple_assign_rhs1 (asgn))); >+ >+ if (cop == ERROR_MARK) >+ /* ??? Can we do better? */ >+ continue; >+ >+ break; >+ >+ /* ??? Maybe handle these too? */ >+ case TRUTH_NOT_EXPR: /* Unary! */ >+ case TRUTH_ANDIF_EXPR: >+ case TRUTH_ORIF_EXPR: >+ case TRUTH_AND_EXPR: >+ case TRUTH_OR_EXPR: >+ case TRUTH_XOR_EXPR: >+ default: >+ continue; >+ } >+ >+ /* These are the operands for the verification. */ >+ tree lhs = gimple_assign_lhs (asgn); >+ tree op1 = gimple_assign_rhs1 (asgn); >+ tree op2 = gimple_assign_rhs2 (asgn); >+ >+ /* Vector booleans can't be used in conditional branches. >+ ??? Can we do better? */ >+ if (VECTOR_TYPE_P (TREE_TYPE (op1))) >+ continue; >+ >+ gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE); >+ >+ tree rhs = copy_ssa_name (lhs); >+ >+ gimple_stmt_iterator gsi_split = gsi; >+ gsi_next (&gsi_split); >+ >+ bool same_p = (op1 == op2); >+ op1 = detach_value (&gsi_split, op1); >+ op2 = same_p ? op1 : detach_value (&gsi_split, op2); >+ >+ gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); >+ gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); >+ >+ if (!gsi_end_p (gsi_split)) >+ { >+ gsi_prev (&gsi_split); >+ split_block (bb, gsi_stmt (gsi_split)); >+ gsi_next (&gsi_split); >+ gcc_checking_assert (gsi_end_p (gsi_split)); >+ >+ if (dump_file) >+ fprintf (dump_file, "Splitting block %i\n", bb->index); >+ } >+ >+ gcc_checking_assert (single_succ_p (bb)); >+ >+ insert_check_and_trap (&gsi_split, EDGE_TRUE_VALUE, >+ EQ_EXPR, lhs, rhs); >+ } >+ >+ return 0; >+} >+ >+gimple_opt_pass * >+make_pass_harden_compares (gcc::context *ctxt) >+{ >+ return new pass_harden_compares (ctxt); >+} >diff --git a/gcc/passes.def b/gcc/passes.def >index 8e4638d20edac..2d81086df1d39 100644 >--- a/gcc/passes.def >+++ b/gcc/passes.def >@@ -421,6 +421,8 @@ along with GCC; see the file COPYING3. If not see > NEXT_PASS (pass_lower_resx); > NEXT_PASS (pass_nrv); > NEXT_PASS (pass_gimple_isel); >+ NEXT_PASS (pass_harden_conditional_branches); >+ NEXT_PASS (pass_harden_compares); > NEXT_PASS (pass_cleanup_cfg_post_optimizing); > NEXT_PASS (pass_warn_function_noreturn); > NEXT_PASS (pass_warn_access); >diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c >new file mode 100644 >index 0000000000000..1ee0b3663443d >--- /dev/null >+++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c >@@ -0,0 +1,14 @@ >+/* { dg-do compile } */ >+/* { dg-options "-fharden-compares -fdump-tree-hardcmp -ffat-lto-objects" } */ >+ >+int >+f (int i, int j) >+{ >+ return i < j; >+} >+ >+/* { dg-final { scan-tree-dump-times "Splitting block" 1 "hardcmp" } } */ >+/* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */ >+/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */ >+/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */ >+/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */ >diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c >new file mode 100644 >index 0000000000000..86de8e155ed38 >--- /dev/null >+++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c >@@ -0,0 +1,18 @@ >+/* { dg-do compile } */ >+/* { dg-options "-fharden-conditional-branches -fdump-tree-hardcbr -ffat-lto-objects" } */ >+ >+extern int f1 (void); >+extern int f2 (void); >+ >+ >+int >+f (int i, int j) >+{ >+ return (i < j) ? f1 () : f2 (); >+} >+ >+/* { dg-final { scan-tree-dump-times "Splitting edge" 2 "hardcbr" } } */ >+/* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */ >+/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */ >+/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */ >+/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */ >diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h >index 9d19228c385f7..7eae2f26f5195 100644 >--- a/gcc/tree-pass.h >+++ b/gcc/tree-pass.h >@@ -643,6 +643,9 @@ extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_gimple_isel (gcc::context *ctxt); >+extern gimple_opt_pass *make_pass_harden_compares (gcc::context *ctxt); >+extern gimple_opt_pass *make_pass_harden_conditional_branches (gcc::context >+ *ctxt); > > /* Current optimization pass. */ > extern opt_pass *current_pass; > >
On Oct 9, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > Why two passes (and two IL traverses?) Different traversals, no reason to force them into a single pass. One only looks at the last stmt of each block, where cond stmts may be, while the other has to look at every stmt. > How do you prevent RTL optimizers (jump threading) from removing the > redundant tests? The trick I'm using to copy of a value without the compiler's knowing it's still the same value is 'asm ("" : "=g" (alt) : "0" (src));' I've pondered introducing __builtin_hidden_copy or somesuch, but it didn't seem worth it. > I'd have expected such hardening to occur very late in the RTL > pipeline. Yeah, that would be another way to do it, but then it would have to be a lot trickier, given all the different ways in which compare-and-branch can be expressed in RTL.
On Tue, Oct 12, 2021 at 8:35 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 9, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > > > Why two passes (and two IL traverses?) > > Different traversals, no reason to force them into a single pass. One > only looks at the last stmt of each block, where cond stmts may be, > while the other has to look at every stmt. > > > How do you prevent RTL optimizers (jump threading) from removing the > > redundant tests? > > The trick I'm using to copy of a value without the compiler's knowing > it's still the same value is 'asm ("" : "=g" (alt) : "0" (src));' > > I've pondered introducing __builtin_hidden_copy or somesuch, but it > didn't seem worth it. I see. I remember Marc using sth similar initially when trying to solve the FENV access problem. Maybe we indeed want to have some kind of more generic "dataflow (optimization) barrier" ... Are there any issues with respect to debugging when using such asm()s? > > I'd have expected such hardening to occur very late in the RTL > > pipeline. > > Yeah, that would be another way to do it, but then it would have to be a > lot trickier, given all the different ways in which compare-and-branch > can be expressed in RTL. Agreed, though it would be less disturbing to the early RTL pipeline and RTL expansion. > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about <https://stallmansupport.org>
On Oct 12, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > Are there any issues with respect to debugging when using such > asm()s? Not in this case. When creating short-lived copies for immediate use, like I do in the proposed patch, either the original value remains live in its original location and we use an actual copy, or the original value was dead, and we'll have a stmt/insn that visibly marks it as such, though the value actually remains there. The newly-added compare statements use these anonymous, temporary copies, so they're not relevant for debug information. Using asms could have effects on debug info and on optimizations in case their outputs *become* the location/value of the variable, i.e., as if in source code we did: asm ("" : "+g" (var)); After this optimization barrier, the compiler wouldn't know any more anything it might have known before about the value held in the variable. And, if the variable is a gimple register, there would have to be a new debug bind stmt to bind the variable to its "new" value. (The debug machinery would assume the asm stmt modifies the value, and the output would thus overwrite the location with a value unrelated to the variable without the restated debug bind) The risk for debug info of introducing such asm stmts after conversion into SSA is that the debug binds wouldn't be added automatically, as they are when they're present in source code. >> Yeah, that would be another way to do it, but then it would have to be a >> lot trickier, given all the different ways in which compare-and-branch >> can be expressed in RTL. > Agreed, though it would be less disturbing to the early RTL pipeline > and RTL expansion. Is that a concern, though? It's not like such structures couldn't be present in source code, after all, so the RTL machinery has to be able to deal with them one way or another. For someone who wishes the compiler to introduce this hardening, the point in which it's added seems immaterial to me, as long as it remains all the way to the end. Now, if we had some kind of optimization barrier at the point of use, we would be able to save a copy in some cases. E.g., instead of: tmp = var; asm ("" : "+g" (tmp)); if (tmp < cst) ... [reads from var] we could have: if (__noopt (var) < cst) ... [reads from var] and that would use var's value without taking an extra copy it, but also without enabling optimizations based on knowledge about the value. ISTM that introducing this sort of __noopt use would be a very significant undertaking. In RTL, a pseudo set to the original value of var, and that eventually resolves to the same location that holds that value, but marked in a way that prevents CSE, fwprop and whatnot (make it volatile?) would likely get 99% of the way there, but making pseudos to that end (and having such marks remain after reload) seems nontrivial to me.
On Wed, Oct 13, 2021 at 8:54 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 12, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > > > Are there any issues with respect to debugging when using such > > asm()s? > > Not in this case. When creating short-lived copies for immediate use, > like I do in the proposed patch, either the original value remains live > in its original location and we use an actual copy, or the original > value was dead, and we'll have a stmt/insn that visibly marks it as > such, though the value actually remains there. The newly-added compare > statements use these anonymous, temporary copies, so they're not > relevant for debug information. > > Using asms could have effects on debug info and on optimizations in case > their outputs *become* the location/value of the variable, i.e., as if > in source code we did: > > asm ("" : "+g" (var)); > > After this optimization barrier, the compiler wouldn't know any more > anything it might have known before about the value held in the > variable. And, if the variable is a gimple register, there would have > to be a new debug bind stmt to bind the variable to its "new" value. > (The debug machinery would assume the asm stmt modifies the value, and > the output would thus overwrite the location with a value unrelated to > the variable without the restated debug bind) > > > The risk for debug info of introducing such asm stmts after conversion > into SSA is that the debug binds wouldn't be added automatically, as > they are when they're present in source code. > > > >> Yeah, that would be another way to do it, but then it would have to be a > >> lot trickier, given all the different ways in which compare-and-branch > >> can be expressed in RTL. > > > Agreed, though it would be less disturbing to the early RTL pipeline > > and RTL expansion. > > Is that a concern, though? It's not like such structures couldn't be > present in source code, after all, so the RTL machinery has to be able > to deal with them one way or another. For someone who wishes the > compiler to introduce this hardening, the point in which it's added > seems immaterial to me, as long as it remains all the way to the end. > > > Now, if we had some kind of optimization barrier at the point of use, we > would be able to save a copy in some cases. E.g., instead of: > > tmp = var; > asm ("" : "+g" (tmp)); > if (tmp < cst) ... > [reads from var] > > we could have: > > if (__noopt (var) < cst) ... > [reads from var] > > and that would use var's value without taking an extra copy it, but also > without enabling optimizations based on knowledge about the value. > > ISTM that introducing this sort of __noopt use would be a very > significant undertaking. In RTL, a pseudo set to the original value of > var, and that eventually resolves to the same location that holds that > value, but marked in a way that prevents CSE, fwprop and whatnot (make > it volatile?) would likely get 99% of the way there, but making pseudos > to that end (and having such marks remain after reload) seems nontrivial > to me. Yeah, I think that eventually marking the operation we want to preserve (with volatile?) would be the best way. On GIMPLE that's difficult, it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the expression at least), and possibly also on RTL (where such flag might already be a thing?). So when going that way doing the hardening on RTL seems easier (if you want to catch all compares, if you want to only catch compare + jump that has your mentioned issue of all the different representations). Of course if volatile non-memory operations are not a thing already on RTL (well, we _do_ have volatile UNSPECs ...) then of course you still need to at least look at redundancy elimination and expression combining passes to honor such flag. Note that I did not look on the actual patch, I'm trying to see whether there's some generic usefulness we can extract from the patchs requirement which to me looks quite specific and given it's "hackish" implementation way might not be the most important one to carry on trunk (I understand that AdaCore will carry it in their compiler). Thanks, Richard. > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about <https://stallmansupport.org>
On Oct 14, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > Yeah, I think that eventually marking the operation we want to preserve > (with volatile?) would be the best way. On GIMPLE that's difficult, > it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the > expression at least), and possibly also on RTL (where such flag > might already be a thing?). Making the expr volatile would likely get gimple to deal with it like memory, which would completely defeat the point of trying to avoid a copy. RTL has support for volatile MEMs and (user-)REGs indeed, but in order to avoid the copy, we don't want the pseudo to be volatile, we want specific users thereof to be. An unspec_volatile would accomplish that, but it would require RTL patterns to match it wherever a pseudo might appear. Considering all forms of insns involving conditionals on all relevant targets, that's far too much effort for no measurable beenefit. > So when going that way doing the hardening on RTL seems easier (if you > want to catch all compares, if you want to only catch compare + jump > that has your mentioned issue of all the different representations) It's not. RTL has various ways to represent store-flags too. Even now that we don't have to worry about implicit CC, a single boolean test may expand to a compare-and-set-[CC-]reg, and then a compare-and-store-CC-reg, or a single compare-and-set-[non-CC-]reg, and IIRC in some cases even more than one (pair of) conditionals. Compare-and-branches also come in such a multitude of settings. It all depends on the target, and I don't really see any benefit whatsoever of implementing such trivial gimple passes with all the potential complexity of RTL on all the architectures relevant for GCC, or even for this project alone. > Note that I did not look on the actual patch, I'm trying to see whether there's > some generic usefulness we can extract from the patchs requirement > which to me looks quite specific and given it's "hackish" implementation > way might not be the most important one to carry on trunk (I understand > that AdaCore will carry it in their compiler). It's also simple, no-maintenance, and entirely self-contained. A good example of something that could be implemented as a plugin, except for command-line options. Maybe we could have a plugin collection in our source tree, to hold stuff like this and to offer examples of plugins, and means to build select plugins as such, or as preloaded modules into the compiler for easier deployment.
On Fri, Oct 15, 2021 at 8:35 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 14, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > > > Yeah, I think that eventually marking the operation we want to preserve > > (with volatile?) would be the best way. On GIMPLE that's difficult, > > it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the > > expression at least), and possibly also on RTL (where such flag > > might already be a thing?). > > Making the expr volatile would likely get gimple to deal with it like > memory, which would completely defeat the point of trying to avoid a > copy. > > RTL has support for volatile MEMs and (user-)REGs indeed, but in order > to avoid the copy, we don't want the pseudo to be volatile, we want > specific users thereof to be. An unspec_volatile would accomplish that, > but it would require RTL patterns to match it wherever a pseudo might > appear. Considering all forms of insns involving conditionals on all > relevant targets, that's far too much effort for no measurable beenefit. > > > > So when going that way doing the hardening on RTL seems easier (if you > > want to catch all compares, if you want to only catch compare + jump > > that has your mentioned issue of all the different representations) > > It's not. RTL has various ways to represent store-flags too. Even now > that we don't have to worry about implicit CC, a single boolean test may > expand to a compare-and-set-[CC-]reg, and then a > compare-and-store-CC-reg, or a single compare-and-set-[non-CC-]reg, and > IIRC in some cases even more than one (pair of) conditionals. > > Compare-and-branches also come in such a multitude of settings. > > It all depends on the target, and I don't really see any benefit > whatsoever of implementing such trivial gimple passes with all the > potential complexity of RTL on all the architectures relevant for GCC, > or even for this project alone. > > > Note that I did not look on the actual patch, I'm trying to see whether there's > > some generic usefulness we can extract from the patchs requirement > > which to me looks quite specific and given it's "hackish" implementation > > way might not be the most important one to carry on trunk (I understand > > that AdaCore will carry it in their compiler). > > It's also simple, no-maintenance, and entirely self-contained. Yes, it is (just having had a quick look most of the functions in the pass lack function-level comments). > A good > example of something that could be implemented as a plugin, except for > command-line options. > > Maybe we could have a plugin collection in our source tree, to hold > stuff like this and to offer examples of plugins, and means to build > select plugins as such, or as preloaded modules into the compiler for > easier deployment. I think this has been suggested before, yes. But if those "plugins" are as self-contained as yours here there's also no reason to not simply compile them in as regular passes (unless they are slightly broken and thus a maintainance burden). Richard. > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about <https://stallmansupport.org>
On Oct 18, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > Yes, it is (just having had a quick look most of the functions in the > pass lack function-level comments). Oh my, I'm so sorry, please accept my apologies. I stepped away from this patch for a few weeks, and when I got back to it, I did not realize it wasn't quite finished yet. I misled myself because I had already written the ChangeLog entry, which I normally do as the last thing before contributing a patch. Besides the comments, it was missing preservation of source location information. I've implemented the missing bit, and added comments to all functions and then some. This patch regstrapped successfully on x86_64-linux-gnu. Unfortunately, both this patch and the earlier patch, applied onto recent trunk along with a patch that enables both passes (both now in aoliva/hardcomp), hit a bootstrap compare error in insn-opinit.c, confirmed with -fcompare-debug. I suppose it's a latent issue exposed by the patch, rather than some problem introduced by the patch, because the earlier patch had bootstrapped successfully with both passes enabled back then. I'm yet to investigate this problem, but I'm a little tied up with something else ATM, and it's likely an unrelated latent problem to be fixed in a separate patch, so I'm posting this right away, and even daring ask: ok to install? This patch introduces optional passes to harden conditionals used in branches, and in computing boolean expressions, by adding redundant tests of the reversed conditions, and trapping in case of unexpected results. Though in abstract machines the redundant tests should never fail, CPUs may be led to misbehave under certain kinds of attacks, such as of power deprivation, and these tests reduce the likelihood of going too far down an unexpected execution path. for gcc/ChangeLog * common.opt (fharden-compares): New. (fharden-conditional-branches): New. * doc/invoke.texi: Document new options. * gimple-harden-conditionals.cc: New. * passes.def: Add new passes. * tree-pass.h (make_pass_harden_compares): Declare. (make_pass_harden_conditional_branches): Declare. for gcc/ada/ChangeLog * doc/gnat_rm/security_hardening_features.rst (Hardened Conditionals): New. for gcc/testsuite/ChangeLog * c-c++-common/torture/harden-comp.c: New. * c-c++-common/torture/harden-cond.c: New. --- gcc/Makefile.in | 1 .../doc/gnat_rm/security_hardening_features.rst | 40 ++ gcc/common.opt | 8 gcc/doc/invoke.texi | 19 + gcc/gimple-harden-conditionals.cc | 435 ++++++++++++++++++++ gcc/passes.def | 2 gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + gcc/tree-pass.h | 3 9 files changed, 540 insertions(+) create mode 100644 gcc/gimple-harden-conditionals.cc create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c diff --git a/gcc/Makefile.in b/gcc/Makefile.in index f36ffa4740b78..a79ff93dd5999 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1389,6 +1389,7 @@ OBJS = \ gimple-if-to-switch.o \ gimple-iterator.o \ gimple-fold.o \ + gimple-harden-conditionals.o \ gimple-laddress.o \ gimple-loop-interchange.o \ gimple-loop-jam.o \ diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst index 1c46e3a4c7b88..52240d7e3dd54 100644 --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst @@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, it is not currently recommended to rely on any effects this pragma might be expected to have when calling subprograms through access-to-subprogram variables. + + +.. Hardened Conditionals: + +Hardened Conditionals +===================== + +GNAT can harden conditionals to protect against control flow attacks. + +This is accomplished by two complementary transformations, each +activated by a separate command-line option. + +The option *-fharden-compares* enables hardening of compares that +compute results stored in variables, adding verification that the +reversed compare yields the opposite result. + +The option *-fharden-conditional-branches* enables hardening of +compares that guard conditional branches, adding verification of the +reversed compare to both execution paths. + +These transformations are introduced late in the compilation pipeline, +long after boolean expressions are decomposed into separate compares, +each one turned into either a conditional branch or a compare whose +result is stored in a boolean variable or temporary. Compiler +optimizations, if enabled, may also turn conditional branches into +stored compares, and vice-versa. Conditionals may also be optimized +out entirely, if their value can be determined at compile time, and +occasionally multiple compares can be combined into one. + +It is thus difficult to predict which of these two options will affect +a specific compare operation expressed in source code. Using both +options ensures that every compare that is not optimized out will be +hardened. + +The addition of reversed compares can be observed by enabling the dump +files of the corresponding passes, through command line options +*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. + +They are separate options, however, because of the significantly +different performance impact of the hardening transformations. diff --git a/gcc/common.opt b/gcc/common.opt index a2af7fb36e0dd..7144755f66d61 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1722,6 +1722,14 @@ fguess-branch-probability Common Var(flag_guess_branch_prob) Optimization Enable guessing of branch probabilities. +fharden-compares +Common Var(flag_harden_compares) Optimization +Harden conditionals not used in branches, checking reversed conditions. + +fharden-conditional-branches +Common Var(flag_harden_conditional_branches) Optimization +Harden conditional branches by checking reversed conditions. + ; Nonzero means ignore `#ident' directives. 0 means handle them. ; Generate position-independent code for executables if possible ; On SVR4 targets, it also controls whether or not to emit a diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 0cc8a8edd058c..a9129a6f11c64 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -596,6 +596,7 @@ Objective-C and Objective-C++ Dialects}. -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol -fsanitize-undefined-trap-on-error -fbounds-check @gol -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol +-fharden-compares -fharden-conditional-branches @gol -fstack-protector -fstack-protector-all -fstack-protector-strong @gol -fstack-protector-explicit -fstack-check @gol -fstack-limit-register=@var{reg} -fstack-limit-symbol=@var{sym} @gol @@ -15532,6 +15533,24 @@ which functions and calls should be skipped from instrumentation Currently the x86 GNU/Linux target provides an implementation based on Intel Control-flow Enforcement Technology (CET). +@item -fharden-compares +@opindex fharden-compares +For every logical test that survives gimple optimizations and is +@emph{not} the condition in a conditional branch (for example, +conditions tested for conditional moves, or to store in boolean +variables), emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the results do not +match. Use with @samp{-fharden-conditional-branches} to cover all +conditionals. + +@item -fharden-conditional-branches +@opindex fharden-conditional-branches +For every non-vectorized conditional branch that survives gimple +optimizations, emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the result is +unexpected. Use with @samp{-fharden-compares} to cover all +conditionals. + @item -fstack-protector @opindex fstack-protector Emit extra code to check for buffer overflows, such as stack smashing diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc new file mode 100644 index 0000000000000..047c5036feaf5 --- /dev/null +++ b/gcc/gimple-harden-conditionals.cc @@ -0,0 +1,435 @@ +/* Harden conditionals. + Copyright (C) 2021 Free Software Foundation, Inc. + Contributed by Alexandre Oliva <oliva@adacore.com>. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "backend.h" +#include "tree.h" +#include "fold-const.h" +#include "gimple.h" +#include "gimplify.h" +#include "tree-pass.h" +#include "ssa.h" +#include "gimple-iterator.h" +#include "tree-cfg.h" +#include "basic-block.h" +#include "cfghooks.h" +#include "cfgloop.h" +#include "diagnostic.h" +#include "intl.h" + +namespace { + +/* These passes introduces redundant, but reversed conditionals at + compares, such as those used in conditional branches, and those + that compute boolean results. This doesn't make much sense for + abstract CPUs, but this kind of hardening may avoid undesirable + execution paths on actual CPUs under such attacks as of power + deprivation. */ + +/* Define a pass to harden conditionals other than branches. */ + +const pass_data pass_data_harden_compares = { + GIMPLE_PASS, + "hardcmp", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_compares : public gimple_opt_pass +{ +public: + pass_harden_compares (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_compares, ctxt) + {} + opt_pass *clone () { return new pass_harden_compares (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_compares; + } + virtual unsigned int execute (function *); +}; + +/* Define a pass to harden conditionals in branches. This pass must + run after the above, otherwise it will re-harden the checks + introduced by the above. */ + +const pass_data pass_data_harden_conditional_branches = { + GIMPLE_PASS, + "hardcbr", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_conditional_branches : public gimple_opt_pass +{ +public: + pass_harden_conditional_branches (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_conditional_branches, ctxt) + {} + opt_pass *clone () { return new pass_harden_conditional_branches (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_conditional_branches; + } + virtual unsigned int execute (function *); +}; + +} + +/* If VAL is an SSA name, return an SSA name holding the same value, + but without the compiler's knowing that it holds the same value, so + that uses thereof can't be optimized the way VAL might. Insert + stmts that initialize it before *GSIP, with LOC. + + Otherwise, VAL must be an invariant, returned unchanged. */ + +static inline tree +detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val) +{ + if (TREE_CONSTANT (val) || TREE_CODE (val) != SSA_NAME) + { + gcc_checking_assert (is_gimple_min_invariant (val)); + return val; + } + + tree ret = copy_ssa_name (val); + + /* Output asm ("" : "=g" (ret) : "0" (val)); */ + vec<tree, va_gc> *inputs = NULL; + vec<tree, va_gc> *outputs = NULL; + vec_safe_push (outputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (2, "=g")), + ret)); + vec_safe_push (inputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (1, "0")), + val)); + gasm *detach = gimple_build_asm_vec ("", inputs, outputs, + NULL, NULL); + gimple_set_location (detach, loc); + gsi_insert_before (gsip, detach, GSI_SAME_STMT); + + SSA_NAME_DEF_STMT (ret) = detach; + + return ret; +} + +/* Build a cond stmt out of COP, LHS, RHS, insert it before *GSIP with + location LOC. *GSIP must be at the end of a basic block. The succ + edge out of the block becomes the true or false edge opposite to + that in FLAGS. Create a new block with a single trap stmt, in the + cold partition if the function is partitioned,, and a new edge to + it as the other edge for the cond. */ + +static inline void +insert_check_and_trap (location_t loc, gimple_stmt_iterator *gsip, + int flags, enum tree_code cop, tree lhs, tree rhs) +{ + basic_block chk = gsi_bb (*gsip); + + gcond *cond = gimple_build_cond (cop, lhs, rhs, NULL, NULL); + gimple_set_location (cond, loc); + gsi_insert_before (gsip, cond, GSI_SAME_STMT); + + basic_block trp = create_empty_bb (chk); + + gimple_stmt_iterator gsit = gsi_after_labels (trp); + gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + gimple_set_location (trap, loc); + gsi_insert_before (&gsit, trap, GSI_SAME_STMT); + + if (dump_file) + fprintf (dump_file, + "Adding reversed compare to block %i, and trap to block %i\n", + chk->index, trp->index); + + if (BB_PARTITION (chk)) + BB_SET_PARTITION (trp, BB_COLD_PARTITION); + + int true_false_flag = flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + gcc_assert (true_false_flag); + int neg_true_false_flag = (~flags) & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + + /* Remove the fallthru bit, and set the truth value for the + preexisting edge and for the newly-created one. In hardcbr, + FLAGS is taken from the edge of the original cond expr that we're + dealing with, so the reversed compare is expected to yield the + negated result, and the same result calls for a trap. In + hardcmp, we're comparing the boolean results of the original and + of the reversed compare, so we're passed FLAGS to trap on + equality. */ + single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU; + single_succ_edge (chk)->flags |= neg_true_false_flag; + edge e = make_edge (chk, trp, true_false_flag); + e->goto_locus = loc; + + if (dom_info_available_p (CDI_DOMINATORS)) + set_immediate_dominator (CDI_DOMINATORS, trp, chk); + if (current_loops) + add_bb_to_loop (trp, current_loops->tree_root); +} + +/* Split edge E, and insert_check_and_trap (see above) in the + newly-created block, using detached copies of LHS's and RHS's + values (see detach_value above) for the COP compare. */ + +static inline void +insert_edge_check_and_trap (location_t loc, edge e, + enum tree_code cop, tree lhs, tree rhs) +{ + int flags = e->flags; + basic_block src = e->src; + basic_block dest = e->dest; + location_t eloc = e->goto_locus; + + basic_block chk = split_edge (e); + e = NULL; + + single_pred_edge (chk)->goto_locus = loc; + single_succ_edge (chk)->goto_locus = eloc; + + if (dump_file) + fprintf (dump_file, + "Splitting edge %i->%i into block %i\n", + src->index, dest->index, chk->index); + + gimple_stmt_iterator gsik = gsi_after_labels (chk); + + bool same_p = (lhs == rhs); + lhs = detach_value (loc, &gsik, lhs); + rhs = same_p ? lhs : detach_value (loc, &gsik, rhs); + + insert_check_and_trap (loc, &gsik, flags, cop, lhs, rhs); +} + +/* Harden cond stmts at the end of FUN's blocks. */ + +unsigned int +pass_harden_conditional_branches::execute (function *fun) +{ + basic_block bb; + FOR_EACH_BB_REVERSE_FN (bb, fun) + { + gimple_stmt_iterator gsi = gsi_last_bb (bb); + + if (gsi_end_p (gsi)) + continue; + + gcond *cond = dyn_cast <gcond *> (gsi_stmt (gsi)); + if (!cond) + continue; + + /* Turn: + + if (x op y) goto l1; else goto l2; + + into: + + if (x op y) goto l1'; else goto l2'; + l1': if (x' cop y') goto l1'trap; else goto l1; + l1'trap: __builtin_trap (); + l2': if (x' cop y') goto l2; else goto l2'trap; + l2'trap: __builtin_trap (); + + where cop is a complementary boolean operation to op; l1', l1'trap, + l2' and l2'trap are newly-created labels; and x' and y' hold the same + value as x and y, but in a way that does not enable the compiler to + optimize the redundant compare away. + */ + + enum tree_code op = gimple_cond_code (cond); + tree lhs = gimple_cond_lhs (cond); + tree rhs = gimple_cond_rhs (cond); + location_t loc = gimple_location (cond); + + enum tree_code cop = invert_tree_comparison (op, HONOR_NANS (lhs)); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 0), cop, lhs, rhs); + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 1), cop, lhs, rhs); + } + + return 0; +} + +/* Instantiate a hardcbr pass. */ + +gimple_opt_pass * +make_pass_harden_conditional_branches (gcc::context *ctxt) +{ + return new pass_harden_conditional_branches (ctxt); +} + +/* Harden boolean-yielding compares in FUN. */ + +unsigned int +pass_harden_compares::execute (function *fun) +{ + basic_block bb; + /* Go backwards over BBs and stmts, so that, even if we split the + block multiple times to insert a cond_expr after each compare we + find, we remain in the same block, visiting every preexisting + stmt exactly once, and not visiting newly-added blocks or + stmts. */ + FOR_EACH_BB_REVERSE_FN (bb, fun) + for (gimple_stmt_iterator gsi = gsi_last_bb (bb); + !gsi_end_p (gsi); gsi_prev (&gsi)) + { + gassign *asgn = dyn_cast <gassign *> (gsi_stmt (gsi)); + if (!asgn) + continue; + + /* Turn: + + z = x op y; + + into: + + z = x op y; + z' = x' cop y'; + if (z == z') __builtin_trap (); + + where cop is a complementary boolean operation to op; and x' + and y' hold the same value as x and y, but in a way that does + not enable the compiler to optimize the redundant compare + away. + */ + + enum tree_code op = gimple_assign_rhs_code (asgn); + + enum tree_code cop; + + switch (op) + { + case EQ_EXPR: + case NE_EXPR: + case GT_EXPR: + case GE_EXPR: + case LT_EXPR: + case LE_EXPR: + case LTGT_EXPR: + case UNEQ_EXPR: + case UNGT_EXPR: + case UNGE_EXPR: + case UNLT_EXPR: + case UNLE_EXPR: + case ORDERED_EXPR: + case UNORDERED_EXPR: + cop = invert_tree_comparison (op, + HONOR_NANS + (gimple_assign_rhs1 (asgn))); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + break; + + /* ??? Maybe handle these too? */ + case TRUTH_NOT_EXPR: + /* ??? The code below assumes binary ops, it would have to + be adjusted for TRUTH_NOT_EXPR, since it's unary. */ + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_XOR_EXPR: + default: + continue; + } + + /* These are the operands for the verification. */ + tree lhs = gimple_assign_lhs (asgn); + tree op1 = gimple_assign_rhs1 (asgn); + tree op2 = gimple_assign_rhs2 (asgn); + location_t loc = gimple_location (asgn); + + /* Vector booleans can't be used in conditional branches. ??? + Can we do better? How to reduce compare and + reversed-compare result vectors to a single boolean? */ + if (VECTOR_TYPE_P (TREE_TYPE (op1))) + continue; + + gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE); + + tree rhs = copy_ssa_name (lhs); + + gimple_stmt_iterator gsi_split = gsi; + gsi_next (&gsi_split); + + bool same_p = (op1 == op2); + op1 = detach_value (loc, &gsi_split, op1); + op2 = same_p ? op1 : detach_value (loc, &gsi_split, op2); + + gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); + gimple_set_location (asgnck, loc); + gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); + + /* We wish to insert a cond_expr after the compare, so arrange + for it to be at the end of a block if it isn't. */ + if (!gsi_end_p (gsi_split)) + { + gsi_prev (&gsi_split); + split_block (bb, gsi_stmt (gsi_split)); + gsi_next (&gsi_split); + gcc_checking_assert (gsi_end_p (gsi_split)); + + single_succ_edge (bb)->goto_locus = loc; + + if (dump_file) + fprintf (dump_file, "Splitting block %i\n", bb->index); + } + + gcc_checking_assert (single_succ_p (bb)); + + insert_check_and_trap (loc, &gsi_split, EDGE_TRUE_VALUE, + EQ_EXPR, lhs, rhs); + } + + return 0; +} + +/* Instantiate a hardcmp pass. */ + +gimple_opt_pass * +make_pass_harden_compares (gcc::context *ctxt) +{ + return new pass_harden_compares (ctxt); +} diff --git a/gcc/passes.def b/gcc/passes.def index c11c237f6d204..268d8595a401e 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -421,6 +421,8 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_lower_resx); NEXT_PASS (pass_nrv); NEXT_PASS (pass_gimple_isel); + NEXT_PASS (pass_harden_conditional_branches); + NEXT_PASS (pass_harden_compares); NEXT_PASS (pass_cleanup_cfg_post_optimizing); NEXT_PASS (pass_warn_function_noreturn); NEXT_PASS (pass_warn_access); diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c new file mode 100644 index 0000000000000..1ee0b3663443d --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-compares -fdump-tree-hardcmp -ffat-lto-objects" } */ + +int +f (int i, int j) +{ + return i < j; +} + +/* { dg-final { scan-tree-dump-times "Splitting block" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */ diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c new file mode 100644 index 0000000000000..86de8e155ed38 --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-conditional-branches -fdump-tree-hardcbr -ffat-lto-objects" } */ + +extern int f1 (void); +extern int f2 (void); + + +int +f (int i, int j) +{ + return (i < j) ? f1 () : f2 (); +} + +/* { dg-final { scan-tree-dump-times "Splitting edge" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */ diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index d379769a94364..e807ad855efd1 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -644,6 +644,9 @@ extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt); extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt); extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt); extern gimple_opt_pass *make_pass_gimple_isel (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_compares (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_conditional_branches (gcc::context + *ctxt); /* Current optimization pass. */ extern opt_pass *current_pass;
On Oct 20, 2021, Alexandre Oliva <oliva@adacore.com> wrote: > I suppose it's a latent issue exposed by the patch, I was mistaken. Though I even had bisected the -fcompare-debug problem back to a patch from back in May, that added a new sink_code pass before store_merging, it was actually a bug in my patch, it was just a little hard to hit with bootstrap-debug, but it came up with -fcompare-debug in ways that misled me. split_block remains slightly risky to use unless you know you have or are going to insert nondebug stmts/insns in both blocks. I've often pondered warning in case split_block completes with only debug stmts/insns in either block, but IIRC there are multiple passes that split first and insert code afterwards, which have to be rearranged to aovid the warning. Anyway, here's the fixed patch. Regstrapped on x86_64-linux-gnu, and bootstrapped with an additional patch that enables both new passes. Ok to install? hardened conditionals From: Alexandre Oliva <oliva@adacore.com> This patch introduces optional passes to harden conditionals used in branches, and in computing boolean expressions, by adding redundant tests of the reversed conditions, and trapping in case of unexpected results. Though in abstract machines the redundant tests should never fail, CPUs may be led to misbehave under certain kinds of attacks, such as of power deprivation, and these tests reduce the likelihood of going too far down an unexpected execution path. for gcc/ChangeLog * common.opt (fharden-compares): New. (fharden-conditional-branches): New. * doc/invoke.texi: Document new options. * gimple-harden-conditionals.cc: New. * passes.def: Add new passes. * tree-pass.h (make_pass_harden_compares): Declare. (make_pass_harden_conditional_branches): Declare. for gcc/ada/ChangeLog * doc/gnat_rm/security_hardening_features.rst (Hardened Conditionals): New. for gcc/testsuite/ChangeLog * c-c++-common/torture/harden-comp.c: New. * c-c++-common/torture/harden-cond.c: New. --- gcc/Makefile.in | 1 .../doc/gnat_rm/security_hardening_features.rst | 40 ++ gcc/common.opt | 8 gcc/doc/invoke.texi | 19 + gcc/gimple-harden-conditionals.cc | 439 ++++++++++++++++++++ gcc/passes.def | 2 gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + gcc/tree-pass.h | 3 9 files changed, 544 insertions(+) create mode 100644 gcc/gimple-harden-conditionals.cc create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c diff --git a/gcc/Makefile.in b/gcc/Makefile.in index f36ffa4740b78..a79ff93dd5999 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1389,6 +1389,7 @@ OBJS = \ gimple-if-to-switch.o \ gimple-iterator.o \ gimple-fold.o \ + gimple-harden-conditionals.o \ gimple-laddress.o \ gimple-loop-interchange.o \ gimple-loop-jam.o \ diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst index 1c46e3a4c7b88..52240d7e3dd54 100644 --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst @@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, it is not currently recommended to rely on any effects this pragma might be expected to have when calling subprograms through access-to-subprogram variables. + + +.. Hardened Conditionals: + +Hardened Conditionals +===================== + +GNAT can harden conditionals to protect against control flow attacks. + +This is accomplished by two complementary transformations, each +activated by a separate command-line option. + +The option *-fharden-compares* enables hardening of compares that +compute results stored in variables, adding verification that the +reversed compare yields the opposite result. + +The option *-fharden-conditional-branches* enables hardening of +compares that guard conditional branches, adding verification of the +reversed compare to both execution paths. + +These transformations are introduced late in the compilation pipeline, +long after boolean expressions are decomposed into separate compares, +each one turned into either a conditional branch or a compare whose +result is stored in a boolean variable or temporary. Compiler +optimizations, if enabled, may also turn conditional branches into +stored compares, and vice-versa. Conditionals may also be optimized +out entirely, if their value can be determined at compile time, and +occasionally multiple compares can be combined into one. + +It is thus difficult to predict which of these two options will affect +a specific compare operation expressed in source code. Using both +options ensures that every compare that is not optimized out will be +hardened. + +The addition of reversed compares can be observed by enabling the dump +files of the corresponding passes, through command line options +*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. + +They are separate options, however, because of the significantly +different performance impact of the hardening transformations. diff --git a/gcc/common.opt b/gcc/common.opt index a2af7fb36e0dd..7144755f66d61 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1722,6 +1722,14 @@ fguess-branch-probability Common Var(flag_guess_branch_prob) Optimization Enable guessing of branch probabilities. +fharden-compares +Common Var(flag_harden_compares) Optimization +Harden conditionals not used in branches, checking reversed conditions. + +fharden-conditional-branches +Common Var(flag_harden_conditional_branches) Optimization +Harden conditional branches by checking reversed conditions. + ; Nonzero means ignore `#ident' directives. 0 means handle them. ; Generate position-independent code for executables if possible ; On SVR4 targets, it also controls whether or not to emit a diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 0cc8a8edd058c..a9129a6f11c64 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -596,6 +596,7 @@ Objective-C and Objective-C++ Dialects}. -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol -fsanitize-undefined-trap-on-error -fbounds-check @gol -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol +-fharden-compares -fharden-conditional-branches @gol -fstack-protector -fstack-protector-all -fstack-protector-strong @gol -fstack-protector-explicit -fstack-check @gol -fstack-limit-register=@var{reg} -fstack-limit-symbol=@var{sym} @gol @@ -15532,6 +15533,24 @@ which functions and calls should be skipped from instrumentation Currently the x86 GNU/Linux target provides an implementation based on Intel Control-flow Enforcement Technology (CET). +@item -fharden-compares +@opindex fharden-compares +For every logical test that survives gimple optimizations and is +@emph{not} the condition in a conditional branch (for example, +conditions tested for conditional moves, or to store in boolean +variables), emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the results do not +match. Use with @samp{-fharden-conditional-branches} to cover all +conditionals. + +@item -fharden-conditional-branches +@opindex fharden-conditional-branches +For every non-vectorized conditional branch that survives gimple +optimizations, emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the result is +unexpected. Use with @samp{-fharden-compares} to cover all +conditionals. + @item -fstack-protector @opindex fstack-protector Emit extra code to check for buffer overflows, such as stack smashing diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc new file mode 100644 index 0000000000000..8916420d7dfe9 --- /dev/null +++ b/gcc/gimple-harden-conditionals.cc @@ -0,0 +1,439 @@ +/* Harden conditionals. + Copyright (C) 2021 Free Software Foundation, Inc. + Contributed by Alexandre Oliva <oliva@adacore.com>. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "backend.h" +#include "tree.h" +#include "fold-const.h" +#include "gimple.h" +#include "gimplify.h" +#include "tree-pass.h" +#include "ssa.h" +#include "gimple-iterator.h" +#include "tree-cfg.h" +#include "basic-block.h" +#include "cfghooks.h" +#include "cfgloop.h" +#include "diagnostic.h" +#include "intl.h" + +namespace { + +/* These passes introduces redundant, but reversed conditionals at + compares, such as those used in conditional branches, and those + that compute boolean results. This doesn't make much sense for + abstract CPUs, but this kind of hardening may avoid undesirable + execution paths on actual CPUs under such attacks as of power + deprivation. */ + +/* Define a pass to harden conditionals other than branches. */ + +const pass_data pass_data_harden_compares = { + GIMPLE_PASS, + "hardcmp", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_compares : public gimple_opt_pass +{ +public: + pass_harden_compares (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_compares, ctxt) + {} + opt_pass *clone () { return new pass_harden_compares (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_compares; + } + virtual unsigned int execute (function *); +}; + +/* Define a pass to harden conditionals in branches. This pass must + run after the above, otherwise it will re-harden the checks + introduced by the above. */ + +const pass_data pass_data_harden_conditional_branches = { + GIMPLE_PASS, + "hardcbr", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_conditional_branches : public gimple_opt_pass +{ +public: + pass_harden_conditional_branches (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_conditional_branches, ctxt) + {} + opt_pass *clone () { return new pass_harden_conditional_branches (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_conditional_branches; + } + virtual unsigned int execute (function *); +}; + +} + +/* If VAL is an SSA name, return an SSA name holding the same value, + but without the compiler's knowing that it holds the same value, so + that uses thereof can't be optimized the way VAL might. Insert + stmts that initialize it before *GSIP, with LOC. + + Otherwise, VAL must be an invariant, returned unchanged. */ + +static inline tree +detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val) +{ + if (TREE_CONSTANT (val) || TREE_CODE (val) != SSA_NAME) + { + gcc_checking_assert (is_gimple_min_invariant (val)); + return val; + } + + tree ret = copy_ssa_name (val); + + /* Output asm ("" : "=g" (ret) : "0" (val)); */ + vec<tree, va_gc> *inputs = NULL; + vec<tree, va_gc> *outputs = NULL; + vec_safe_push (outputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (2, "=g")), + ret)); + vec_safe_push (inputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (1, "0")), + val)); + gasm *detach = gimple_build_asm_vec ("", inputs, outputs, + NULL, NULL); + gimple_set_location (detach, loc); + gsi_insert_before (gsip, detach, GSI_SAME_STMT); + + SSA_NAME_DEF_STMT (ret) = detach; + + return ret; +} + +/* Build a cond stmt out of COP, LHS, RHS, insert it before *GSIP with + location LOC. *GSIP must be at the end of a basic block. The succ + edge out of the block becomes the true or false edge opposite to + that in FLAGS. Create a new block with a single trap stmt, in the + cold partition if the function is partitioned,, and a new edge to + it as the other edge for the cond. */ + +static inline void +insert_check_and_trap (location_t loc, gimple_stmt_iterator *gsip, + int flags, enum tree_code cop, tree lhs, tree rhs) +{ + basic_block chk = gsi_bb (*gsip); + + gcond *cond = gimple_build_cond (cop, lhs, rhs, NULL, NULL); + gimple_set_location (cond, loc); + gsi_insert_before (gsip, cond, GSI_SAME_STMT); + + basic_block trp = create_empty_bb (chk); + + gimple_stmt_iterator gsit = gsi_after_labels (trp); + gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + gimple_set_location (trap, loc); + gsi_insert_before (&gsit, trap, GSI_SAME_STMT); + + if (dump_file) + fprintf (dump_file, + "Adding reversed compare to block %i, and trap to block %i\n", + chk->index, trp->index); + + if (BB_PARTITION (chk)) + BB_SET_PARTITION (trp, BB_COLD_PARTITION); + + int true_false_flag = flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + gcc_assert (true_false_flag); + int neg_true_false_flag = (~flags) & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + + /* Remove the fallthru bit, and set the truth value for the + preexisting edge and for the newly-created one. In hardcbr, + FLAGS is taken from the edge of the original cond expr that we're + dealing with, so the reversed compare is expected to yield the + negated result, and the same result calls for a trap. In + hardcmp, we're comparing the boolean results of the original and + of the reversed compare, so we're passed FLAGS to trap on + equality. */ + single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU; + single_succ_edge (chk)->flags |= neg_true_false_flag; + edge e = make_edge (chk, trp, true_false_flag); + e->goto_locus = loc; + + if (dom_info_available_p (CDI_DOMINATORS)) + set_immediate_dominator (CDI_DOMINATORS, trp, chk); + if (current_loops) + add_bb_to_loop (trp, current_loops->tree_root); +} + +/* Split edge E, and insert_check_and_trap (see above) in the + newly-created block, using detached copies of LHS's and RHS's + values (see detach_value above) for the COP compare. */ + +static inline void +insert_edge_check_and_trap (location_t loc, edge e, + enum tree_code cop, tree lhs, tree rhs) +{ + int flags = e->flags; + basic_block src = e->src; + basic_block dest = e->dest; + location_t eloc = e->goto_locus; + + basic_block chk = split_edge (e); + e = NULL; + + single_pred_edge (chk)->goto_locus = loc; + single_succ_edge (chk)->goto_locus = eloc; + + if (dump_file) + fprintf (dump_file, + "Splitting edge %i->%i into block %i\n", + src->index, dest->index, chk->index); + + gimple_stmt_iterator gsik = gsi_after_labels (chk); + + bool same_p = (lhs == rhs); + lhs = detach_value (loc, &gsik, lhs); + rhs = same_p ? lhs : detach_value (loc, &gsik, rhs); + + insert_check_and_trap (loc, &gsik, flags, cop, lhs, rhs); +} + +/* Harden cond stmts at the end of FUN's blocks. */ + +unsigned int +pass_harden_conditional_branches::execute (function *fun) +{ + basic_block bb; + FOR_EACH_BB_REVERSE_FN (bb, fun) + { + gimple_stmt_iterator gsi = gsi_last_bb (bb); + + if (gsi_end_p (gsi)) + continue; + + gcond *cond = dyn_cast <gcond *> (gsi_stmt (gsi)); + if (!cond) + continue; + + /* Turn: + + if (x op y) goto l1; else goto l2; + + into: + + if (x op y) goto l1'; else goto l2'; + l1': if (x' cop y') goto l1'trap; else goto l1; + l1'trap: __builtin_trap (); + l2': if (x' cop y') goto l2; else goto l2'trap; + l2'trap: __builtin_trap (); + + where cop is a complementary boolean operation to op; l1', l1'trap, + l2' and l2'trap are newly-created labels; and x' and y' hold the same + value as x and y, but in a way that does not enable the compiler to + optimize the redundant compare away. + */ + + enum tree_code op = gimple_cond_code (cond); + tree lhs = gimple_cond_lhs (cond); + tree rhs = gimple_cond_rhs (cond); + location_t loc = gimple_location (cond); + + enum tree_code cop = invert_tree_comparison (op, HONOR_NANS (lhs)); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 0), cop, lhs, rhs); + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 1), cop, lhs, rhs); + } + + return 0; +} + +/* Instantiate a hardcbr pass. */ + +gimple_opt_pass * +make_pass_harden_conditional_branches (gcc::context *ctxt) +{ + return new pass_harden_conditional_branches (ctxt); +} + +/* Harden boolean-yielding compares in FUN. */ + +unsigned int +pass_harden_compares::execute (function *fun) +{ + basic_block bb; + /* Go backwards over BBs and stmts, so that, even if we split the + block multiple times to insert a cond_expr after each compare we + find, we remain in the same block, visiting every preexisting + stmt exactly once, and not visiting newly-added blocks or + stmts. */ + FOR_EACH_BB_REVERSE_FN (bb, fun) + for (gimple_stmt_iterator gsi = gsi_last_bb (bb); + !gsi_end_p (gsi); gsi_prev (&gsi)) + { + gassign *asgn = dyn_cast <gassign *> (gsi_stmt (gsi)); + if (!asgn) + continue; + + /* Turn: + + z = x op y; + + into: + + z = x op y; + z' = x' cop y'; + if (z == z') __builtin_trap (); + + where cop is a complementary boolean operation to op; and x' + and y' hold the same value as x and y, but in a way that does + not enable the compiler to optimize the redundant compare + away. + */ + + enum tree_code op = gimple_assign_rhs_code (asgn); + + enum tree_code cop; + + switch (op) + { + case EQ_EXPR: + case NE_EXPR: + case GT_EXPR: + case GE_EXPR: + case LT_EXPR: + case LE_EXPR: + case LTGT_EXPR: + case UNEQ_EXPR: + case UNGT_EXPR: + case UNGE_EXPR: + case UNLT_EXPR: + case UNLE_EXPR: + case ORDERED_EXPR: + case UNORDERED_EXPR: + cop = invert_tree_comparison (op, + HONOR_NANS + (gimple_assign_rhs1 (asgn))); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + break; + + /* ??? Maybe handle these too? */ + case TRUTH_NOT_EXPR: + /* ??? The code below assumes binary ops, it would have to + be adjusted for TRUTH_NOT_EXPR, since it's unary. */ + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_XOR_EXPR: + default: + continue; + } + + /* These are the operands for the verification. */ + tree lhs = gimple_assign_lhs (asgn); + tree op1 = gimple_assign_rhs1 (asgn); + tree op2 = gimple_assign_rhs2 (asgn); + location_t loc = gimple_location (asgn); + + /* Vector booleans can't be used in conditional branches. ??? + Can we do better? How to reduce compare and + reversed-compare result vectors to a single boolean? */ + if (VECTOR_TYPE_P (TREE_TYPE (op1))) + continue; + + gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE); + + tree rhs = copy_ssa_name (lhs); + + gimple_stmt_iterator gsi_split = gsi; + /* Don't separate the original assignment from debug stmts + that might be associated with it, and arrange to split the + block after debug stmts, so as to make sure the split block + won't be debug stmts only. */ + gsi_next_nondebug (&gsi_split); + + bool same_p = (op1 == op2); + op1 = detach_value (loc, &gsi_split, op1); + op2 = same_p ? op1 : detach_value (loc, &gsi_split, op2); + + gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); + gimple_set_location (asgnck, loc); + gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); + + /* We wish to insert a cond_expr after the compare, so arrange + for it to be at the end of a block if it isn't. */ + if (!gsi_end_p (gsi_split)) + { + gsi_prev (&gsi_split); + split_block (bb, gsi_stmt (gsi_split)); + gsi_next (&gsi_split); + gcc_checking_assert (gsi_end_p (gsi_split)); + + single_succ_edge (bb)->goto_locus = loc; + + if (dump_file) + fprintf (dump_file, "Splitting block %i\n", bb->index); + } + + gcc_checking_assert (single_succ_p (bb)); + + insert_check_and_trap (loc, &gsi_split, EDGE_TRUE_VALUE, + EQ_EXPR, lhs, rhs); + } + + return 0; +} + +/* Instantiate a hardcmp pass. */ + +gimple_opt_pass * +make_pass_harden_compares (gcc::context *ctxt) +{ + return new pass_harden_compares (ctxt); +} diff --git a/gcc/passes.def b/gcc/passes.def index c11c237f6d204..268d8595a401e 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -421,6 +421,8 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_lower_resx); NEXT_PASS (pass_nrv); NEXT_PASS (pass_gimple_isel); + NEXT_PASS (pass_harden_conditional_branches); + NEXT_PASS (pass_harden_compares); NEXT_PASS (pass_cleanup_cfg_post_optimizing); NEXT_PASS (pass_warn_function_noreturn); NEXT_PASS (pass_warn_access); diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c new file mode 100644 index 0000000000000..1ee0b3663443d --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-compares -fdump-tree-hardcmp -ffat-lto-objects" } */ + +int +f (int i, int j) +{ + return i < j; +} + +/* { dg-final { scan-tree-dump-times "Splitting block" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */ diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c new file mode 100644 index 0000000000000..86de8e155ed38 --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-conditional-branches -fdump-tree-hardcbr -ffat-lto-objects" } */ + +extern int f1 (void); +extern int f2 (void); + + +int +f (int i, int j) +{ + return (i < j) ? f1 () : f2 (); +} + +/* { dg-final { scan-tree-dump-times "Splitting edge" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */ diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index d379769a94364..e807ad855efd1 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -644,6 +644,9 @@ extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt); extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt); extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt); extern gimple_opt_pass *make_pass_gimple_isel (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_compares (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_conditional_branches (gcc::context + *ctxt); /* Current optimization pass. */ extern opt_pass *current_pass;
On Fri, Oct 22, 2021 at 4:19 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 20, 2021, Alexandre Oliva <oliva@adacore.com> wrote: > > > I suppose it's a latent issue exposed by the patch, > > I was mistaken. Though I even had bisected the -fcompare-debug problem > back to a patch from back in May, that added a new sink_code pass before > store_merging, it was actually a bug in my patch, it was just a little > hard to hit with bootstrap-debug, but it came up with -fcompare-debug in > ways that misled me. > > split_block remains slightly risky to use unless you know you have or > are going to insert nondebug stmts/insns in both blocks. I've often > pondered warning in case split_block completes with only debug > stmts/insns in either block, but IIRC there are multiple passes that > split first and insert code afterwards, which have to be rearranged to > aovid the warning. > > Anyway, here's the fixed patch. Regstrapped on x86_64-linux-gnu, and > bootstrapped with an additional patch that enables both new passes. Ok > to install? OK. Thanks, Richard. > > hardened conditionals > > From: Alexandre Oliva <oliva@adacore.com> > > This patch introduces optional passes to harden conditionals used in > branches, and in computing boolean expressions, by adding redundant > tests of the reversed conditions, and trapping in case of unexpected > results. Though in abstract machines the redundant tests should never > fail, CPUs may be led to misbehave under certain kinds of attacks, > such as of power deprivation, and these tests reduce the likelihood of > going too far down an unexpected execution path. > > > for gcc/ChangeLog > > * common.opt (fharden-compares): New. > (fharden-conditional-branches): New. > * doc/invoke.texi: Document new options. > * gimple-harden-conditionals.cc: New. > * passes.def: Add new passes. > * tree-pass.h (make_pass_harden_compares): Declare. > (make_pass_harden_conditional_branches): Declare. > > for gcc/ada/ChangeLog > > * doc/gnat_rm/security_hardening_features.rst > (Hardened Conditionals): New. > > for gcc/testsuite/ChangeLog > > * c-c++-common/torture/harden-comp.c: New. > * c-c++-common/torture/harden-cond.c: New. > --- > gcc/Makefile.in | 1 > .../doc/gnat_rm/security_hardening_features.rst | 40 ++ > gcc/common.opt | 8 > gcc/doc/invoke.texi | 19 + > gcc/gimple-harden-conditionals.cc | 439 ++++++++++++++++++++ > gcc/passes.def | 2 > gcc/testsuite/c-c++-common/torture/harden-comp.c | 14 + > gcc/testsuite/c-c++-common/torture/harden-cond.c | 18 + > gcc/tree-pass.h | 3 > 9 files changed, 544 insertions(+) > create mode 100644 gcc/gimple-harden-conditionals.cc > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-comp.c > create mode 100644 gcc/testsuite/c-c++-common/torture/harden-cond.c > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index f36ffa4740b78..a79ff93dd5999 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -1389,6 +1389,7 @@ OBJS = \ > gimple-if-to-switch.o \ > gimple-iterator.o \ > gimple-fold.o \ > + gimple-harden-conditionals.o \ > gimple-laddress.o \ > gimple-loop-interchange.o \ > gimple-loop-jam.o \ > diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst > index 1c46e3a4c7b88..52240d7e3dd54 100644 > --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst > +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst > @@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, it is not > currently recommended to rely on any effects this pragma might be > expected to have when calling subprograms through access-to-subprogram > variables. > + > + > +.. Hardened Conditionals: > + > +Hardened Conditionals > +===================== > + > +GNAT can harden conditionals to protect against control flow attacks. > + > +This is accomplished by two complementary transformations, each > +activated by a separate command-line option. > + > +The option *-fharden-compares* enables hardening of compares that > +compute results stored in variables, adding verification that the > +reversed compare yields the opposite result. > + > +The option *-fharden-conditional-branches* enables hardening of > +compares that guard conditional branches, adding verification of the > +reversed compare to both execution paths. > + > +These transformations are introduced late in the compilation pipeline, > +long after boolean expressions are decomposed into separate compares, > +each one turned into either a conditional branch or a compare whose > +result is stored in a boolean variable or temporary. Compiler > +optimizations, if enabled, may also turn conditional branches into > +stored compares, and vice-versa. Conditionals may also be optimized > +out entirely, if their value can be determined at compile time, and > +occasionally multiple compares can be combined into one. > + > +It is thus difficult to predict which of these two options will affect > +a specific compare operation expressed in source code. Using both > +options ensures that every compare that is not optimized out will be > +hardened. > + > +The addition of reversed compares can be observed by enabling the dump > +files of the corresponding passes, through command line options > +*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. > + > +They are separate options, however, because of the significantly > +different performance impact of the hardening transformations. > diff --git a/gcc/common.opt b/gcc/common.opt > index a2af7fb36e0dd..7144755f66d61 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -1722,6 +1722,14 @@ fguess-branch-probability > Common Var(flag_guess_branch_prob) Optimization > Enable guessing of branch probabilities. > > +fharden-compares > +Common Var(flag_harden_compares) Optimization > +Harden conditionals not used in branches, checking reversed conditions. > + > +fharden-conditional-branches > +Common Var(flag_harden_conditional_branches) Optimization > +Harden conditional branches by checking reversed conditions. > + > ; Nonzero means ignore `#ident' directives. 0 means handle them. > ; Generate position-independent code for executables if possible > ; On SVR4 targets, it also controls whether or not to emit a > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 0cc8a8edd058c..a9129a6f11c64 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -596,6 +596,7 @@ Objective-C and Objective-C++ Dialects}. > -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol > -fsanitize-undefined-trap-on-error -fbounds-check @gol > -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol > +-fharden-compares -fharden-conditional-branches @gol > -fstack-protector -fstack-protector-all -fstack-protector-strong @gol > -fstack-protector-explicit -fstack-check @gol > -fstack-limit-register=@var{reg} -fstack-limit-symbol=@var{sym} @gol > @@ -15532,6 +15533,24 @@ which functions and calls should be skipped from instrumentation > Currently the x86 GNU/Linux target provides an implementation based > on Intel Control-flow Enforcement Technology (CET). > > +@item -fharden-compares > +@opindex fharden-compares > +For every logical test that survives gimple optimizations and is > +@emph{not} the condition in a conditional branch (for example, > +conditions tested for conditional moves, or to store in boolean > +variables), emit extra code to compute and verify the reversed > +condition, and to call @code{__builtin_trap} if the results do not > +match. Use with @samp{-fharden-conditional-branches} to cover all > +conditionals. > + > +@item -fharden-conditional-branches > +@opindex fharden-conditional-branches > +For every non-vectorized conditional branch that survives gimple > +optimizations, emit extra code to compute and verify the reversed > +condition, and to call @code{__builtin_trap} if the result is > +unexpected. Use with @samp{-fharden-compares} to cover all > +conditionals. > + > @item -fstack-protector > @opindex fstack-protector > Emit extra code to check for buffer overflows, such as stack smashing > diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc > new file mode 100644 > index 0000000000000..8916420d7dfe9 > --- /dev/null > +++ b/gcc/gimple-harden-conditionals.cc > @@ -0,0 +1,439 @@ > +/* Harden conditionals. > + Copyright (C) 2021 Free Software Foundation, Inc. > + Contributed by Alexandre Oliva <oliva@adacore.com>. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify it under > +the terms of the GNU General Public License as published by the Free > +Software Foundation; either version 3, or (at your option) any later > +version. > + > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY > +WARRANTY; without even the implied warranty of MERCHANTABILITY or > +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "backend.h" > +#include "tree.h" > +#include "fold-const.h" > +#include "gimple.h" > +#include "gimplify.h" > +#include "tree-pass.h" > +#include "ssa.h" > +#include "gimple-iterator.h" > +#include "tree-cfg.h" > +#include "basic-block.h" > +#include "cfghooks.h" > +#include "cfgloop.h" > +#include "diagnostic.h" > +#include "intl.h" > + > +namespace { > + > +/* These passes introduces redundant, but reversed conditionals at > + compares, such as those used in conditional branches, and those > + that compute boolean results. This doesn't make much sense for > + abstract CPUs, but this kind of hardening may avoid undesirable > + execution paths on actual CPUs under such attacks as of power > + deprivation. */ > + > +/* Define a pass to harden conditionals other than branches. */ > + > +const pass_data pass_data_harden_compares = { > + GIMPLE_PASS, > + "hardcmp", > + OPTGROUP_NONE, > + TV_NONE, > + PROP_cfg | PROP_ssa, // properties_required > + 0, // properties_provided > + 0, // properties_destroyed > + 0, // properties_start > + TODO_update_ssa > + | TODO_cleanup_cfg > + | TODO_verify_il, // properties_finish > +}; > + > +class pass_harden_compares : public gimple_opt_pass > +{ > +public: > + pass_harden_compares (gcc::context *ctxt) > + : gimple_opt_pass (pass_data_harden_compares, ctxt) > + {} > + opt_pass *clone () { return new pass_harden_compares (m_ctxt); } > + virtual bool gate (function *) { > + return flag_harden_compares; > + } > + virtual unsigned int execute (function *); > +}; > + > +/* Define a pass to harden conditionals in branches. This pass must > + run after the above, otherwise it will re-harden the checks > + introduced by the above. */ > + > +const pass_data pass_data_harden_conditional_branches = { > + GIMPLE_PASS, > + "hardcbr", > + OPTGROUP_NONE, > + TV_NONE, > + PROP_cfg | PROP_ssa, // properties_required > + 0, // properties_provided > + 0, // properties_destroyed > + 0, // properties_start > + TODO_update_ssa > + | TODO_cleanup_cfg > + | TODO_verify_il, // properties_finish > +}; > + > +class pass_harden_conditional_branches : public gimple_opt_pass > +{ > +public: > + pass_harden_conditional_branches (gcc::context *ctxt) > + : gimple_opt_pass (pass_data_harden_conditional_branches, ctxt) > + {} > + opt_pass *clone () { return new pass_harden_conditional_branches (m_ctxt); } > + virtual bool gate (function *) { > + return flag_harden_conditional_branches; > + } > + virtual unsigned int execute (function *); > +}; > + > +} > + > +/* If VAL is an SSA name, return an SSA name holding the same value, > + but without the compiler's knowing that it holds the same value, so > + that uses thereof can't be optimized the way VAL might. Insert > + stmts that initialize it before *GSIP, with LOC. > + > + Otherwise, VAL must be an invariant, returned unchanged. */ > + > +static inline tree > +detach_value (location_t loc, gimple_stmt_iterator *gsip, tree val) > +{ > + if (TREE_CONSTANT (val) || TREE_CODE (val) != SSA_NAME) > + { > + gcc_checking_assert (is_gimple_min_invariant (val)); > + return val; > + } > + > + tree ret = copy_ssa_name (val); > + > + /* Output asm ("" : "=g" (ret) : "0" (val)); */ > + vec<tree, va_gc> *inputs = NULL; > + vec<tree, va_gc> *outputs = NULL; > + vec_safe_push (outputs, > + build_tree_list > + (build_tree_list > + (NULL_TREE, build_string (2, "=g")), > + ret)); > + vec_safe_push (inputs, > + build_tree_list > + (build_tree_list > + (NULL_TREE, build_string (1, "0")), > + val)); > + gasm *detach = gimple_build_asm_vec ("", inputs, outputs, > + NULL, NULL); > + gimple_set_location (detach, loc); > + gsi_insert_before (gsip, detach, GSI_SAME_STMT); > + > + SSA_NAME_DEF_STMT (ret) = detach; > + > + return ret; > +} > + > +/* Build a cond stmt out of COP, LHS, RHS, insert it before *GSIP with > + location LOC. *GSIP must be at the end of a basic block. The succ > + edge out of the block becomes the true or false edge opposite to > + that in FLAGS. Create a new block with a single trap stmt, in the > + cold partition if the function is partitioned,, and a new edge to > + it as the other edge for the cond. */ > + > +static inline void > +insert_check_and_trap (location_t loc, gimple_stmt_iterator *gsip, > + int flags, enum tree_code cop, tree lhs, tree rhs) > +{ > + basic_block chk = gsi_bb (*gsip); > + > + gcond *cond = gimple_build_cond (cop, lhs, rhs, NULL, NULL); > + gimple_set_location (cond, loc); > + gsi_insert_before (gsip, cond, GSI_SAME_STMT); > + > + basic_block trp = create_empty_bb (chk); > + > + gimple_stmt_iterator gsit = gsi_after_labels (trp); > + gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > + gimple_set_location (trap, loc); > + gsi_insert_before (&gsit, trap, GSI_SAME_STMT); > + > + if (dump_file) > + fprintf (dump_file, > + "Adding reversed compare to block %i, and trap to block %i\n", > + chk->index, trp->index); > + > + if (BB_PARTITION (chk)) > + BB_SET_PARTITION (trp, BB_COLD_PARTITION); > + > + int true_false_flag = flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > + gcc_assert (true_false_flag); > + int neg_true_false_flag = (~flags) & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); > + > + /* Remove the fallthru bit, and set the truth value for the > + preexisting edge and for the newly-created one. In hardcbr, > + FLAGS is taken from the edge of the original cond expr that we're > + dealing with, so the reversed compare is expected to yield the > + negated result, and the same result calls for a trap. In > + hardcmp, we're comparing the boolean results of the original and > + of the reversed compare, so we're passed FLAGS to trap on > + equality. */ > + single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU; > + single_succ_edge (chk)->flags |= neg_true_false_flag; > + edge e = make_edge (chk, trp, true_false_flag); > + e->goto_locus = loc; > + > + if (dom_info_available_p (CDI_DOMINATORS)) > + set_immediate_dominator (CDI_DOMINATORS, trp, chk); > + if (current_loops) > + add_bb_to_loop (trp, current_loops->tree_root); > +} > + > +/* Split edge E, and insert_check_and_trap (see above) in the > + newly-created block, using detached copies of LHS's and RHS's > + values (see detach_value above) for the COP compare. */ > + > +static inline void > +insert_edge_check_and_trap (location_t loc, edge e, > + enum tree_code cop, tree lhs, tree rhs) > +{ > + int flags = e->flags; > + basic_block src = e->src; > + basic_block dest = e->dest; > + location_t eloc = e->goto_locus; > + > + basic_block chk = split_edge (e); > + e = NULL; > + > + single_pred_edge (chk)->goto_locus = loc; > + single_succ_edge (chk)->goto_locus = eloc; > + > + if (dump_file) > + fprintf (dump_file, > + "Splitting edge %i->%i into block %i\n", > + src->index, dest->index, chk->index); > + > + gimple_stmt_iterator gsik = gsi_after_labels (chk); > + > + bool same_p = (lhs == rhs); > + lhs = detach_value (loc, &gsik, lhs); > + rhs = same_p ? lhs : detach_value (loc, &gsik, rhs); > + > + insert_check_and_trap (loc, &gsik, flags, cop, lhs, rhs); > +} > + > +/* Harden cond stmts at the end of FUN's blocks. */ > + > +unsigned int > +pass_harden_conditional_branches::execute (function *fun) > +{ > + basic_block bb; > + FOR_EACH_BB_REVERSE_FN (bb, fun) > + { > + gimple_stmt_iterator gsi = gsi_last_bb (bb); > + > + if (gsi_end_p (gsi)) > + continue; > + > + gcond *cond = dyn_cast <gcond *> (gsi_stmt (gsi)); > + if (!cond) > + continue; > + > + /* Turn: > + > + if (x op y) goto l1; else goto l2; > + > + into: > + > + if (x op y) goto l1'; else goto l2'; > + l1': if (x' cop y') goto l1'trap; else goto l1; > + l1'trap: __builtin_trap (); > + l2': if (x' cop y') goto l2; else goto l2'trap; > + l2'trap: __builtin_trap (); > + > + where cop is a complementary boolean operation to op; l1', l1'trap, > + l2' and l2'trap are newly-created labels; and x' and y' hold the same > + value as x and y, but in a way that does not enable the compiler to > + optimize the redundant compare away. > + */ > + > + enum tree_code op = gimple_cond_code (cond); > + tree lhs = gimple_cond_lhs (cond); > + tree rhs = gimple_cond_rhs (cond); > + location_t loc = gimple_location (cond); > + > + enum tree_code cop = invert_tree_comparison (op, HONOR_NANS (lhs)); > + > + if (cop == ERROR_MARK) > + /* ??? Can we do better? */ > + continue; > + > + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 0), cop, lhs, rhs); > + insert_edge_check_and_trap (loc, EDGE_SUCC (bb, 1), cop, lhs, rhs); > + } > + > + return 0; > +} > + > +/* Instantiate a hardcbr pass. */ > + > +gimple_opt_pass * > +make_pass_harden_conditional_branches (gcc::context *ctxt) > +{ > + return new pass_harden_conditional_branches (ctxt); > +} > + > +/* Harden boolean-yielding compares in FUN. */ > + > +unsigned int > +pass_harden_compares::execute (function *fun) > +{ > + basic_block bb; > + /* Go backwards over BBs and stmts, so that, even if we split the > + block multiple times to insert a cond_expr after each compare we > + find, we remain in the same block, visiting every preexisting > + stmt exactly once, and not visiting newly-added blocks or > + stmts. */ > + FOR_EACH_BB_REVERSE_FN (bb, fun) > + for (gimple_stmt_iterator gsi = gsi_last_bb (bb); > + !gsi_end_p (gsi); gsi_prev (&gsi)) > + { > + gassign *asgn = dyn_cast <gassign *> (gsi_stmt (gsi)); > + if (!asgn) > + continue; > + > + /* Turn: > + > + z = x op y; > + > + into: > + > + z = x op y; > + z' = x' cop y'; > + if (z == z') __builtin_trap (); > + > + where cop is a complementary boolean operation to op; and x' > + and y' hold the same value as x and y, but in a way that does > + not enable the compiler to optimize the redundant compare > + away. > + */ > + > + enum tree_code op = gimple_assign_rhs_code (asgn); > + > + enum tree_code cop; > + > + switch (op) > + { > + case EQ_EXPR: > + case NE_EXPR: > + case GT_EXPR: > + case GE_EXPR: > + case LT_EXPR: > + case LE_EXPR: > + case LTGT_EXPR: > + case UNEQ_EXPR: > + case UNGT_EXPR: > + case UNGE_EXPR: > + case UNLT_EXPR: > + case UNLE_EXPR: > + case ORDERED_EXPR: > + case UNORDERED_EXPR: > + cop = invert_tree_comparison (op, > + HONOR_NANS > + (gimple_assign_rhs1 (asgn))); > + > + if (cop == ERROR_MARK) > + /* ??? Can we do better? */ > + continue; > + > + break; > + > + /* ??? Maybe handle these too? */ > + case TRUTH_NOT_EXPR: > + /* ??? The code below assumes binary ops, it would have to > + be adjusted for TRUTH_NOT_EXPR, since it's unary. */ > + case TRUTH_ANDIF_EXPR: > + case TRUTH_ORIF_EXPR: > + case TRUTH_AND_EXPR: > + case TRUTH_OR_EXPR: > + case TRUTH_XOR_EXPR: > + default: > + continue; > + } > + > + /* These are the operands for the verification. */ > + tree lhs = gimple_assign_lhs (asgn); > + tree op1 = gimple_assign_rhs1 (asgn); > + tree op2 = gimple_assign_rhs2 (asgn); > + location_t loc = gimple_location (asgn); > + > + /* Vector booleans can't be used in conditional branches. ??? > + Can we do better? How to reduce compare and > + reversed-compare result vectors to a single boolean? */ > + if (VECTOR_TYPE_P (TREE_TYPE (op1))) > + continue; > + > + gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE); > + > + tree rhs = copy_ssa_name (lhs); > + > + gimple_stmt_iterator gsi_split = gsi; > + /* Don't separate the original assignment from debug stmts > + that might be associated with it, and arrange to split the > + block after debug stmts, so as to make sure the split block > + won't be debug stmts only. */ > + gsi_next_nondebug (&gsi_split); > + > + bool same_p = (op1 == op2); > + op1 = detach_value (loc, &gsi_split, op1); > + op2 = same_p ? op1 : detach_value (loc, &gsi_split, op2); > + > + gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); > + gimple_set_location (asgnck, loc); > + gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); > + > + /* We wish to insert a cond_expr after the compare, so arrange > + for it to be at the end of a block if it isn't. */ > + if (!gsi_end_p (gsi_split)) > + { > + gsi_prev (&gsi_split); > + split_block (bb, gsi_stmt (gsi_split)); > + gsi_next (&gsi_split); > + gcc_checking_assert (gsi_end_p (gsi_split)); > + > + single_succ_edge (bb)->goto_locus = loc; > + > + if (dump_file) > + fprintf (dump_file, "Splitting block %i\n", bb->index); > + } > + > + gcc_checking_assert (single_succ_p (bb)); > + > + insert_check_and_trap (loc, &gsi_split, EDGE_TRUE_VALUE, > + EQ_EXPR, lhs, rhs); > + } > + > + return 0; > +} > + > +/* Instantiate a hardcmp pass. */ > + > +gimple_opt_pass * > +make_pass_harden_compares (gcc::context *ctxt) > +{ > + return new pass_harden_compares (ctxt); > +} > diff --git a/gcc/passes.def b/gcc/passes.def > index c11c237f6d204..268d8595a401e 100644 > --- a/gcc/passes.def > +++ b/gcc/passes.def > @@ -421,6 +421,8 @@ along with GCC; see the file COPYING3. If not see > NEXT_PASS (pass_lower_resx); > NEXT_PASS (pass_nrv); > NEXT_PASS (pass_gimple_isel); > + NEXT_PASS (pass_harden_conditional_branches); > + NEXT_PASS (pass_harden_compares); > NEXT_PASS (pass_cleanup_cfg_post_optimizing); > NEXT_PASS (pass_warn_function_noreturn); > NEXT_PASS (pass_warn_access); > diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c > new file mode 100644 > index 0000000000000..1ee0b3663443d > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fharden-compares -fdump-tree-hardcmp -ffat-lto-objects" } */ > + > +int > +f (int i, int j) > +{ > + return i < j; > +} > + > +/* { dg-final { scan-tree-dump-times "Splitting block" 1 "hardcmp" } } */ > +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */ > +/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */ > +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */ > +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */ > diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c > new file mode 100644 > index 0000000000000..86de8e155ed38 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fharden-conditional-branches -fdump-tree-hardcbr -ffat-lto-objects" } */ > + > +extern int f1 (void); > +extern int f2 (void); > + > + > +int > +f (int i, int j) > +{ > + return (i < j) ? f1 () : f2 (); > +} > + > +/* { dg-final { scan-tree-dump-times "Splitting edge" 2 "hardcbr" } } */ > +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */ > +/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */ > +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */ > +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */ > diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h > index d379769a94364..e807ad855efd1 100644 > --- a/gcc/tree-pass.h > +++ b/gcc/tree-pass.h > @@ -644,6 +644,9 @@ extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt); > extern gimple_opt_pass *make_pass_gimple_isel (gcc::context *ctxt); > +extern gimple_opt_pass *make_pass_harden_compares (gcc::context *ctxt); > +extern gimple_opt_pass *make_pass_harden_conditional_branches (gcc::context > + *ctxt); > > /* Current optimization pass. */ > extern opt_pass *current_pass; > > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Disinformation flourishes because many people care deeply about injustice > but very few check the facts. Ask me about <https://stallmansupport.org>
On Oct 26, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > OK. Thanks. I've just fixed the ChangeLog entry and pushed it: >> * common.opt (fharden-compares): New. >> (fharden-conditional-branches): New. >> * doc/invoke.texi: Document new options. >> * gimple-harden-conditionals.cc: New. + * Makefile.in (OBJS): Build it. >> * passes.def: Add new passes. >> * tree-pass.h (make_pass_harden_compares): Declare. >> (make_pass_harden_conditional_branches): Declare.
FYI, I'm putting in this follow-up tweak to the GNAT manual.
Implied compares in Ada Harded Conditionals documentation
From: Alexandre Oliva <oliva@adacore.com>
Improve the wording on optimizations that prevent compare hardening,
so as to also cover cases in which explicit compares get combined into
operations with implied compares.
for gcc/ada/ChangeLog
* doc/gnat_rm/security_hardening_features.rst: Mention
optimization to operations with implied compares.
---
.../doc/gnat_rm/security_hardening_features.rst | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
index 52240d7e3dd54..cf76938d91d13 100644
--- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst
+++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst
@@ -112,14 +112,15 @@ long after boolean expressions are decomposed into separate compares,
each one turned into either a conditional branch or a compare whose
result is stored in a boolean variable or temporary. Compiler
optimizations, if enabled, may also turn conditional branches into
-stored compares, and vice-versa. Conditionals may also be optimized
+stored compares, and vice-versa, or into operations with implied
+conditionals (e.g. MIN and MAX). Conditionals may also be optimized
out entirely, if their value can be determined at compile time, and
occasionally multiple compares can be combined into one.
It is thus difficult to predict which of these two options will affect
a specific compare operation expressed in source code. Using both
-options ensures that every compare that is not optimized out will be
-hardened.
+options ensures that every compare that is neither optimized out nor
+optimized into implied conditionals will be hardened.
The addition of reversed compares can be observed by enabling the dump
files of the corresponding passes, through command line options
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 64252697573a7..7209ed117d09d 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1389,6 +1389,7 @@ OBJS = \ gimple-if-to-switch.o \ gimple-iterator.o \ gimple-fold.o \ + gimple-harden-conditionals.o \ gimple-laddress.o \ gimple-loop-interchange.o \ gimple-loop-jam.o \ diff --git a/gcc/ada/doc/gnat_rm/security_hardening_features.rst b/gcc/ada/doc/gnat_rm/security_hardening_features.rst index 1c46e3a4c7b88..52240d7e3dd54 100644 --- a/gcc/ada/doc/gnat_rm/security_hardening_features.rst +++ b/gcc/ada/doc/gnat_rm/security_hardening_features.rst @@ -87,3 +87,43 @@ types and subtypes, may be silently ignored. Specifically, it is not currently recommended to rely on any effects this pragma might be expected to have when calling subprograms through access-to-subprogram variables. + + +.. Hardened Conditionals: + +Hardened Conditionals +===================== + +GNAT can harden conditionals to protect against control flow attacks. + +This is accomplished by two complementary transformations, each +activated by a separate command-line option. + +The option *-fharden-compares* enables hardening of compares that +compute results stored in variables, adding verification that the +reversed compare yields the opposite result. + +The option *-fharden-conditional-branches* enables hardening of +compares that guard conditional branches, adding verification of the +reversed compare to both execution paths. + +These transformations are introduced late in the compilation pipeline, +long after boolean expressions are decomposed into separate compares, +each one turned into either a conditional branch or a compare whose +result is stored in a boolean variable or temporary. Compiler +optimizations, if enabled, may also turn conditional branches into +stored compares, and vice-versa. Conditionals may also be optimized +out entirely, if their value can be determined at compile time, and +occasionally multiple compares can be combined into one. + +It is thus difficult to predict which of these two options will affect +a specific compare operation expressed in source code. Using both +options ensures that every compare that is not optimized out will be +hardened. + +The addition of reversed compares can be observed by enabling the dump +files of the corresponding passes, through command line options +*-fdump-tree-hardcmp* and *-fdump-tree-hardcbr*, respectively. + +They are separate options, however, because of the significantly +different performance impact of the hardening transformations. diff --git a/gcc/common.opt b/gcc/common.opt index e867055fc000d..89f2e6da6e56e 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1719,6 +1719,14 @@ fguess-branch-probability Common Var(flag_guess_branch_prob) Optimization Enable guessing of branch probabilities. +fharden-compares +Common Var(flag_harden_compares) Optimization +Harden conditionals not used in branches, checking reversed conditions. + +fharden-conditional-branches +Common Var(flag_harden_conditional_branches) Optimization +Harden conditional branches by checking reversed conditions. + ; Nonzero means ignore `#ident' directives. 0 means handle them. ; Generate position-independent code for executables if possible ; On SVR4 targets, it also controls whether or not to emit a diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 281dd10fff96c..5ed6a55f729e1 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -595,6 +595,7 @@ Objective-C and Objective-C++ Dialects}. -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol -fsanitize-undefined-trap-on-error -fbounds-check @gol -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol +-fharden-compares -fharden-conditional-branches @gol -fstack-protector -fstack-protector-all -fstack-protector-strong @gol -fstack-protector-explicit -fstack-check @gol -fstack-limit-register=@var{reg} -fstack-limit-symbol=@var{sym} @gol @@ -15491,6 +15492,24 @@ which functions and calls should be skipped from instrumentation Currently the x86 GNU/Linux target provides an implementation based on Intel Control-flow Enforcement Technology (CET). +@item -fharden-compares +@opindex fharden-compares +For every logical test that survives gimple optimizations and is +@emph{not} the condition in a conditional branch (for example, +conditions tested for conditional moves, or to store in boolean +variables), emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the results do not +match. Use with @samp{-fharden-conditional-branches} to cover all +conditionals. + +@item -fharden-conditional-branches +@opindex fharden-conditional-branches +For every non-vectorized conditional branch that survives gimple +optimizations, emit extra code to compute and verify the reversed +condition, and to call @code{__builtin_trap} if the result is +unexpected. Use with @samp{-fharden-compares} to cover all +conditionals. + @item -fstack-protector @opindex fstack-protector Emit extra code to check for buffer overflows, such as stack smashing diff --git a/gcc/gimple-harden-conditionals.cc b/gcc/gimple-harden-conditionals.cc new file mode 100644 index 0000000000000..08187b21ebddf --- /dev/null +++ b/gcc/gimple-harden-conditionals.cc @@ -0,0 +1,379 @@ +/* Harden conditionals. + Copyright (C) 2021 Free Software Foundation, Inc. + Contributed by Alexandre Oliva <oliva@adacore.com>. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "backend.h" +#include "tree.h" +#include "fold-const.h" +#include "gimple.h" +#include "gimplify.h" +#include "tree-pass.h" +#include "ssa.h" +#include "gimple-iterator.h" +#include "tree-cfg.h" +#include "basic-block.h" +#include "cfghooks.h" +#include "cfgloop.h" +#include "diagnostic.h" +#include "intl.h" + +namespace { + +/* This pass introduces redundant, but reversed conditionals at every + compare, be it used for conditional branches, other conditional + operations, or for boolean computation. This doesn't make sense + for abstract CPUs, but this kind of hardening may avoid undesirable + execution paths on CPUs under such attacks as of power + deprivation. */ + +/* Define a pass to harden conditionals other than branches. */ +const pass_data pass_data_harden_compares = { + GIMPLE_PASS, + "hardcmp", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_compares : public gimple_opt_pass +{ +public: + pass_harden_compares (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_compares, ctxt) + {} + opt_pass *clone () { return new pass_harden_compares (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_compares; + } + virtual unsigned int execute (function *); +}; + +/* This pass introduces redundant, but reversed conditionals at every + conditional branch. This doesn't make sense for abstract CPUs, but + this kind of hardening may avoid undesirable execution paths on + CPUs under such attacks as of power deprivation. This pass must + run after the above, otherwise it will re-harden the checks + introduced by the above. */ + +/* Define a pass to harden conditionals in branches. */ +const pass_data pass_data_harden_conditional_branches = { + GIMPLE_PASS, + "hardcbr", + OPTGROUP_NONE, + TV_NONE, + PROP_cfg | PROP_ssa, // properties_required + 0, // properties_provided + 0, // properties_destroyed + 0, // properties_start + TODO_update_ssa + | TODO_cleanup_cfg + | TODO_verify_il, // properties_finish +}; + +class pass_harden_conditional_branches : public gimple_opt_pass +{ +public: + pass_harden_conditional_branches (gcc::context *ctxt) + : gimple_opt_pass (pass_data_harden_conditional_branches, ctxt) + {} + opt_pass *clone () { return new pass_harden_conditional_branches (m_ctxt); } + virtual bool gate (function *) { + return flag_harden_conditional_branches; + } + virtual unsigned int execute (function *); +}; + +} + +static inline tree +detach_value (gimple_stmt_iterator *gsip, tree val) +{ + if (TREE_CONSTANT (val) || TREE_CODE (val) != SSA_NAME) + { + gcc_checking_assert (is_gimple_min_invariant (val)); + return val; + } + + tree ret = copy_ssa_name (val); + + vec<tree, va_gc> *inputs = NULL; + vec<tree, va_gc> *outputs = NULL; + vec_safe_push (outputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (2, "=g")), + ret)); + vec_safe_push (inputs, + build_tree_list + (build_tree_list + (NULL_TREE, build_string (1, "0")), + val)); + gasm *detach = gimple_build_asm_vec ("", inputs, outputs, + NULL, NULL); + gsi_insert_before (gsip, detach, GSI_SAME_STMT); + + SSA_NAME_DEF_STMT (ret) = detach; + + return ret; +} + +static inline void +insert_check_and_trap (gimple_stmt_iterator *gsip, int flags, + enum tree_code cop, tree lhs, tree rhs) +{ + basic_block chk = gsi_bb (*gsip); + + gcond *cond = gimple_build_cond (cop, lhs, rhs, NULL, NULL); + gsi_insert_before (gsip, cond, GSI_SAME_STMT); + + basic_block trp = create_empty_bb (chk); + + gimple_stmt_iterator gsit = gsi_after_labels (trp); + gcall *trap = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + gsi_insert_before (&gsit, trap, GSI_SAME_STMT); + + if (dump_file) + fprintf (dump_file, + "Adding reversed compare to block %i, and trap to block %i\n", + chk->index, trp->index); + + if (BB_PARTITION (chk)) + BB_SET_PARTITION (trp, BB_COLD_PARTITION); + + int true_false_flag = flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + gcc_assert (true_false_flag); + int neg_true_false_flag = (~flags) & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE); + + single_succ_edge (chk)->flags &= ~EDGE_FALLTHRU; + single_succ_edge (chk)->flags |= neg_true_false_flag; + make_edge (chk, trp, true_false_flag); + + if (dom_info_available_p (CDI_DOMINATORS)) + set_immediate_dominator (CDI_DOMINATORS, trp, chk); + if (current_loops) + add_bb_to_loop (trp, current_loops->tree_root); +} + +static inline void +insert_edge_check_and_trap (edge e, enum tree_code cop, tree lhs, tree rhs) +{ + int flags = e->flags; + basic_block src = e->src; + basic_block dest = e->dest; + + basic_block chk = split_edge (e); + e = NULL; + + if (dump_file) + fprintf (dump_file, + "Splitting edge %i->%i into block %i\n", + src->index, dest->index, chk->index); + + gimple_stmt_iterator gsik = gsi_after_labels (chk); + + bool same_p = (lhs == rhs); + lhs = detach_value (&gsik, lhs); + rhs = same_p ? lhs : detach_value (&gsik, rhs); + + insert_check_and_trap (&gsik, flags, cop, lhs, rhs); +} + +unsigned int +pass_harden_conditional_branches::execute (function *fun) +{ + basic_block bb; + FOR_EACH_BB_REVERSE_FN (bb, fun) + { + gimple_stmt_iterator gsi = gsi_last_bb (bb); + + if (gsi_end_p (gsi)) + continue; + + gcond *cond = dyn_cast <gcond *> (gsi_stmt (gsi)); + if (!cond) + continue; + + /* Turn: + + if (x op y) goto l1; else goto l2; + + into: + + if (x op y) goto l1'; else goto l2'; + l1': if (x' cop y') goto l1'trap; else goto l1; + l1'trap: __builtin_trap (); + l2': if (x' cop y') goto l2; else goto l2'trap; + l2'trap: __builtin_trap (); + + where cop is a complementary boolean operation to op; l1', l1'trap, + l2' and l2'trap are newly-created labels; and x' and y' hold the same + value as x and y, but in a way that does not enable the compiler to + optimize the redundant compare away. + */ + + enum tree_code op = gimple_cond_code (cond); + tree lhs = gimple_cond_lhs (cond); + tree rhs = gimple_cond_rhs (cond); + + enum tree_code cop = invert_tree_comparison (op, HONOR_NANS (lhs)); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + insert_edge_check_and_trap (EDGE_SUCC (bb, 0), cop, lhs, rhs); + insert_edge_check_and_trap (EDGE_SUCC (bb, 1), cop, lhs, rhs); + } + + return 0; +} + +gimple_opt_pass * +make_pass_harden_conditional_branches (gcc::context *ctxt) +{ + return new pass_harden_conditional_branches (ctxt); +} + +unsigned int +pass_harden_compares::execute (function *fun) +{ + basic_block bb; + FOR_EACH_BB_REVERSE_FN (bb, fun) + for (gimple_stmt_iterator gsi = gsi_last_bb (bb); + !gsi_end_p (gsi); gsi_prev (&gsi)) + { + gassign *asgn = dyn_cast <gassign *> (gsi_stmt (gsi)); + if (!asgn) + continue; + + /* Turn: + + z = x op y; + + into: + + z = x op y; + z' = x' cop y'; + if (z == z') __builtin_trap (); + + where cop is a complementary boolean operation to op; and x' + and y' hold the same value as x and y, but in a way that does + not enable the compiler to optimize the redundant compare + away. + */ + + enum tree_code op = gimple_assign_rhs_code (asgn); + + enum tree_code cop; + + switch (op) + { + case EQ_EXPR: + case NE_EXPR: + case GT_EXPR: + case GE_EXPR: + case LT_EXPR: + case LE_EXPR: + case LTGT_EXPR: + case UNEQ_EXPR: + case UNGT_EXPR: + case UNGE_EXPR: + case UNLT_EXPR: + case UNLE_EXPR: + case ORDERED_EXPR: + case UNORDERED_EXPR: + cop = invert_tree_comparison (op, + HONOR_NANS + (gimple_assign_rhs1 (asgn))); + + if (cop == ERROR_MARK) + /* ??? Can we do better? */ + continue; + + break; + + /* ??? Maybe handle these too? */ + case TRUTH_NOT_EXPR: /* Unary! */ + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_XOR_EXPR: + default: + continue; + } + + /* These are the operands for the verification. */ + tree lhs = gimple_assign_lhs (asgn); + tree op1 = gimple_assign_rhs1 (asgn); + tree op2 = gimple_assign_rhs2 (asgn); + + /* Vector booleans can't be used in conditional branches. + ??? Can we do better? */ + if (VECTOR_TYPE_P (TREE_TYPE (op1))) + continue; + + gcc_checking_assert (TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE); + + tree rhs = copy_ssa_name (lhs); + + gimple_stmt_iterator gsi_split = gsi; + gsi_next (&gsi_split); + + bool same_p = (op1 == op2); + op1 = detach_value (&gsi_split, op1); + op2 = same_p ? op1 : detach_value (&gsi_split, op2); + + gassign *asgnck = gimple_build_assign (rhs, cop, op1, op2); + gsi_insert_before (&gsi_split, asgnck, GSI_SAME_STMT); + + if (!gsi_end_p (gsi_split)) + { + gsi_prev (&gsi_split); + split_block (bb, gsi_stmt (gsi_split)); + gsi_next (&gsi_split); + gcc_checking_assert (gsi_end_p (gsi_split)); + + if (dump_file) + fprintf (dump_file, "Splitting block %i\n", bb->index); + } + + gcc_checking_assert (single_succ_p (bb)); + + insert_check_and_trap (&gsi_split, EDGE_TRUE_VALUE, + EQ_EXPR, lhs, rhs); + } + + return 0; +} + +gimple_opt_pass * +make_pass_harden_compares (gcc::context *ctxt) +{ + return new pass_harden_compares (ctxt); +} diff --git a/gcc/passes.def b/gcc/passes.def index 8e4638d20edac..2d81086df1d39 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -421,6 +421,8 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_lower_resx); NEXT_PASS (pass_nrv); NEXT_PASS (pass_gimple_isel); + NEXT_PASS (pass_harden_conditional_branches); + NEXT_PASS (pass_harden_compares); NEXT_PASS (pass_cleanup_cfg_post_optimizing); NEXT_PASS (pass_warn_function_noreturn); NEXT_PASS (pass_warn_access); diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c b/gcc/testsuite/c-c++-common/torture/harden-comp.c new file mode 100644 index 0000000000000..1ee0b3663443d --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-compares -fdump-tree-hardcmp -ffat-lto-objects" } */ + +int +f (int i, int j) +{ + return i < j; +} + +/* { dg-final { scan-tree-dump-times "Splitting block" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */ +/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 "hardcmp" } } */ diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c b/gcc/testsuite/c-c++-common/torture/harden-cond.c new file mode 100644 index 0000000000000..86de8e155ed38 --- /dev/null +++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-fharden-conditional-branches -fdump-tree-hardcbr -ffat-lto-objects" } */ + +extern int f1 (void); +extern int f2 (void); + + +int +f (int i, int j) +{ + return (i < j) ? f1 () : f2 (); +} + +/* { dg-final { scan-tree-dump-times "Splitting edge" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */ +/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 "hardcbr" } } */ diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 9d19228c385f7..7eae2f26f5195 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -643,6 +643,9 @@ extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt); extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt); extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt); extern gimple_opt_pass *make_pass_gimple_isel (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_compares (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_harden_conditional_branches (gcc::context + *ctxt); /* Current optimization pass. */ extern opt_pass *current_pass;