[BOLT] Gadget scanner: detect signing oracles

Implement the detection of signing oracles. In this patch, a signing
oracle is defined as a sign instruction that accepts a "non-protected"
pointer, but for a slightly different definition of "non-protected"
compared to control flow instructions.

A second BitVector named TrustedRegs is added to the register state
computed by the data-flow analysis. The difference between a
"safe-to-dereference" and a "trusted" register states is that to make
an unsafe register trusted by authentication, one has to make sure
that the authentication succeeded. For example, on AArch64 without
FEAT_PAuth2 and FEAT_EPAC, an authentication instruction produces an
invalid pointer on failure, so that subsequent memory access triggers
an error, but re-signing such pointer would "fix" the signature.

Note that while a separate "trusted" register state may be redundant
depending on the specific semantics of auth and sign operations, it is
still important to check signing operations: while code like this

    resign:
      autda x0, x1
      pacda x0, x2
      ret

is probably safe provided `autda` generates an error on authentication
failure, this function

    sign_anything:
      pacda x0, x1
      ret

is inherently unsafe.

Author: atrosinenko

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -49,6 +49,7 @@ class MCSymbol;
 class raw_ostream;
 
 namespace bolt {
+class BinaryBasicBlock;
 class BinaryFunction;
 
 /// Different types of indirect branches encountered during disassembly.
@@ -572,6 +573,11 @@ class MCPlusBuilder {
     return false;
   }
 
+  virtual MCPhysReg getSignedReg(const MCInst &Inst) const {
+    llvm_unreachable("not implemented");
+    return getNoRegister();
+  }
+
   virtual ErrorOr<MCPhysReg> getRegUsedAsRetDest(const MCInst &Inst) const {
     llvm_unreachable("not implemented");
     return getNoRegister();
@@ -622,6 +628,40 @@ class MCPlusBuilder {
     return std::make_pair(getNoRegister(), getNoRegister());
   }
 
+  /// Analyzes if a pointer is checked to be valid by the end of BB.
+  ///
+  /// It is possible for pointer authentication instructions not to terminate
+  /// the program abnormally on authentication failure and return some *invalid
+  /// pointer* instead (like it is done on AArch64 when FEAT_FPAC is not
+  /// implemented). This might be enough to crash on invalid memory access
+  /// when the pointer is later used as the destination of load/store or branch
+  /// instruction. On the other hand, when the pointer is not used right away,
+  /// it may be important for the compiler to check the address explicitly not
+  /// to introduce signing or authentication oracle.
+  ///
+  /// If this function returns a (Reg, Inst) pair, then it is known that in any
+  /// successor of BB either
+  /// * Reg is trusted, provided it was safe-to-dereference before Inst, or
+  /// * the program is terminated abnormally without introducing any signing
+  ///   or authentication oracles
+  virtual std::optional<std::pair<MCPhysReg, MCInst *>>
+  getAuthCheckedReg(BinaryBasicBlock &BB) const {
+    llvm_unreachable("not implemented");
+    return std::nullopt;
+  }
+
+  /// Returns the register that is checked to be authenticated successfully.
+  ///
+  /// If the returned register was safe-to-dereference before execution of Inst,
+  /// it becomes trusted afterward (if MayOverwrite is false) or at least does
+  /// not escape in a way usable as an authentication oracle (if MayOverwrite
+  /// is true).
+  virtual MCPhysReg getAuthCheckedReg(const MCInst &Inst,
+                                      bool MayOverwrite) const {
+    llvm_unreachable("not implemented");
+    return getNoRegister();
+  }
+
   virtual bool isTerminator(const MCInst &Inst) const;
 
   virtual bool isNoop(const MCInst &Inst) const {

bolt/lib/Passes/PAuthGadgetScanner.cpp

@@ -173,10 +173,24 @@ class TrackedRegisters {
 ///   X30 is safe-to-dereference - the state computed for sub- and
 ///   super-registers is not inspected.
 struct State {
-  /// A BitVector containing the registers that are either safe at function
-  /// entry and were not clobbered yet, or those not clobbered since being
-  /// authenticated.
+  /// A BitVector containing the registers that are either authenticated
+  /// (assuming failed authentication is permitted to produce an invalid
+  /// address, provided it generates an error on memory access) or whose
+  /// value is known not to be attacker-controlled under Pointer Authentication
+  /// threat model. The registers in this set are either
+  /// * not clobbered since being authenticated, or
+  /// * trusted at function entry and were not clobbered yet, or
+  /// * contain a safely materialized address.
   BitVector SafeToDerefRegs;
+  /// A BitVector containing the registers that are either authenticated
+  /// *successfully* or whose value is known not to be attacker-controlled
+  /// under Pointer Authentication threat model.
+  /// The registers in this set are either
+  /// * authenticated and then checked to be authenticated successfully
+  ///   (and not clobbered since then), or
+  /// * trusted at function entry and were not clobbered yet, or
+  /// * contain a safely materialized address.
+  BitVector TrustedRegs;
   /// A vector of sets, only used in the second data flow run.
   /// Each element in the vector represents one of the registers for which we
   /// track the set of last instructions that wrote to this register. For
@@ -189,7 +203,8 @@ struct State {
   State() {}
 
   State(unsigned NumRegs, unsigned NumRegsToTrack)
-      : SafeToDerefRegs(NumRegs), LastInstWritingReg(NumRegsToTrack) {}
+      : SafeToDerefRegs(NumRegs), TrustedRegs(NumRegs),
+        LastInstWritingReg(NumRegsToTrack) {}
 
   State &merge(const State &StateIn) {
     if (StateIn.empty())
@@ -198,6 +213,7 @@ struct State {
       return (*this = StateIn);
 
     SafeToDerefRegs &= StateIn.SafeToDerefRegs;
+    TrustedRegs &= StateIn.TrustedRegs;
     for (unsigned I = 0; I < LastInstWritingReg.size(); ++I)
       for (const MCInst *J : StateIn.LastInstWritingReg[I])
         LastInstWritingReg[I].insert(J);
@@ -210,6 +226,7 @@ struct State {
 
   bool operator==(const State &RHS) const {
     return SafeToDerefRegs == RHS.SafeToDerefRegs &&
+           TrustedRegs == RHS.TrustedRegs &&
            LastInstWritingReg == RHS.LastInstWritingReg;
   }
   bool operator!=(const State &RHS) const { return !((*this) == RHS); }
@@ -234,6 +251,7 @@ raw_ostream &operator<<(raw_ostream &OS, const State &S) {
     OS << "empty";
   } else {
     OS << "SafeToDerefRegs: " << S.SafeToDerefRegs << ", ";
+    OS << "TrustedRegs: " << S.TrustedRegs << ", ";
     printLastInsts(OS, S.LastInstWritingReg);
   }
   OS << ">";
@@ -254,18 +272,22 @@ void PacStatePrinter::print(raw_ostream &OS, const State &S) const {
   OS << "pacret-state<";
   if (S.empty()) {
     assert(S.SafeToDerefRegs.empty());
+    assert(S.TrustedRegs.empty());
     assert(S.LastInstWritingReg.empty());
     OS << "empty";
   } else {
     OS << "SafeToDerefRegs: ";
     RegStatePrinter.print(OS, S.SafeToDerefRegs);
+    OS << ", TrustedRegs: ";
+    RegStatePrinter.print(OS, S.TrustedRegs);
     OS << ", ";
     printLastInsts(OS, S.LastInstWritingReg);
   }
   OS << ">";
 }
 
-/// Computes which registers are safe to be used by control flow instructions.
+/// Computes which registers are safe to be used by control flow and signing
+/// instructions.
 ///
 /// This is the base class for two implementations: a dataflow-based analysis
 /// which is intended to be used for most functions and a simplified CFG-unaware
@@ -292,6 +314,17 @@ class RegisterSafetyAnalysis {
   /// RegToTrackInstsFor is the set of registers for which the dataflow analysis
   /// must compute which the last set of instructions writing to it are.
   const TrackedRegisters RegsToTrackInstsFor;
+  /// Stores information about the detected instruction sequences emitted to
+  /// check an authenticated pointer. Specifically, if such sequence is detected
+  /// in a basic block, it maps the last instruction of that basic block to
+  /// (CheckedRegister, FirstInstOfTheSequence) pair, see the description of
+  /// MCPlusBuilder::getAuthCheckedReg(BB) method.
+  ///
+  /// As the detection of such sequences requires iterating over the adjacent
+  /// instructions, it should be done before calling computeNext(), which
+  /// operates on separate instructions.
+  DenseMap<const MCInst *, std::pair<MCPhysReg, const MCInst *>>
+      CheckerSequenceInfo;
 
   SmallPtrSet<const MCInst *, 4> &lastWritingInsts(State &S,
                                                    MCPhysReg Reg) const {
@@ -306,8 +339,10 @@ class RegisterSafetyAnalysis {
 
   State createEntryState() {
     State S(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
-    for (MCPhysReg Reg : BC.MIB->getTrustedLiveInRegs())
-      S.SafeToDerefRegs |= BC.MIB->getAliases(Reg, /*OnlySmaller=*/true);
+    for (MCPhysReg Reg : BC.MIB->getTrustedLiveInRegs()) {
+      S.TrustedRegs |= BC.MIB->getAliases(Reg, /*OnlySmaller=*/true);
+      S.SafeToDerefRegs = S.TrustedRegs;
+    }
     return S;
   }
 
@@ -354,6 +389,45 @@ class RegisterSafetyAnalysis {
     return Regs;
   }
 
+  // Returns all registers made trusted by this instruction.
+  SmallVector<MCPhysReg> getRegsMadeTrusted(const MCInst &Point,
+                                            const State &Cur) const {
+    SmallVector<MCPhysReg> Regs;
+    const MCPhysReg NoReg = BC.MIB->getNoRegister();
+
+    // An authenticated pointer can be checked, or
+    MCPhysReg CheckedReg =
+        BC.MIB->getAuthCheckedReg(Point, /*MayOverwrite=*/false);
+    if (CheckedReg != NoReg && Cur.SafeToDerefRegs[CheckedReg])
+      Regs.push_back(CheckedReg);
+
+    if (CheckerSequenceInfo.contains(&Point)) {
+      MCPhysReg CheckedReg;
+      const MCInst *FirstCheckerInst;
+      std::tie(CheckedReg, FirstCheckerInst) = CheckerSequenceInfo.at(&Point);
+
+      // FirstCheckerInst should belong to the same basic block, meaning
+      // it was deterministically processed a few steps before this instruction.
+      const State &StateBeforeChecker = getStateBefore(*FirstCheckerInst).get();
+      if (StateBeforeChecker.SafeToDerefRegs[CheckedReg])
+        Regs.push_back(CheckedReg);
+    }
+
+    // ... a safe address can be materialized, or
+    MCPhysReg NewAddrReg = BC.MIB->getMaterializedAddressRegForPtrAuth(Point);
+    if (NewAddrReg != NoReg)
+      Regs.push_back(NewAddrReg);
+
+    // ... an address can be updated in a safe manner, producing the result
+    // which is as trusted as the input address.
+    if (auto DstAndSrc = BC.MIB->analyzeAddressArithmeticsForPtrAuth(Point)) {
+      if (Cur.TrustedRegs[DstAndSrc->second])
+        Regs.push_back(DstAndSrc->first);
+    }
+
+    return Regs;
+  }
+
   State computeNext(const MCInst &Point, const State &Cur) {
     PacStatePrinter P(BC);
     LLVM_DEBUG({
@@ -380,11 +454,34 @@ class RegisterSafetyAnalysis {
     BitVector Clobbered = getClobberedRegs(Point);
     SmallVector<MCPhysReg> NewSafeToDerefRegs =
         getRegsMadeSafeToDeref(Point, Cur);
+    SmallVector<MCPhysReg> NewTrustedRegs = getRegsMadeTrusted(Point, Cur);
+
+    // Ideally, being trusted is a strictly stronger property than being
+    // safe-to-dereference. To simplify the computation of Next state, enforce
+    // this for NewSafeToDerefRegs and NewTrustedRegs. Additionally, this
+    // fixes the properly for "cumulative" register states in tricky cases
+    // like the following:
+    //
+    //    ; LR is safe to dereference here
+    //    mov   x16, x30  ; start of the sequence, LR is s-t-d right before
+    //    xpaclri         ; clobbers LR, LR is not safe anymore
+    //    cmp   x30, x16
+    //    b.eq  1f        ; end of the sequence: LR is marked as trusted
+    //    brk   0x1234
+    //  1:
+    //    ; at this point LR would be marked as trusted,
+    //    ; but not safe-to-dereference
+    //
+    for (auto TrustedReg : NewTrustedRegs) {
+      if (!is_contained(NewSafeToDerefRegs, TrustedReg))
+        NewSafeToDerefRegs.push_back(TrustedReg);
+    }
 
     // Then, compute the state after this instruction is executed.
     State Next = Cur;
 
     Next.SafeToDerefRegs.reset(Clobbered);
+    Next.TrustedRegs.reset(Clobbered);
     // Keep track of this instruction if it writes to any of the registers we
     // need to track that for:
     for (MCPhysReg Reg : RegsToTrackInstsFor.getRegisters())
@@ -405,6 +502,10 @@ class RegisterSafetyAnalysis {
         lastWritingInsts(Next, Reg).clear();
     }
 
+    // Process new trusted registers.
+    for (MCPhysReg TrustedReg : NewTrustedRegs)
+      Next.TrustedRegs |= BC.MIB->getAliases(TrustedReg, /*OnlySmaller=*/true);
+
     LLVM_DEBUG({
       dbgs() << "    .. result: (";
       P.print(dbgs(), Next);
@@ -462,7 +563,22 @@ class DataflowRegisterSafetyAnalysis
     return DFParent::getStateBefore(Inst);
   }
 
-  void run() override { DFParent::run(); }
+  void run() override {
+    for (BinaryBasicBlock &BB : Func) {
+      if (auto CheckerInfo = BC.MIB->getAuthCheckedReg(BB)) {
+        MCInst *LastInstOfChecker = BB.getLastNonPseudoInstr();
+        LLVM_DEBUG({
+          dbgs() << "Found pointer checking sequence in " << BB.getName()
+                 << ":\n";
+          traceReg(BC, "Checked register", CheckerInfo->first);
+          traceInst(BC, "First instruction", *CheckerInfo->second);
+          traceInst(BC, "Last instruction", *LastInstOfChecker);
+        });
+        CheckerSequenceInfo[LastInstOfChecker] = *CheckerInfo;
+      }
+    }
+    DFParent::run();
+  }
 
 protected:
   void preflight() {}
@@ -658,6 +774,26 @@ shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
   return std::make_shared<GadgetReport>(CallKind, Inst, DestReg);
 }
 
+static std::shared_ptr<Report>
+shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
+                          const State &S) {
+  static const GadgetKind SigningOracleKind("signing oracle found");
+
+  MCPhysReg SignedReg = BC.MIB->getSignedReg(Inst);
+  if (SignedReg == BC.MIB->getNoRegister())
+    return nullptr;
+
+  LLVM_DEBUG({
+    traceInst(BC, "Found sign inst", Inst);
+    traceReg(BC, "Signed reg", SignedReg);
+    traceRegMask(BC, "TrustedRegs", S.TrustedRegs);
+  });
+  if (S.TrustedRegs[SignedReg])
+    return nullptr;
+
+  return std::make_shared<GadgetReport>(SigningOracleKind, Inst, SignedReg);
+}
+
 template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
   if (BF.hasCFG()) {
     for (BinaryBasicBlock &BB : BF)
@@ -702,6 +838,8 @@ Analysis::findGadgets(BinaryFunction &BF,
 
     if (auto Report = shouldReportCallGadget(BC, Inst, S))
       Result.Diagnostics.push_back(Report);
+    if (auto Report = shouldReportSigningOracle(BC, Inst, S))
+      Result.Diagnostics.push_back(Report);
   });
   return Result;
 }