Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFI: Add an option to not emit endbr64 for functions with !kcfi_type metadata #1735

Closed
samitolvanen opened this issue Oct 14, 2022 · 5 comments
Labels
[FEATURE] CFI Related to building the kernel with Clang Control Flow Integrity feature-request Not a bug per-se

Comments

@samitolvanen
Copy link
Member

On x86_64 when KCFI is patched to FineIBT at runtime, we no longer need the endbr64 instruction at the beginning of address-taken functions, as we won't be calling them indirectly.

As requested by Peter, add an option to Clang to omit the endbr64 instruction from functions that have !kcfi_type metadata. This could be an additional command-line flag that's simply passed to LLVM as a module attribute, e.g. -fsanitize-kcfi-omit-endbr.

Note that the current KCFI+IBT combination does require endbr64 to be emitted, so this flag is only relevant when we gain FineIBT support.

@kees @lvwr

@samitolvanen samitolvanen added feature-request Not a bug per-se [FEATURE] CFI Related to building the kernel with Clang Control Flow Integrity labels Oct 14, 2022
@kees
Copy link

kees commented Oct 14, 2022

I don't understand this -- isn't FineIBT going to be a runtime patch? i.e. we must always emit endbr in case the system doesn't support IBT. And if it does, FineIBT would just rewrite stuff to move endbr?

@samitolvanen
Copy link
Member Author

I don't understand this -- isn't FineIBT going to be a runtime patch?

Yes, I believe that's the current plan.

i.e. we must always emit endbr in case the system doesn't support IBT.

If the system doesn't support IBT, we don't need endbr in the first place. The reason Peter wanted this feature was to save four bytes per function. It's nice to have, but not absolutely necessary.

@kees
Copy link

kees commented Oct 15, 2022

Right, I guess I was thinking about "distro" kernels that need to support both IBT and non-IBT systems. So this would be either a system without IBT (and without FineIBT), or for a system with IBT, but it must then use FineIBT?

@samitolvanen
Copy link
Member Author

So this would be either a system without IBT (and without FineIBT), or for a system with IBT, but it must then use FineIBT?

Correct, Peter wanted to always patch KCFI into FineIBT on systems that support IBT.

@samitolvanen
Copy link
Member Author

As discussed in this thread, since the kernel handles FineIBT patching, it should also poison the endbr when needed:

https://lore.kernel.org/lkml/20230615193722.194131053@infradead.org/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[FEATURE] CFI Related to building the kernel with Clang Control Flow Integrity feature-request Not a bug per-se
Projects
None yet
Development

No branches or pull requests

2 participants