[llvm-branch-commits] [llvm] a6bc750 - Retain all jump table range checks when using BTI.

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Aug 7 00:06:49 PDT 2023


Author: Simon Tatham
Date: 2023-08-07T09:04:56+02:00
New Revision: a6bc75058b183a31e79e3c62577058972760e04a

URL: https://github.com/llvm/llvm-project/commit/a6bc75058b183a31e79e3c62577058972760e04a
DIFF: https://github.com/llvm/llvm-project/commit/a6bc75058b183a31e79e3c62577058972760e04a.diff

LOG: Retain all jump table range checks when using BTI.

This modifies the switch-statement generation in SelectionDAGBuilder,
specifically the part that generates case clusters of type CC_JumpTable.

A table-based branch of any kind is at risk of being a JOP gadget, if
it doesn't range-check the offset into the table. For some types of
table branch, such as Arm TBB/TBH, the impact of this is limited
because the value loaded from the table is a relative offset of
limited size; for others, such as a MOV PC,Rn computed branch into a
table of further branch instructions, the gadget is fully general.

When compiling for branch-target enforcement via Arm's BTI system,
many of these table branch idioms use branch instructions of types
that do not require a BTI instruction at the branch destination. This
avoids the need to put a BTI at the start of each case handler,
reducing the number of available gadgets //with// BTIs (i.e. ones
which could be used by a JOP attack in spite of the BTI system). But
without a range check, the use of a non-BTI-requiring branch also
opens up a larger range of followup gadgets for an attacker's use.

A defence against this is to avoid optimising away the range check on
the table offset, even if the compiler believes that no out-of-range
value should be able to reach the table branch. (Rationale: that may
be true for values generated legitimately by the program, but not
those generated maliciously by attackers who have already corrupted
the control flow.)

The effect of keeping the range check and branching to an unreachable
block is that no actual code is generated at that block, so it will
typically point at the end of the function. That may still cause some
kind of unpredictable code execution (such as executing data as code,
or falling through to the next function in the code section), but even
if so, there will only be //one// possible invalid branch target,
rather than giving an attacker the choice of many possibilities.

This defence is enabled only when branch target enforcement is in use.
Without branch target enforcement, the range check is easily bypassed
anyway, by branching in to a location just after it. But with
enforcement, the attacker will have to enter the jump table dispatcher
at the initial BTI and then go through the range check. (Or, if they
don't, it's because they //already// have a general BTI-bypassing
gadget.)

Reviewed By: MaskRay, chill

Differential Revision: https://reviews.llvm.org/D155485

(cherry picked from commit 60b98363c7ed0a549be4d51ee07c32dc2bf47d2f)

Added: 
    llvm/test/CodeGen/Thumb2/jump-table-bti.ll

Modified: 
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 9595da9d0d8a0a..bd4fa40b41c809 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -11310,8 +11310,32 @@ void SelectionDAGBuilder::lowerWorkItem(SwitchWorkListItem W, Value *Cond,
           }
         }
 
-        if (FallthroughUnreachable)
-          JTH->FallthroughUnreachable = true;
+        // If the default clause is unreachable, propagate that knowledge into
+        // JTH->FallthroughUnreachable which will use it to suppress the range
+        // check.
+        //
+        // However, don't do this if we're doing branch target enforcement,
+        // because a table branch _without_ a range check can be a tempting JOP
+        // gadget - out-of-bounds inputs that are impossible in correct
+        // execution become possible again if an attacker can influence the
+        // control flow. So if an attacker doesn't already have a BTI bypass
+        // available, we don't want them to be able to get one out of this
+        // table branch.
+        if (FallthroughUnreachable) {
+          Function &CurFunc = CurMF->getFunction();
+          bool HasBranchTargetEnforcement = false;
+          if (CurFunc.hasFnAttribute("branch-target-enforcement")) {
+            HasBranchTargetEnforcement =
+                CurFunc.getFnAttribute("branch-target-enforcement")
+                    .getValueAsBool();
+          } else {
+            HasBranchTargetEnforcement =
+                CurMF->getMMI().getModule()->getModuleFlag(
+                    "branch-target-enforcement");
+          }
+          if (!HasBranchTargetEnforcement)
+            JTH->FallthroughUnreachable = true;
+        }
 
         if (!JTH->FallthroughUnreachable)
           addSuccessorWithProb(CurMBB, Fallthrough, FallthroughProb);

