CFI: Support different patchable-function-prefix values
Opened this issue · 3 comments
KCFI currently emits the type hash before patchable-function-prefix nops, and assumes that all functions have the same number of prefix nops. Specifically, indirect call targets must have the same number of prefix nops as the callers. However, Mark Rutland has plans to use prefix nops in a way that breaks this assumption.
Based on an earlier IRC discussion, the preferred way of solving this would be to move the hash after the prefix nops, which would mean callers no longer have to adjust the read offset. This would simplify things, but emitting the hash between the nops and the function entry would break anyone planning to execute the prefix nops and fall through to the actual function, unless they explicitly jump over the hash.
Such use cases don't appear to exist in the kernel right now, but unless we make this architecture specific, moving the hash does mean some changes are needed to the retbleed/FineIBT patching on x86. We also need to figure out if the __cfi_function symbol still needs to be emitted before the nops.
Tests for the current KCFI + patchable-function-prefix behavior are here:
https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/AArch64/kcfi-patchable-function-prefix.ll
https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/X86/kcfi-patchable-function-prefix.ll
Another option would be to add a command line flag for specifying the hash offset, and ensuring that we always emit the hash at the specified offset no matter how many prefix nops are requested.
Since the kcfi code expects the hash to appear in a specific location so that an instrumented indirect jump can reliably obtain the hash. For a translation unit -fpatchable-function-entry=N,M may be specified or not, and we want both to work. Therefore, I agree that a consistent hash location will help. This argument favors placing M nops before the hash. The downside is a restriction on how the M nops can be used. Previously if M>0, the runtime code needs to check whether a BTI exists to locate the N-M after-function-entry NOPs. If the hash appears after the M nops, the runtime code needs to additionally knows whether the hash exists. My question is how to reliably detect this.
If there is motivation using M>0, I'd like to know the concrete code sequence for -fpatchable-function-entry=N,M and how the runtime code detects the NOPs with optional hash and optional BTI.
@MaskRay it looks like the latest suggestion was to add a section argument to the patchable_function_entry attribute. Any thoughts about that?
https://lore.kernel.org/lkml/Y1QEzk%2FA41PKLEPe@hirez.programming.kicks-ass.net/