Page MenuHomePhabricator

[cfi-verify] Detect more protected calls using cross-DSO.
AbandonedPublic

Authored by jgalenson on Jul 23 2018, 9:39 AM.

Details

Summary

When using cross-DSO, some indirect calls are not guarded by a branch to a trap but instead follow a call to __cfi_slowpath. For example:

if (!InlinedFastCheck(f)) {

call *f

} else {

__cfi_slowpath(CallSiteTypeId, f);
call *f

}

In this case, the second call to f is not marked as protected by the current code. We thus recognize if an indirect call directly follows a call to a function that will trap on CFI violations and treat them as protected.

We also ignore indirect calls in the PLT, since on AArch64 each entry contains an indirect call that should not be protected by CFI.

Diff Detail

Event Timeline

jgalenson created this revision.Jul 23 2018, 9:39 AM
jgalenson updated this revision to Diff 157298.Jul 25 2018, 9:57 AM
jgalenson edited the summary of this revision. (Show Details)

Note that this could be combined with its parent patch, as that is now about supporting cross-DSO.

Remind me, was there a reason why you wanted to use binary test inputs instead of assembling them?

tools/llvm-cfi-verify/lib/FileAnalysis.cpp
448

Why?

Remind me, was there a reason why you wanted to use binary test inputs instead of assembling them?

The same reason as in https://reviews.llvm.org/D49383:

Second, I don't know the best way of testing this. I need correctly-linked files to populate the symbol table and PLT, but I don't think I can assume a linker, and llvm-mc doesn't seem to handle calls to undefined functions well. So for now I just updated a binary with a valid PLT, but I'd much prefer to use .s and unit tests like the others. Is there a good way to do that, or does this suffice?

As an aside, when I finish the rewrite suggested in https://reviews.llvm.org/D49383, I'm planning to merge this into that patch so that it adds proper support for cross-DSO mode.

tools/llvm-cfi-verify/lib/FileAnalysis.cpp
448

AArch64 plt entries look like:

adrp
ldr x17
add
br x17

So of course they're unprotected indirect calls.

What I left off this comment is that this is only needed when using the -ignore-dwarf flag. Without that, these entries are filtered out because they have no line information. But when using that flag, something like this is very helpful at removing spurious failures.