diff  --git a/llvm/test/CodeGen/Thumb2/jump-table-bti.ll b/llvm/test/CodeGen/Thumb2/jump-table-bti.ll
new file mode 100644
index 00000000000000..b54bffc837e0e5
--- /dev/null
+++ b/llvm/test/CodeGen/Thumb2/jump-table-bti.ll
@@ -0,0 +1,129 @@
+;; When BTI is enabled, keep the range check for a jump table for hardening,
+;; even with an unreachable default.
+;;
+;; We check with and without the branch-target-enforcement module attribute,
+;; and in each case, try overriding it with the opposite function attribute.
+;; Expect to see a range check whenever there is BTI, and not where there
+;; isn't.
+
+; RUN: sed s/SPACE/4/ %s | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-TBB
+; RUN: sed s/SPACE/4/ %s | sed '/test_jumptable/s/{/#0 {/' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-TBB
+; RUN: sed s/SPACE/4/ %s | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-TBB
+; RUN: sed s/SPACE/4/ %s | sed '/test_jumptable/s/{/#1 {/' | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-TBB
+
+; RUN: sed s/SPACE/400/ %s | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-TBH
+; RUN: sed s/SPACE/400/ %s | sed '/test_jumptable/s/{/#0 {/' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-TBH
+; RUN: sed s/SPACE/400/ %s | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-TBH
+; RUN: sed s/SPACE/400/ %s | sed '/test_jumptable/s/{/#1 {/' | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-TBH
+
+; RUN: sed s/SPACE/400000/ %s | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-MOV
+; RUN: sed s/SPACE/400000/ %s | sed '/test_jumptable/s/{/#0 {/' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-MOV
+; RUN: sed s/SPACE/400000/ %s | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=NOBTI-MOV
+; RUN: sed s/SPACE/400000/ %s | sed '/test_jumptable/s/{/#1 {/' | sed '/^..for-non-bti-build-sed-will-delete-everything-after-this-line/q' | llc -mtriple=thumbv8.1m.main-linux-gnu -mattr=+pacbti -o - | FileCheck %s --check-prefix=BTI-MOV
+
+declare i32 @llvm.arm.space(i32, i32)
+
+attributes #0 = { "branch-target-enforcement"="false" }
+attributes #1 = { "branch-target-enforcement"="true"  }
+
+define ptr @test_jumptable(ptr %src, ptr %dst) {
+entry:
+  %sw = load i32, ptr %src, align 4
+  %src.postinc = getelementptr inbounds i32, ptr %src, i32 1
+  switch i32 %sw, label %default [
+    i32 0, label %sw.0
+    i32 1, label %sw.1
+    i32 2, label %sw.2
+    i32 3, label %sw.3
+  ]
+
+sw.0:
+  %store.0 = call i32 @llvm.arm.space(i32 SPACE, i32 14142)
+  store i32 %store.0, ptr %dst, align 4
+  br label %exit
+
+sw.1:
+  %store.1 = call i32 @llvm.arm.space(i32 SPACE, i32 31415)
+  %dst.1 = getelementptr inbounds i32, ptr %dst, i32 1
+  store i32 %store.1, ptr %dst.1, align 4
+  br label %exit
+
+sw.2:
+  %store.2 = call i32 @llvm.arm.space(i32 SPACE, i32 27182)
+  %dst.2 = getelementptr inbounds i32, ptr %dst, i32 2
+  store i32 %store.2, ptr %dst.2, align 4
+  br label %exit
+
+sw.3:
+  %store.3 = call i32 @llvm.arm.space(i32 SPACE, i32 16180)
+  %dst.3 = getelementptr inbounds i32, ptr %dst, i32 3
+  store i32 %store.3, ptr %dst.3, align 4
+  br label %exit
+
+default:
+  unreachable
+
+exit:
+  ret ptr %src.postinc
+}
+
+; NOBTI-TBB:      test_jumptable:
+; NOBTI-TBB-NEXT:         .fnstart
+; NOBTI-TBB-NEXT: @ %bb
+; NOBTI-TBB-NEXT:         ldr     [[INDEX:r[0-9]+]], [r0], #4
+; NOBTI-TBB-NEXT: .LCPI
+; NOBTI-TBB-NEXT:         tbb     [pc, [[INDEX]]]
+
+; BTI-TBB:        test_jumptable:
+; BTI-TBB-NEXT:           .fnstart
+; BTI-TBB-NEXT:   @ %bb
+; BTI-TBB-NEXT:           bti
+; BTI-TBB-NEXT:           ldr     [[INDEX:r[0-9]+]], [r0], #4
+; BTI-TBB-NEXT:           cmp     [[INDEX]], #3
+; BTI-TBB-NEXT:           bhi     .LBB
+; BTI-TBB-NEXT:   @ %bb
+; BTI-TBB-NEXT:   .LCPI
+; BTI-TBB-NEXT:           tbb     [pc, [[INDEX]]]
+
+; NOBTI-TBH:      test_jumptable:
+; NOBTI-TBH-NEXT:         .fnstart
+; NOBTI-TBH-NEXT: @ %bb
+; NOBTI-TBH-NEXT:         ldr     [[INDEX:r[0-9]+]], [r0], #4
+; NOBTI-TBH-NEXT: .LCPI
+; NOBTI-TBH-NEXT:         tbh     [pc, [[INDEX]], lsl #1]
+
+; BTI-TBH:        test_jumptable:
+; BTI-TBH-NEXT:           .fnstart
+; BTI-TBH-NEXT:   @ %bb
+; BTI-TBH-NEXT:           bti
+; BTI-TBH-NEXT:           ldr     [[INDEX:r[0-9]+]], [r0], #4
+; BTI-TBH-NEXT:           cmp     [[INDEX]], #3
+; BTI-TBH-NEXT:           bhi.w   .LBB
+; BTI-TBH-NEXT:   @ %bb
+; BTI-TBH-NEXT:   .LCPI
+; BTI-TBH-NEXT:           tbh     [pc, [[INDEX]], lsl #1]
+
+; NOBTI-MOV:      test_jumptable:
+; NOBTI-MOV-NEXT:         .fnstart
+; NOBTI-MOV-NEXT: @ %bb
+; NOBTI-MOV-NEXT:         ldr     [[INDEX:r[0-9]+]], [r0], #4
+; NOBTI-MOV-NEXT:         adr.w   [[ADDR:r[0-9]+]], .LJTI
+; NOBTI-MOV-NEXT:         add.w   [[ADDR]], [[ADDR]], [[INDEX]], lsl #2
+; NOBTI-MOV-NEXT:         mov     pc, [[ADDR]]
+
+; BTI-MOV:        test_jumptable:
+; BTI-MOV-NEXT:           .fnstart
+; BTI-MOV-NEXT:   @ %bb
+; BTI-MOV-NEXT:           bti
+; BTI-MOV-NEXT:           ldr     [[INDEX:r[0-9]+]], [r0], #4
+; BTI-MOV-NEXT:           cmp     [[INDEX]], #3
+; BTI-MOV-NEXT:           bls     .LBB
+; BTI-MOV-NEXT:           b.w     .LBB
+; BTI-MOV-NEXT:   .LBB
+; BTI-MOV-NEXT:           adr.w   [[ADDR:r[0-9]+]], .LJTI
+; BTI-MOV-NEXT:           add.w   [[ADDR]], [[ADDR]], [[INDEX]], lsl #2
+; BTI-MOV-NEXT:           mov     pc, [[ADDR]]
+
+; for-non-bti-build-sed-will-delete-everything-after-this-line
+!llvm.module.flags = !{!0}
+!0 = !{i32 8, !"branch-target-enforcement", i32 1}


        


More information about the llvm-branch-commits mailing list