Skip to content

Commit 54c1b3a

Browse files
atrosinenkoatrosinenko
committed
[BOLT] Gadget scanner: fix LR to be safe in leaf functions without CFG
After a label in a function without CFG information, use a reasonably pessimistic estimation of register state (assume that any register that can be clobbered in this function was actually clobbered) instead of the most pessimistic "all registers are unsafe". This is the same estimation as used by the dataflow variant of the analysis when the preceding instruction is not known for sure. Without this, leaf functions without CFG information are likely to have false positive reports about non-protected return instructions, as 1) LR is unlikely to be signed and authenticated in a leaf function and 2) LR is likely to be used by a return instruction near the end of the function and 3) the register state is likely to be reset at least once during the linear scan through the function
1 parent 8377b20 commit 54c1b3a

File tree

5 files changed

+29
-93
lines changed

5 files changed

+29
-93
lines changed

‎bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -737,19 +737,14 @@ template <typename StateTy> class CFGUnawareAnalysis {
737737
//
738738
// Then, a function can be split into a number of disjoint contiguous sequences
739739
// of instructions without labels in between. These sequences can be processed
740-
// the same way basic blocks are processed by data-flow analysis, assuming
741-
// pessimistically that all registers are unsafe at the start of each sequence.
740+
// the same way basic blocks are processed by data-flow analysis, with the same
741+
// pessimistic estimation of the initial state at the start of each sequence
742+
// (except the first instruction of the function).
742743
class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
743744
public CFGUnawareAnalysis<SrcState> {
744745
using SrcSafetyAnalysis::BC;
745746
BinaryFunction &BF;
746747

747-
/// Creates a state with all registers marked unsafe (not to be confused
748-
/// with empty state).
749-
SrcState createUnsafeState() const {
750-
return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
751-
}
752-
753748
public:
754749
CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF,
755750
MCPlusBuilder::AllocatorIdTy AllocId,
@@ -759,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
759754
}
760755

761756
void run() override {
757+
const SrcState DefaultState = computePessimisticState(BF);
762758
SrcState S = createEntryState();
763759
for (auto &I : BF.instrs()) {
764760
MCInst &Inst = I.second;
@@ -773,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis,
773769
LLVM_DEBUG({
774770
traceInst(BC, "Due to label, resetting the state before", Inst);
775771
});
776-
S = createUnsafeState();
772+
S = DefaultState;
777773
}
778774

779775
// Attach the state *before* this instruction executes.

‎bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -224,20 +224,33 @@ f_unreachable_instruction:
224224
ret
225225
.size f_unreachable_instruction, .-f_unreachable_instruction
226226

227-
// Expected false positive: without CFG, the state is reset to all-unsafe
228-
// after an unconditional branch.
229-
230-
.globl state_is_reset_after_indirect_branch_nocfg
231-
.type state_is_reset_after_indirect_branch_nocfg,@function
232-
state_is_reset_after_indirect_branch_nocfg:
233-
// CHECK-LABEL: GS-PAUTH: non-protected ret found in function state_is_reset_after_indirect_branch_nocfg, at address
234-
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
227+
// Without CFG, the state is reset at labels, assuming every register that can
228+
// be clobbered in the function was actually clobbered.
229+
230+
.globl lr_untouched_nocfg
231+
.type lr_untouched_nocfg,@function
232+
lr_untouched_nocfg:
233+
// CHECK-NOT: lr_untouched_nocfg
234+
adr x2, 1f
235+
br x2
236+
1:
237+
ret
238+
.size lr_untouched_nocfg, .-lr_untouched_nocfg
239+
240+
.globl lr_clobbered_nocfg
241+
.type lr_clobbered_nocfg,@function
242+
lr_clobbered_nocfg:
243+
// CHECK-LABEL: GS-PAUTH: non-protected ret found in function lr_clobbered_nocfg, at address
244+
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret
235245
// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are:
236246
adr x2, 1f
237247
br x2
238248
1:
249+
b 2f
250+
bl g // never executed, but affects the expected worst-case scenario
251+
2:
239252
ret
240-
.size state_is_reset_after_indirect_branch_nocfg, .-state_is_reset_after_indirect_branch_nocfg
253+
.size lr_clobbered_nocfg, .-lr_clobbered_nocfg
241254

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

‎bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -491,10 +491,6 @@ good_address_arith_multi_bb:
491491
ret
492492
.size good_address_arith_multi_bb, .-good_address_arith_multi_bb
493493

494-
// FIXME: Most *_nocfg test cases contain paciasp+autiasp instructions even if
495-
// LR is not spilled - this is a workaround for RET instructions being
496-
// reported as non-protected, because LR state is reset at every label.
497-
498494
.globl good_ret_nocfg
499495
.type good_ret_nocfg,@function
500496
good_ret_nocfg:
@@ -541,29 +537,25 @@ good_branch_nocfg:
541537
.type good_load_other_reg_nocfg,@function
542538
good_load_other_reg_nocfg:
543539
// CHECK-NOT: good_load_other_reg_nocfg
544-
paciasp
545540
adr x2, 1f
546541
br x2
547542
1:
548543
autia x0, x1
549544
ldr x2, [x0]
550545

551-
autiasp
552546
ret
553547
.size good_load_other_reg_nocfg, .-good_load_other_reg_nocfg
554548

555549
.globl good_load_same_reg_nocfg
556550
.type good_load_same_reg_nocfg,@function
557551
good_load_same_reg_nocfg:
558552
// CHECK-NOT: good_load_same_reg_nocfg
559-
paciasp
560553
adr x2, 1f
561554
br x2
562555
1:
563556
autia x0, x1
564557
ldr x0, [x0]
565558

566-
autiasp
567559
ret
568560
.size good_load_same_reg_nocfg, .-good_load_same_reg_nocfg
569561

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

584-
autiasp
585575
ret
586576
.size bad_unchecked_nocfg, .-bad_unchecked_nocfg
587577

@@ -615,15 +605,13 @@ bad_unknown_usage_read_nocfg:
615605
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
616606
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
617607
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul x3, x0, x1
618-
paciasp
619608
adr x2, 1f
620609
br x2
621610
1:
622611
autia x0, x1
623612
mul x3, x0, x1
624613
ldr x2, [x0]
625614

626-
autiasp
627615
ret
628616
.size bad_unknown_usage_read_nocfg, .-bad_unknown_usage_read_nocfg
629617

@@ -634,15 +622,13 @@ bad_unknown_usage_subreg_read_nocfg:
634622
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
635623
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
636624
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul w3, w0, w1
637-
paciasp
638625
adr x2, 1f
639626
br x2
640627
1:
641628
autia x0, x1
642629
mul w3, w0, w1
643630
ldr x2, [x0]
644631

645-
autiasp
646632
ret
647633
.size bad_unknown_usage_subreg_read_nocfg, .-bad_unknown_usage_subreg_read_nocfg
648634

@@ -653,38 +639,33 @@ bad_unknown_usage_update_nocfg:
653639
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
654640
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
655641
// CHECK-NEXT: 1. {{[0-9a-f]+}}: movk x0, #0x2a, lsl #16
656-
paciasp
657642
adr x2, 1f
658643
br x2
659644
1:
660645
autia x0, x1
661646
movk x0, #42, lsl #16 // does not overwrite x0 completely
662647
ldr x2, [x0]
663648

664-
autiasp
665649
ret
666650
.size bad_unknown_usage_update_nocfg, .-bad_unknown_usage_update_nocfg
667651

668652
.globl good_overwrite_with_constant_nocfg
669653
.type good_overwrite_with_constant_nocfg,@function
670654
good_overwrite_with_constant_nocfg:
671655
// CHECK-NOT: good_overwrite_with_constant_nocfg
672-
paciasp
673656
adr x2, 1f
674657
br x2
675658
1:
676659
autia x0, x1
677660
mov x0, #42
678661

679-
autiasp
680662
ret
681663
.size good_overwrite_with_constant_nocfg, .-good_overwrite_with_constant_nocfg
682664

683665
.globl good_address_arith_nocfg
684666
.type good_address_arith_nocfg,@function
685667
good_address_arith_nocfg:
686668
// CHECK-NOT: good_address_arith_nocfg
687-
paciasp
688669
adr x2, 1f
689670
br x2
690671
1:
@@ -698,7 +679,6 @@ good_address_arith_nocfg:
698679
mov x1, #0
699680
mov x2, #0
700681

701-
autiasp
702682
ret
703683
.size good_address_arith_nocfg, .-good_address_arith_nocfg
704684

‎bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,8 @@ nocfg:
199199
// 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: >)
200200
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI , TrustedRegs: LR W0 W30 X0 W0_HI W30_HI , Insts: >)
201201
// CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8
202-
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: , TrustedRegs: , Insts: >)
203-
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: , TrustedRegs: , Insts: >)
202+
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: >)
203+
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: LR W30 W30_HI , TrustedRegs: LR W30 W30_HI , Insts: >)
204204
// CHECK-NEXT: After src register safety analysis:
205205
// CHECK-NEXT: Binary Function "nocfg" {
206206
// CHECK-NEXT: Number : 3
@@ -224,32 +224,6 @@ nocfg:
224224
// CHECK-NEXT: Found RET inst: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: >
225225
// CHECK-NEXT: RetReg: LR
226226
// CHECK-NEXT: SafeToDerefRegs:{{[ \t]*$}}
227-
// CHECK-EMPTY:
228-
// CHECK-NEXT: Running detailed src register safety analysis...
229-
// 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]()>)
230-
// 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]()>)
231-
// 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]()>)
232-
// 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]()>)
233-
// CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8
234-
// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state<SafeToDerefRegs: , TrustedRegs: , Insts: [0]()>)
235-
// CHECK-NEXT: .. result: (src-state<SafeToDerefRegs: , TrustedRegs: , Insts: [0]()>)
236-
// CHECK-NEXT: After detailed src register safety analysis:
237-
// CHECK-NEXT: Binary Function "nocfg" {
238-
// CHECK-NEXT: Number : 3
239-
// ...
240-
// CHECK: Secondary Entry Points : __ENTRY_nocfg@0x[[ENTRY_ADDR]]
241-
// CHECK-NEXT: }
242-
// CHECK-NEXT: .{{[A-Za-z0-9]+}}:
243-
// CHECK-NEXT: 00000000: adr x0, __ENTRY_nocfg@0x[[ENTRY_ADDR]] # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
244-
// CHECK-NEXT: 00000004: br x0 # UNKNOWN CONTROL FLOW # Offset: 4 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
245-
// CHECK-NEXT: __ENTRY_nocfg@0x[[ENTRY_ADDR]] (Entry Point):
246-
// CHECK-NEXT: .{{[A-Za-z0-9]+}}:
247-
// CHECK-NEXT: 00000008: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
248-
// CHECK-NEXT: DWARF CFI Instructions:
249-
// CHECK-NEXT: <empty>
250-
// CHECK-NEXT: End of Function "nocfg"
251-
// CHECK-EMPTY:
252-
// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state<SafeToDerefRegs: BitVector, TrustedRegs: BitVector, Insts: [0]()>
253227

254228
.globl auth_oracle
255229
.type auth_oracle,@function

0 commit comments

Comments
 (0)