Skip to content

[BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG #141824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions bolt/lib/Passes/PAuthGadgetScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,19 +737,14 @@ template <typename StateTy> class CFGUnawareAnalysis {
//
// Then, a function can be split into a number of disjoint contiguous sequences
// of instructions without labels in between. These sequences can be processed
// the same way basic blocks are processed by data-flow analysis, assuming
// pessimistically that all registers are unsafe at the start of each sequence.
// the same way basic blocks are processed by data-flow analysis, with the same
// pessimistic estimation of the initial state at the start of each sequence
// (except the first instruction of the function).
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
public CFGUnawareAnalysis<SrcState> {
using SrcSafetyAnalysis::BC;
BinaryFunction &BF;

/// Creates a state with all registers marked unsafe (not to be confused
/// with empty state).
SrcState createUnsafeState() const {
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
}

public:
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
MCPlusBuilder::AllocatorIdTy AllocId,
Expand All @@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
}

void run() override {
const SrcState DefaultState = computePessimisticState(BF);
SrcState S = createEntryState();
for (auto &I : BF.instrs()) {
MCInst &Inst = I.second;
Expand All @@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
LLVM_DEBUG({
traceInst(BC, "Due to label, resetting the state before", Inst);
});
S = createUnsafeState();
S = DefaultState;
}

// Attach the state *before* this instruction executes.
Expand Down
31 changes: 22 additions & 9 deletions bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
Original file line number Diff line number Diff line change
Expand Up @@ -224,20 +224,33 @@ f_unreachable_instruction:
ret
.size f_unreachable_instruction, .-f_unreachable_instruction

// Expected false positive: without CFG, the state is reset to all-unsafe
// after an unconditional branch.

.globl state_is_reset_after_indirect_branch_nocfg
.type state_is_reset_after_indirect_branch_nocfg,@function
state_is_reset_after_indirect_branch_nocfg:
// CHECK-LABEL: GS-PAUTH: non-protected ret found in function state_is_reset_after_indirect_branch_nocfg, at address
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
// Without CFG, the state is reset at labels, assuming every register that can
// be clobbered in the function was actually clobbered.

.globl lr_untouched_nocfg
.type lr_untouched_nocfg,@function
lr_untouched_nocfg:
// CHECK-NOT: lr_untouched_nocfg
adr x2, 1f
br x2
1:
ret
.size lr_untouched_nocfg, .-lr_untouched_nocfg

.globl lr_clobbered_nocfg
.type lr_clobbered_nocfg,@function
lr_clobbered_nocfg:
// CHECK-LABEL: GS-PAUTH: non-protected ret found in function lr_clobbered_nocfg, at address
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
adr x2, 1f
br x2
1:
b 2f
bl g // never executed, but affects the expected worst-case scenario
2:
ret
.size state_is_reset_after_indirect_branch_nocfg, .-state_is_reset_after_indirect_branch_nocfg
.size lr_clobbered_nocfg, .-lr_clobbered_nocfg

/// Now do a basic sanity check on every different Authentication instruction:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,10 +491,6 @@ good_address_arith_multi_bb:
ret
.size good_address_arith_multi_bb, .-good_address_arith_multi_bb

// FIXME: Most *_nocfg test cases contain paciasp+autiasp instructions even if
// LR is not spilled - this is a workaround for RET instructions being
// reported as non-protected, because LR state is reset at every label.

.globl good_ret_nocfg
.type good_ret_nocfg,@function
good_ret_nocfg:
Expand Down Expand Up @@ -541,29 +537,25 @@ good_branch_nocfg:
.type good_load_other_reg_nocfg,@function
good_load_other_reg_nocfg:
// CHECK-NOT: good_load_other_reg_nocfg
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
ldr x2, [x0]

autiasp
ret
.size good_load_other_reg_nocfg, .-good_load_other_reg_nocfg

.globl good_load_same_reg_nocfg
.type good_load_same_reg_nocfg,@function
good_load_same_reg_nocfg:
// CHECK-NOT: good_load_same_reg_nocfg
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
ldr x0, [x0]

autiasp
ret
.size good_load_same_reg_nocfg, .-good_load_same_reg_nocfg

Expand All @@ -575,13 +567,11 @@ bad_unchecked_nocfg:
// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_unchecked_nocfg, at address
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
// CHECK-NEXT: The 0 instructions that leak the affected registers are:
paciasp
adr x2, 1f
br x2
1:
autia x0, x1

autiasp
ret
.size bad_unchecked_nocfg, .-bad_unchecked_nocfg

Expand Down Expand Up @@ -615,15 +605,13 @@ bad_unknown_usage_read_nocfg:
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul x3, x0, x1
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
mul x3, x0, x1
ldr x2, [x0]

autiasp
ret
.size bad_unknown_usage_read_nocfg, .-bad_unknown_usage_read_nocfg

Expand All @@ -634,15 +622,13 @@ bad_unknown_usage_subreg_read_nocfg:
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul w3, w0, w1
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
mul w3, w0, w1
ldr x2, [x0]

autiasp
ret
.size bad_unknown_usage_subreg_read_nocfg, .-bad_unknown_usage_subreg_read_nocfg

Expand All @@ -653,38 +639,33 @@ bad_unknown_usage_update_nocfg:
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
// CHECK-NEXT: 1. {{[0-9a-f]+}}: movk x0, #0x2a, lsl #16
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
movk x0, #42, lsl #16 // does not overwrite x0 completely
ldr x2, [x0]

autiasp
ret
.size bad_unknown_usage_update_nocfg, .-bad_unknown_usage_update_nocfg

.globl good_overwrite_with_constant_nocfg
.type good_overwrite_with_constant_nocfg,@function
good_overwrite_with_constant_nocfg:
// CHECK-NOT: good_overwrite_with_constant_nocfg
paciasp
adr x2, 1f
br x2
1:
autia x0, x1
mov x0, #42

autiasp
ret
.size good_overwrite_with_constant_nocfg, .-good_overwrite_with_constant_nocfg

.globl good_address_arith_nocfg
.type good_address_arith_nocfg,@function
good_address_arith_nocfg:
// CHECK-NOT: good_address_arith_nocfg
paciasp
adr x2, 1f
br x2
1:
Expand All @@ -698,7 +679,6 @@ good_address_arith_nocfg:
mov x1, #0
mov x2, #0

autiasp
ret
.size good_address_arith_nocfg, .-good_address_arith_nocfg

Expand Down
32 changes: 3 additions & 29 deletions bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ nocfg:
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( br x0, src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: >)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: >)
// CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: , TrustedRegs: , Insts: >)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: , TrustedRegs: , Insts: >)
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: >)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: >)
// CHECK-NEXT: After src register safety analysis:
// CHECK-NEXT: Binary Function "nocfg" {
// CHECK-NEXT: Number : 3
Expand All @@ -223,33 +223,7 @@ nocfg:
// PAUTH-NEXT: SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI{{[ \t]*$}}
// CHECK-NEXT: Found RET inst: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: >
// CHECK-NEXT: RetReg: LR
// CHECK-NEXT: SafeToDerefRegs:{{[ \t]*$}}
// CHECK-EMPTY:
// CHECK-NEXT: Running detailed src register safety analysis...
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( adr x0, __ENTRY_nocfg@0x[[ENTRY_ADDR]], src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: [0]()>)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: [0]()>)
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( br x0, src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: [0]()>)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: [0]()>)
// CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: , TrustedRegs: , Insts: [0]()>)
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: , TrustedRegs: , Insts: [0]()>)
// CHECK-NEXT: After detailed src register safety analysis:
// CHECK-NEXT: Binary Function "nocfg" {
// CHECK-NEXT: Number : 3
// ...
// CHECK: Secondary Entry Points : __ENTRY_nocfg@0x[[ENTRY_ADDR]]
// CHECK-NEXT: }
// CHECK-NEXT: .{{[A-Za-z0-9]+}}:
// CHECK-NEXT: 00000000: adr x0, __ENTRY_nocfg@0x[[ENTRY_ADDR]] # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
// CHECK-NEXT: 00000004: br x0 # UNKNOWN CONTROL FLOW # Offset: 4 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
// CHECK-NEXT: __ENTRY_nocfg@0x[[ENTRY_ADDR]] (Entry Point):
// CHECK-NEXT: .{{[A-Za-z0-9]+}}:
// CHECK-NEXT: 00000008: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
// CHECK-NEXT: DWARF CFI Instructions:
// CHECK-NEXT: <empty>
// CHECK-NEXT: End of Function "nocfg"
// CHECK-EMPTY:
// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
// CHECK-NEXT: SafeToDerefRegs: LR W30 W30_HI{{[ \t]*$}}

.globl auth_oracle
.type auth_oracle,@function
Expand Down
Loading
Loading