From a6bc75058b183a31e79e3c62577058972760e04a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 31 Jul 2023 09:09:09 +0100 Subject: [PATCH] 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) --- .../CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 28 ++++- llvm/test/CodeGen/Thumb2/jump-table-bti.ll | 129 +++++++++++++++++++++ 2 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/Thumb2/jump-table-bti.ll diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 9595da9..bd4fa40 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 0000000..b54bffc --- /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} -- 2.7.4