Skip to content

Commit ff3dc1d

Browse files
atrosinenkoatrosinenko
committed
[BOLT] Gadget scanner: account for BRK when searching for auth oracles
An authenticated pointer can be explicitly checked by the compiler via a sequence of instructions that executes BRK on failure. It is important to recognize such BRK instruction as checking every register (as it is expected to immediately trigger an abnormal program termination) to prevent false positive reports about authentication oracles: autia x2, x3 autia x0, x1 ; neither x0 nor x2 are checked at this point eor x16, x0, x0, lsl #1 tbz x16, #62, on_success ; marks x0 as checked ; end of BB: for x2 to be checked here, it must be checked in both ; successor basic blocks on_failure: brk 0xc470 on_success: ; x2 is checked ldr x1, [x2] ; marks x2 as checked
1 parent 630128c commit ff3dc1d

File tree

6 files changed

+75
-35
lines changed

6 files changed

+75
-35
lines changed

‎bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,20 @@ class MCPlusBuilder {
707707
return false;
708708
}
709709

710+
/// Returns true if Inst is a trap instruction.
711+
///
712+
/// Tests if Inst is an instruction that immediately causes an abnormal
713+
/// program termination, for example when a security violation is detected
714+
/// by a compiler-inserted check.
715+
///
716+
/// @note An implementation of this method should likely return false for
717+
/// calls to library functions like abort(), as it is possible that the
718+
/// execution state is partially attacker-controlled at this point.
719+
virtual bool isTrap(const MCInst &Inst) const {
720+
llvm_unreachable("not implemented");
721+
return false;
722+
}
723+
710724
virtual bool isBreakpoint(const MCInst &Inst) const {
711725
llvm_unreachable("not implemented");
712726
return false;

‎bolt/lib/Passes/PAuthGadgetScanner.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,15 @@ class DstSafetyAnalysis {
10661066
dbgs() << ")\n";
10671067
});
10681068

