[PATCH] D119296: KCFI sanitizer

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen added a comment.

In D119296#3466027 <https://reviews.llvm.org/D119296#3466027>, @joaomoreira wrote:

> I played a little bit with kcfi and here are some thoughts:
>
> - under -Os I saw functions being inlined, regardless of the source code calling them indirectly. In these scenarios, the KCFI check was still in place, even though there was not a pointer involved in the call. Although not semantically incorrect, it would be great to prevent the unnecessary overhead (see attached source/compile it with -Os and -fsanitize=kcfi).

Yes, I suspect this might be an issue with Clang's existing CFI schemes too, and would probably require an additional pass to drop checks before calls that were either inlined or optimized into direct calls.

> - I noticed that KCFI checks are placed much before the indirect call arguments are properly placed in the passing registers. This makes the position of the check unpredictable. I assume this is a bad thing in case the kernel eventually decides to come up with an approach that uses alternatives CFI schemes through text-patching.

It shouldn't matter for text patching as we'll still know the exact location of the instruction sequence that we want to patch by looking at the `.kcfi_traps` section.

> Because of things like the above, in the past I decided to implement these things in the very backend of the compiler, so other optimizations would not break the layout nor leave dummy checks around. I find it nice to have this implemented as a more architecture general feature, but maybe it would be cool to have a finalization pass in the X86 backend just to tie things.

@pcc, any thoughts about these issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296