[BOLT] Gadget scanner: account for BRK when searching for auth oracles

[atrosinenko]

An authenticated pointer can be explicitly checked by the compiler via a
sequence of instructions that executes BRK on failure. It is important
to recognize such BRK instruction as checking every register (as it is
expected to immediately trigger an abnormal program termination) to
prevent false positive reports about authentication oracles:

  autia   x2, x3
  autia   x0, x1
  ; neither x0 nor x2 are checked at this point
  eor     x16, x0, x0, lsl #1
  tbz     x16, #62, on_success ; marks x0 as checked
  ; end of BB: for x2 to be checked here, it must be checked in both
  ; successor basic blocks
on_failure:
  brk     0xc470
on_success:
  ; x2 is checked
  ldr     x1, [x2] ; marks x2 as checked

[atrosinenko]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• [BOLT] Gadget scanner: make use of C++17 features and LLVM helpers #141665
• [BOLT] Gadget scanner: optionally assume auth traps on failure #139778
• [BOLT] Gadget scanner: prevent false positives due to jump tables #138884
• [BOLT] Introduce helpers to match MCInsts one at a time (NFC) #138883
• [BOLT] Factor out MCInstReference from gadget scanner (NFC) #138655
• [BOLT] Gadget scanner: account for BRK when searching for auth oracles #137975 👈 (View in Graphite)
• [BOLT] Gadget scanner: detect untrusted LR before tail call #137224
• [BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG #141824
• [BOLT] Gadget scanner: improve handling of unreachable basic blocks #136183
• [BOLT] Gadget scanner: do not crash on debug-printing CFI instructions #136151
• [BOLT] Gadget scanner: detect authentication oracles #135663
• [BOLT] Gadget scanner: clarify MCPlusBuilder callbacks interface #136147
• [BOLT] Gadget scanner: refactor issue reporting #135662
• [BOLT] Gadget scanner: use more appropriate types (NFC) #135661
• [BOLT] Gadget scanner: detect signing oracles #134146
• [BOLT] Gadget scanner: analyze functions without CFG information #133461
• [BOLT] Gadget scanner: refine class names and debug output (NFC) #135073
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Anatoly Trosinenko (atrosinenko)

Changes

An authenticated pointer can be explicitly checked by the compiler via a
sequence of instructions that executes BRK on failure. It is important
to recognize such BRK instruction as checking every register (as it is
expected to immediately trigger an abnormal program termination) to
prevent false positive reports about authentication oracles:

  autia   x2, x3
  autia   x0, x1
  ; neither x0 nor x2 are checked at this point
  eor     x16, x0, x0, lsl #<!-- -->1
  tbz     x16, #<!-- -->62, on_success ; marks x0 as checked
  ; end of BB: for x2 to be checked here, it must be checked in both
  ; successor basic blocks
on_failure:
  brk     0xc470
on_success:
  ; x2 is checked
  ldr     x1, [x2] ; marks x2 as checked

---

Full diff: https://github.com/llvm/llvm-project/pull/137975.diff

6 Files Affected:

• (modified) bolt/include/bolt/Core/MCPlusBuilder.h (+14)
• (modified) bolt/lib/Passes/PAuthGadgetScanner.cpp (+11-2)
• (modified) bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp (+21-3)
• (modified) bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s (+22-22)
• (modified) bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s (+4-5)
• (modified) bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s (+3-3)

diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 83ad70ea97076..b0484964cfbad 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -706,6 +706,20 @@ class MCPlusBuilder {
     return false;
   }
 
+  /// Returns true if Inst is a trap instruction.
+  ///
+  /// Tests if Inst is an instruction that immediately causes an abnormal
+  /// program termination, for example when a security violation is detected
+  /// by a compiler-inserted check.
+  ///
+  /// @note An implementation of this method should likely return false for
+  /// calls to library functions like abort(), as it is possible that the
+  /// execution state is partially attacker-controlled at this point.
+  virtual bool isTrap(const MCInst &Inst) const {
+    llvm_unreachable("not implemented");
+    return false;
+  }
+
   virtual bool isBreakpoint(const MCInst &Inst) const {
     llvm_unreachable("not implemented");
     return false;
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index fab92947f6d67..20d4fdf42f03a 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -1003,6 +1003,15 @@ class DstSafetyAnalysis {
       dbgs() << ")\n";
     });
 
+    // If this instruction terminates the program immediately, no
+    // authentication oracles are possible past this point.
+    if (BC.MIB->isTrap(Point)) {
+      LLVM_DEBUG({ traceInst(BC, "Trap instruction found", Point); });
+      DstState Next(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
+      Next.CannotEscapeUnchecked.set();
+      return Next;
+    }
+
     // If this instruction is reachable by the analysis, a non-empty state will
     // be propagated to it sooner or later. Until then, skip computeNext().
     if (Cur.empty()) {
@@ -1108,8 +1117,8 @@ class DataflowDstSafetyAnalysis
     //
     // A basic block without any successors, on the other hand, can be
     // pessimistically initialized to everything-is-unsafe: this will naturally
-    // handle both return and tail call instructions and is harmless for
-    // internal indirect branch instructions (such as computed gotos).
+    // handle return, trap and tail call instructions. At the same time, it is
+    // harmless for internal indirect branch instructions, like computed gotos.
     if (BB.succ_empty())
       return createUnsafeState();
 
diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
index f3c29e6ee43b9..4d11c5b206eab 100644
--- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
+++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
@@ -386,10 +386,9 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
     // the list of successors of this basic block as appropriate.
 
     // Any of the above code sequences assume the fall-through basic block
-    // is a dead-end BRK instruction (any immediate operand is accepted).
+    // is a dead-end trap instruction.
     const BinaryBasicBlock *BreakBB = BB.getFallthrough();
-    if (!BreakBB || BreakBB->empty() ||
-        BreakBB->front().getOpcode() != AArch64::BRK)
+    if (!BreakBB || BreakBB->empty() || !isTrap(BreakBB->front()))
       return std::nullopt;
 
     // Iterate over the instructions of BB in reverse order, matching opcodes
@@ -1745,6 +1744,25 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
     Inst.addOperand(MCOperand::createImm(0));
   }
 
+  bool isTrap(const MCInst &Inst) const override {
+    if (Inst.getOpcode() != AArch64::BRK)
+      return false;
+    // Only match the immediate values that are likely to indicate this BRK
+    // instruction is emitted to terminate the program immediately and not to
+    // be handled by a SIGTRAP handler, for example.
+    switch (Inst.getOperand(0).getImm()) {
+    case 0xc470:
+    case 0xc471:
+    case 0xc472:
+    case 0xc473:
+      // Explicit Pointer Authentication check failed, see
+      // AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue().
+      return true;
+    default:
+      return false;
+    }
+  }
+
   bool isStorePair(const MCInst &Inst) const {
     const unsigned opcode = Inst.getOpcode();
 
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s b/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
index 3f982ddaf6e38..74f276197923f 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s
@@ -31,7 +31,7 @@ resign_xpaci_good:
         xpaci   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -46,7 +46,7 @@ resign_xpacd_good:
         xpacd   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc473
 1:
         pacda   x0, x2
         ret
@@ -117,7 +117,7 @@ resign_xpaci_unrelated_auth_and_check:
         xpaci   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x10, x2
         ret
@@ -139,7 +139,7 @@ resign_xpaci_wrong_pattern_1:
         xpaci   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -157,7 +157,7 @@ resign_xpaci_wrong_pattern_2:
         xpaci   x0        // x0 instead of x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -174,7 +174,7 @@ resign_xpaci_wrong_pattern_3:
         xpaci   x16
         cmp     x16, x16  // x16 instead of x0
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -191,7 +191,7 @@ resign_xpaci_wrong_pattern_4:
         xpaci   x16
         cmp     x0, x0    // x0 instead of x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -208,7 +208,7 @@ resign_xpaci_wrong_pattern_5:
         mov     x16, x16  // replace xpaci with a no-op instruction
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -228,7 +228,7 @@ resign_xpaclri_good:
         xpaclri
         cmp     x30, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x30, x2
 
@@ -246,7 +246,7 @@ xpaclri_check_keeps_lr_safe:
         xpaclri         // clobbers LR
         cmp     x30, x16
         b.eq    1f
-        brk     0x1234    // marks LR as trusted and safe-to-dereference
+        brk     0xc471    // marks LR as trusted and safe-to-dereference
 1:
         ret             // not reporting non-protected return
         .size xpaclri_check_keeps_lr_safe, .-xpaclri_check_keeps_lr_safe
@@ -265,7 +265,7 @@ xpaclri_check_requires_safe_lr:
         xpaclri
         cmp     x30, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         ret
         .size xpaclri_check_requires_safe_lr, .-xpaclri_check_requires_safe_lr
@@ -283,7 +283,7 @@ resign_xpaclri_wrong_reg:
         xpaclri         // ... but xpaclri still operates on x30
         cmp     x20, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x20, x2
 
@@ -303,7 +303,7 @@ resign_checked_not_authenticated:
         xpaci   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -323,7 +323,7 @@ resign_checked_before_authenticated:
         xpaci   x16
         cmp     x0, x16
         b.eq    1f
-        brk     0x1234
+        brk     0xc471
 1:
         autib   x0, x1
         pacia   x0, x2
@@ -339,7 +339,7 @@ resign_high_bits_tbz_good:
         autib   x0, x1
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -378,7 +378,7 @@ resign_high_bits_tbz_wrong_bit:
         autib   x0, x1
         eor     x16, x0, x0, lsl #1
         tbz     x16, #63, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -393,7 +393,7 @@ resign_high_bits_tbz_wrong_shift_amount:
         autib   x0, x1
         eor     x16, x0, x0, lsl #2
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -408,7 +408,7 @@ resign_high_bits_tbz_wrong_shift_type:
         autib   x0, x1
         eor     x16, x0, x0, lsr #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -423,7 +423,7 @@ resign_high_bits_tbz_wrong_pattern_1:
         autib   x0, x1
         eor     x16, x0, x0, lsl #1
         tbz     x17, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -438,7 +438,7 @@ resign_high_bits_tbz_wrong_pattern_2:
         autib   x0, x1
         eor     x16, x10, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -453,7 +453,7 @@ resign_high_bits_tbz_wrong_pattern_3:
         autib   x0, x1
         eor     x16, x0, x10, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc471
 1:
         pacia   x0, x2
         ret
@@ -648,7 +648,7 @@ many_checked_regs:
         xpacd   x16       // ...
         cmp     x2, x16   // ...
         b.eq    2f        // end of basic block
-        brk     0x1234
+        brk     0xc473
 2:
         pacdza  x0
         pacdza  x1
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
index ecca78c80a8f5..276f11829d249 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s
@@ -79,7 +79,7 @@ good_explicit_check:
         autia   x0, x1
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc470
 1:
         ret
         .size good_explicit_check, .-good_explicit_check
@@ -301,7 +301,7 @@ good_explicit_check_multi_bb:
 1:
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 2f
-        brk     0x1234
+        brk     0xc470
 2:
         cbz     x1, 3f
         nop
@@ -629,8 +629,7 @@ good_address_arith_nocfg:
         .globl  good_explicit_check_unrelated_reg
         .type   good_explicit_check_unrelated_reg,@function
 good_explicit_check_unrelated_reg:
-// CHECK-LABEL: GS-PAUTH: authentication oracle found in function good_explicit_check_unrelated_reg, basic block {{[^,]+}}, at address
-        // FIXME: The below instruction is not an authentication oracle
+// CHECK-NOT: good_explicit_check_unrelated_reg
         autia   x2, x3    // One of possible execution paths after this instruction
                           // ends at BRK below, thus BRK used as a trap instruction
                           // should formally "check everything" not to introduce
@@ -638,7 +637,7 @@ good_explicit_check_unrelated_reg:
         autia   x0, x1
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc470
 1:
         ldr     x4, [x2]  // Right before this instruction X2 is checked - this
                           // should be propagated to the basic block ending with
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
index 9b10879094a03..9eb656639c191 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s
@@ -57,7 +57,7 @@ good_sign_auted_checked_brk:
         autda   x0, x2
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc472
 1:
         pacda   x0, x1
         ret
@@ -351,7 +351,7 @@ good_sign_auted_checked_brk_multi_bb:
 1:
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 2f
-        brk     0x1234
+        brk     0xc472
 2:
         cbz     x4, 3f
         nop
@@ -732,7 +732,7 @@ good_resign_with_increment_brk:
         add     x0, x0, #8
         eor     x16, x0, x0, lsl #1
         tbz     x16, #62, 1f
-        brk     0x1234
+        brk     0xc472
 1:
         mov     x2, x0
         pacda   x2, x1