1069+
// If this instruction terminates the program immediately, no
1070+
// authentication oracles are possible past this point.
1071+
if (BC.MIB->isTrap(Point)) {
1072+
LLVM_DEBUG({ traceInst(BC, "Trap instruction found", Point); });
1073+
DstState Next(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
1074+
Next.CannotEscapeUnchecked.set();
1075+
return Next;
1076+
}
1077+
10691078
// If this instruction is reachable by the analysis, a non-empty state will
10701079
// be propagated to it sooner or later. Until then, skip computeNext().
10711080
if (Cur.empty()) {
@@ -1173,8 +1182,8 @@ class DataflowDstSafetyAnalysis
11731182
//
11741183
// A basic block without any successors, on the other hand, can be
11751184
// pessimistically initialized to everything-is-unsafe: this will naturally
1176-
// handle both return and tail call instructions and is harmless for
1177-
// internal indirect branch instructions (such as computed gotos).
1185+
// handle return, trap and tail call instructions. At the same time, it is
1186+
// harmless for internal indirect branch instructions, like computed gotos.
11781187
if (BB.succ_empty())
11791188
return createUnsafeState();
11801189

‎bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,10 +386,9 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
386386
// the list of successors of this basic block as appropriate.
387387

388388
// Any of the above code sequences assume the fall-through basic block
389-
// is a dead-end BRK instruction (any immediate operand is accepted).
389+
// is a dead-end trap instruction.
390390
const BinaryBasicBlock *BreakBB = BB.getFallthrough();
391-
if (!BreakBB || BreakBB->empty() ||
392-
BreakBB->front().getOpcode() != AArch64::BRK)
391+
if (!BreakBB || BreakBB->empty() || !isTrap(BreakBB->front()))
393392
return std::nullopt;
394393

395394
// Iterate over the instructions of BB in reverse order, matching opcodes
@@ -1751,6 +1750,25 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
17511750
Inst.addOperand(MCOperand::createImm(0));
17521751
}
17531752

1753+
bool isTrap(const MCInst &Inst) const override {
1754+
if (Inst.getOpcode() != AArch64::BRK)
1755+
return false;
1756+
// Only match the immediate values that are likely to indicate this BRK
1757+
// instruction is emitted to terminate the program immediately and not to
1758+
// be handled by a SIGTRAP handler, for example.
1759+
switch (Inst.getOperand(0).getImm()) {
1760+
case 0xc470:
1761+
case 0xc471:
1762+
case 0xc472:
1763+
case 0xc473:
1764+
// Explicit Pointer Authentication check failed, see
1765+
// AArch64AsmPrinter::emitPtrauthCheckAuthenticatedValue().
1766+
return true;
1767+
default:
1768+
return false;
1769+
}
1770+
}
1771+
17541772
bool isStorePair(const MCInst &Inst) const {
17551773
const unsigned opcode = Inst.getOpcode();
17561774

‎bolt/test/binary-analysis/AArch64/gs-pauth-address-checks.s

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ resign_xpaci_good:
3131
xpaci x16
3232
cmp x0, x16
3333
b.eq 1f
34-
brk 0x1234
34+
brk 0xc471
3535
1:
3636
pacia x0, x2
3737
ret
@@ -46,7 +46,7 @@ resign_xpacd_good:
4646
xpacd x16
4747
cmp x0, x16
4848
b.eq 1f
49-
brk 0x1234
49+
brk 0xc473
5050
1:
5151
pacda x0, x2
5252
ret
@@ -117,7 +117,7 @@ resign_xpaci_unrelated_auth_and_check:
117117
xpaci x16
118118
cmp x0, x16
119119
b.eq 1f
120-
brk 0x1234
120+
brk 0xc471
121121
1:
122122
pacia x10, x2
123123
ret
@@ -139,7 +139,7 @@ resign_xpaci_wrong_pattern_1:
139139
xpaci x16
140140
cmp x0, x16
141141
b.eq 1f
142-
brk 0x1234
142+
brk 0xc471
143143
1:
144144
pacia x0, x2
145145
ret
@@ -157,7 +157,7 @@ resign_xpaci_wrong_pattern_2:
157157
xpaci x0 // x0 instead of x16
158158
cmp x0, x16
159159
b.eq 1f
160-
brk 0x1234
160+
brk 0xc471
161161
1:
162162
pacia x0, x2
163163
ret
@@ -174,7 +174,7 @@ resign_xpaci_wrong_pattern_3:
174174
xpaci x16
175175
cmp x16, x16 // x16 instead of x0
176176
b.eq 1f
177-
brk 0x1234
177+
brk 0xc471
178178
1:
179179
pacia x0, x2
180180
ret
@@ -191,7 +191,7 @@ resign_xpaci_wrong_pattern_4:
191191
xpaci x16
192192
cmp x0, x0 // x0 instead of x16
193193
b.eq 1f
194-
brk 0x1234
194+
brk 0xc471
195195
1:
196196
pacia x0, x2
197197
ret
@@ -208,7 +208,7 @@ resign_xpaci_wrong_pattern_5:
208208
mov x16, x16 // replace xpaci with a no-op instruction
209209
cmp x0, x16
210210
b.eq 1f
211-
brk 0x1234
211+
brk 0xc471
212212
1:
213213
pacia x0, x2
214214
ret
@@ -228,7 +228,7 @@ resign_xpaclri_good:
228228
xpaclri
229229
cmp x30, x16
230230
b.eq 1f
231-
brk 0x1234
231+
brk 0xc471
232232
1:
233233
pacia x30, x2
234234

@@ -246,7 +246,7 @@ xpaclri_check_keeps_lr_safe:
246246
xpaclri // clobbers LR
247247
cmp x30, x16
248248
b.eq 1f
249-
brk 0x1234 // marks LR as trusted and safe-to-dereference
249+
brk 0xc471 // marks LR as trusted and safe-to-dereference
250250
1:
251251
ret // not reporting non-protected return
252252
.size xpaclri_check_keeps_lr_safe, .-xpaclri_check_keeps_lr_safe
@@ -265,7 +265,7 @@ xpaclri_check_requires_safe_lr:
265265
xpaclri
266266
cmp x30, x16
267267
b.eq 1f
268-
brk 0x1234
268+
brk 0xc471
269269
1:
270270
ret
271271
.size xpaclri_check_requires_safe_lr, .-xpaclri_check_requires_safe_lr
@@ -283,7 +283,7 @@ resign_xpaclri_wrong_reg:
283283
xpaclri // ... but xpaclri still operates on x30
284284
cmp x20, x16
285285
b.eq 1f
286-
brk 0x1234
286+
brk 0xc471
287287
1:
288288
pacia x20, x2
289289

@@ -303,7 +303,7 @@ resign_checked_not_authenticated:
303303
xpaci x16
304304
cmp x0, x16
305305
b.eq 1f
306-
brk 0x1234
306+
brk 0xc471
307307
1:
308308
pacia x0, x2
309309
ret
@@ -323,7 +323,7 @@ resign_checked_before_authenticated:
323323
xpaci x16
324324
cmp x0, x16
325325
b.eq 1f
326-
brk 0x1234
326+
brk 0xc471
327327
1:
328328
autib x0, x1
329329
pacia x0, x2
@@ -339,7 +339,7 @@ resign_high_bits_tbz_good:
339339
autib x0, x1
340340
eor x16, x0, x0, lsl #1
341341
tbz x16, #62, 1f
342-
brk 0x1234
342+
brk 0xc471
343343
1:
344344
pacia x0, x2
345345
ret
@@ -378,7 +378,7 @@ resign_high_bits_tbz_wrong_bit:
378378
autib x0, x1
379379
eor x16, x0, x0, lsl #1
380380
tbz x16, #63, 1f
381-
brk 0x1234
381+
brk 0xc471
382382
1:
383383
pacia x0, x2
384384
ret
@@ -393,7 +393,7 @@ resign_high_bits_tbz_wrong_shift_amount:
393393
autib x0, x1
394394
eor x16, x0, x0, lsl #2
395395
tbz x16, #62, 1f
396-
brk 0x1234
396+
brk 0xc471
397397
1:
398398
pacia x0, x2
399399
ret
@@ -408,7 +408,7 @@ resign_high_bits_tbz_wrong_shift_type:
408408
autib x0, x1
409409
eor x16, x0, x0, lsr #1
410410
tbz x16, #62, 1f
411-
brk 0x1234
411+
brk 0xc471
412412
1:
413413
pacia x0, x2
414414
ret
@@ -423,7 +423,7 @@ resign_high_bits_tbz_wrong_pattern_1:
423423
autib x0, x1
424424
eor x16, x0, x0, lsl #1
425425
tbz x17, #62, 1f
426-
brk 0x1234
426+
brk 0xc471
427427
1:
428428
pacia x0, x2
429429
ret
@@ -438,7 +438,7 @@ resign_high_bits_tbz_wrong_pattern_2:
438438
autib x0, x1
439439
eor x16, x10, x0, lsl #1
440440
tbz x16, #62, 1f
441-
brk 0x1234
441+
brk 0xc471
442442
1:
443443
pacia x0, x2
444444
ret
@@ -453,7 +453,7 @@ resign_high_bits_tbz_wrong_pattern_3:
453453
autib x0, x1
454454
eor x16, x0, x10, lsl #1
455455
tbz x16, #62, 1f
456-
brk 0x1234
456+
brk 0xc471
457457
1:
458458
pacia x0, x2
459459
ret
@@ -648,7 +648,7 @@ many_checked_regs:
648648
xpacd x16 // ...
649649
cmp x2, x16 // ...
650650
b.eq 2f // end of basic block
651-
brk 0x1234
651+
brk 0xc473
652652
2:
653653
pacdza x0
654654
pacdza x1

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ good_explicit_check:
7979
autia x0, x1
8080
eor x16, x0, x0, lsl #1
8181
tbz x16, #62, 1f
82-
brk 0x1234
82+
brk 0xc470
8383
1:
8484
ret
8585
.size good_explicit_check, .-good_explicit_check
@@ -346,7 +346,7 @@ good_explicit_check_multi_bb:
346346
1:
347347
eor x16, x0, x0, lsl #1
348348
tbz x16, #62, 2f
349-
brk 0x1234
349+
brk 0xc470
350350
2:
351351
cbz x1, 3f
352352
nop
@@ -658,16 +658,15 @@ good_address_arith_nocfg:
658658
.globl good_explicit_check_unrelated_reg
659659
.type good_explicit_check_unrelated_reg,@function
660660
good_explicit_check_unrelated_reg:
661-
// CHECK-LABEL: GS-PAUTH: authentication oracle found in function good_explicit_check_unrelated_reg, basic block {{[^,]+}}, at address
662-
// FIXME: The below instruction is not an authentication oracle
661+
// CHECK-NOT: good_explicit_check_unrelated_reg
663662
autia x2, x3 // One of possible execution paths after this instruction
664663
// ends at BRK below, thus BRK used as a trap instruction
665664
// should formally "check everything" not to introduce
666665
// false-positive here.
667666
autia x0, x1
668667
eor x16, x0, x0, lsl #1
669668
tbz x16, #62, 1f
670-
brk 0x1234
669+
brk 0xc470
671670
1:
672671
ldr x4, [x2] // Right before this instruction X2 is checked - this
673672
// should be propagated to the basic block ending with

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ good_sign_auted_checked_brk:
5757
autda x0, x2
5858
eor x16, x0, x0, lsl #1
5959
tbz x16, #62, 1f
60-
brk 0x1234
60+
brk 0xc472
6161
1:
6262
pacda x0, x1
6363
ret
@@ -351,7 +351,7 @@ good_sign_auted_checked_brk_multi_bb:
351351
1:
352352
eor x16, x0, x0, lsl #1
353353
tbz x16, #62, 2f
354-
brk 0x1234
354+
brk 0xc472
355355
2:
356356
cbz x4, 3f
357357
nop
@@ -705,7 +705,7 @@ good_resign_with_increment_brk:
705705
add x0, x0, #8
706706
eor x16, x0, x0, lsl #1
707707
tbz x16, #62, 1f
708-
brk 0x1234
708+
brk 0xc472
709709
1:
710710
mov x2, x0
711711
pacda x2, x1

0 commit comments

Comments
 (0)