Skip to content

Commit 7d38c3e

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 6ca76c6 commit 7d38c3e

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
@@ -464,10 +464,6 @@ good_address_arith_multi_bb:
464464
ret
465465
.size good_address_arith_multi_bb, .-good_address_arith_multi_bb
466466

467-
// FIXME: Most *_nocfg test cases contain paciasp+autiasp instructions even if
468-
// LR is not spilled - this is a workaround for RET instructions being
469-
// reported as non-protected, because LR state is reset at every label.
470-
471467
.globl good_ret_nocfg
472468
.type good_ret_nocfg,@function
473469
good_ret_nocfg:
@@ -514,29 +510,25 @@ good_branch_nocfg:
514510
.type good_load_other_reg_nocfg,@function
515511
good_load_other_reg_nocfg:
516512
// CHECK-NOT: good_load_other_reg_nocfg
517-
paciasp
518513
adr x2, 1f
519514
br x2
520515
1:
521516
autia x0, x1
522517
ldr x2, [x0]
523518

524-
autiasp
525519
ret
526520
.size good_load_other_reg_nocfg, .-good_load_other_reg_nocfg
527521

528522
.globl good_load_same_reg_nocfg
529523
.type good_load_same_reg_nocfg,@function
530524
good_load_same_reg_nocfg:
531525
// CHECK-NOT: good_load_same_reg_nocfg
532-
paciasp
533526
adr x2, 1f
534527
br x2
535528
1:
536529
autia x0, x1
537530
ldr x0, [x0]
538531

539-
autiasp
540532
ret
541533
.size good_load_same_reg_nocfg, .-good_load_same_reg_nocfg
542534

@@ -548,13 +540,11 @@ bad_unchecked_nocfg:
548540
// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_unchecked_nocfg, at address
549541
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
550542
// CHECK-NEXT: The 0 instructions that leak the affected registers are:
551-
paciasp
552543
adr x2, 1f
553544
br x2
554545
1:
555546
autia x0, x1
556547

557-
autiasp
558548
ret
559549
.size bad_unchecked_nocfg, .-bad_unchecked_nocfg
560550

@@ -588,15 +578,13 @@ bad_unknown_usage_read_nocfg:
588578
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
589579
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
590580
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul x3, x0, x1
591-
paciasp
592581
adr x2, 1f
593582
br x2
594583
1:
595584
autia x0, x1
596585
mul x3, x0, x1
597586
ldr x2, [x0]
598587

599-
autiasp
600588
ret
601589
.size bad_unknown_usage_read_nocfg, .-bad_unknown_usage_read_nocfg
602590

@@ -607,15 +595,13 @@ bad_unknown_usage_subreg_read_nocfg:
607595
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
608596
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
609597
// CHECK-NEXT: 1. {{[0-9a-f]+}}: mul w3, w0, w1
610-
paciasp
611598
adr x2, 1f
612599
br x2
613600
1:
614601
autia x0, x1
615602
mul w3, w0, w1
616603
ldr x2, [x0]
617604

618-
autiasp
619605
ret
620606
.size bad_unknown_usage_subreg_read_nocfg, .-bad_unknown_usage_subreg_read_nocfg
621607

@@ -626,38 +612,33 @@ bad_unknown_usage_update_nocfg:
626612
// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1
627613
// CHECK-NEXT: The 1 instructions that leak the affected registers are:
628614
// CHECK-NEXT: 1. {{[0-9a-f]+}}: movk x0, #0x2a, lsl #16
629-
paciasp
630615
adr x2, 1f
631616
br x2
632617
1:
633618
autia x0, x1
634619
movk x0, #42, lsl #16 // does not overwrite x0 completely
635620
ldr x2, [x0]
636621

637-
autiasp
638622
ret
639623
.size bad_unknown_usage_update_nocfg, .-bad_unknown_usage_update_nocfg
640624

641625
.globl good_unknown_overwrite_nocfg
642626
.type good_unknown_overwrite_nocfg,@function
643627
good_unknown_overwrite_nocfg:
644628
// CHECK-NOT: good_unknown_overwrite_nocfg
645-
paciasp
646629
adr x2, 1f
647630
br x2
648631
1:
649632
autia x0, x1
650633
mul x0, x1, x2
651634

652-
autiasp
653635
ret
654636
.size good_unknown_overwrite_nocfg, .-good_unknown_overwrite_nocfg
655637

656638
.globl good_address_arith_nocfg
657639
.type good_address_arith_nocfg,@function
658640
good_address_arith_nocfg:
659641
// CHECK-NOT: good_address_arith_nocfg
660-
paciasp
661642
adr x2, 1f
662643
br x2
663644
1:
@@ -671,7 +652,6 @@ good_address_arith_nocfg:
671652
mov x1, #0
672653
mov x2, #0
673654

674-
autiasp
675655
ret
676656
.size good_address_arith_nocfg, .-good_address_arith_nocfg
677657

‎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:
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)