[PATCH] D119296: KCFI sanitizer

[Joao Moreira via Phabricator via llvm-commits]

joaomoreira added a subscriber: craig.topper.
joaomoreira added a comment.

> This seems like a reasonable approach, and was also the approach taken for the PAuth ABI. The PAuth ABI attaches an operand bundle to the call instruction and arranges for the code for the check to be generated together with the call. This helps with avoiding spills of the verified function pointer between the check and the call. The code isn't upstream but is available on this branch: https://github.com/pcc/llvm-project/tree/apple-pac4
>
> Grep for something like `undle.*ptrauth` and you should find the relevant code.

FWIIW, the way I implemented this on FineIBT (https://github.com/lvwr/llvm-project/tree/fineibt/kernel ) was by adding an attribute to the MachineInstr class and then forwarding the prototype to be checked whenever a call instruction was translated from IR into MachineIR. Then, in the back-end, the IBT pass expands the hash-related instructions from the MachineInstr attribute. It is simple and concise and I think it resembles what @pcc is suggesting (please, correct me if I'm mistaken). Of course, the above mentioned branch is super raw, but it gives an insight.

At first I was on the boat of KCFI being implemented in the IR level because being architecture agnostic is certainly a great plus. Yet, once we end up needing additional passes (regardless of it being in IR or in the arch-specific back-end) to tie things and achieve the best possible instrumentation, I start to think we are over-complicating the design of the feature -- if we have a single pass at the very end of each supported arch back-end that just traverses the functions and adds KCFI checks to indirect branches that have a "PrototypeHash" attribute set (or an special operand as also suggested by @pcc ) we can get the best possible instrumentation while inherently dodging any pitfall due to unforeseen optimizations or code transformations that may happen between the KCFI checks placement placement and the last stage of the back-end.

I'm not an expert on LLVM's pipeline, but it just feels a little awkward and redundant that we need passes to fix what other passes messed up regarding a pass that executed before everything. Also, I don't see clearly how it could not be in the back-end if we want to avoid all the call argument setup in between the check and the call. Perhaps more people with a broader view regarding "CodeGen->Arch Backend" (like @craig.topper :)) could be brought into this discussion and share some thoughts on how to stitch it all more efficiently.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119296/new/

https://reviews.llvm.org/D119296