On 10/5/26 9:40 AM, Kees Cook wrote:
Implements the Linux Kernel Control Flow Integrity ABI, which provides a
function prototype based forward edge control flow integrity protection
by instrumenting every indirect call to check for a hash value before
the target function address. If the hash at the call site and the hash
at the target do not match, execution will trap.
See the start of kcfi.cc for design details.
gcc/ChangeLog:
* kcfi.h: New file with KCFI public interface declarations.
* kcfi.cc: New file implementing Kernel Control Flow Integrity
infrastructure.
* Makefile.in (OBJS): Add kcfi.o.
* flag-types.h (enum sanitize_code): Add SANITIZE_KCFI.
* gimple.h (enum gf_mask): Add GF_CALL_INLINED_FROM_KCFI_NOSANTIZE.
(gimple_call_set_inlined_from_kcfi_nosantize): New function.
(gimple_call_inlined_from_kcfi_nosantize_p): New function.
* df-scan.cc (df_uses_record): Add KCFI case to handle KCFI RTL
patterns and process wrapped RTL.
* doc/extend.texi: Update nocf_check for kcfi.
* doc/invoke.texi (fsanitize=kcfi): Add documentation for KCFI
sanitizer option.
* doc/tm.texi.in: Add Kernel Control Flow Integrity section with
TARGET_KCFI_SUPPORTED, TARGET_KCFI_MASK_TYPE_ID,
TARGET_KCFI_EMIT_TYPE_ID hooks.
* doc/tm.texi: Regenerate.
* final.cc (call_from_call_insn): Add KCFI case to handle
KCFI-wrapped calls. Add kcfi_next_labelno().
* opts.cc (sanitizer_opts): Add kcfi entry.
(parse_sanitizer_options): Exclude SANITIZE_KCFI from
-fsanitize-recover=all.
* rtl.def (KCFI): Add new RTL code for KCFI instrumentation.
* rtlanal.cc (reg_referenced_p): Add KCFI case.
* target.def: Add KCFI target hooks.
* toplev.cc (process_options): Add KCFI option processing.
* tree-inline.cc (copy_bb): Handle KCFI no_sanitize attribute
propagation during inlining.
* tree-ssa-tail-merge.cc: Include kcfi.h.
(gimple_equal_p): Don't treat calls that get different KCFI
checks as equal.
* varasm.cc (assemble_start_function): Emit KCFI preambles.
(assemble_external_real): Emit KCFI typeid symbols.
(default_elf_asm_named_section): Handle .kcfi_traps using
SECTION_LINK_ORDER flag.
gcc/c-family/ChangeLog:
* c-attribs.cc: Include asan.h.
(handle_nocf_check_attribute): Enable nocf_check under kcfi.
(handle_patchable_function_entry_attribute): Add error for using
patchable_function_entry attribute with -fsanitize=kcfi.
Signed-off-by: Kees Cook <[email protected]>
---
+
+#include "coretypes.h"
+#include "rtl.h"
In general we try to avoid having .h files include other .h files.
Instead push these into the appropriate .cc files. This make it easier
to spot potential layering violations since the use point is explicit in
the .cc file rather than blindly carried around in a .h file and
potentially getting exposed somewhere we don't want.
+
+/* Common helper for RTL patterns to emit .kcfi_traps section entry.
+ Call after emitting trap label and instruction with the trap symbol
+ reference. */
+extern void kcfi_emit_traps_section (FILE *file, rtx trap_label_sym,
+ int labelno);
+
+/* Extract KCFI type ID from current GIMPLE statement. */
+extern rtx internal_kcfi_get_type_id_for_expanding_gimple_call (void);
Pick one style. You've got the parameter names in one prototype, but
not the other. The general preference within GCC is without the name.
+
+- The check-call instruction sequence must be treated as a single unit: it
+ cannot be rearranged or split or optimized. The pattern is that
+ indirect calls, "call *%target", get converted into:
+
+ mov $target_expression, %target ; only present if the expression was
+ ; not already in %target register
+ load -$offset(%target), %tmp ; load typeid hash from target preamble
+ cmp $typeid, %tmp ; compare expected typeid with loaded
+ je .Lkcfi_call$N ; success: jump to the indirect call
+ .Lkcfi_trap$N: ; label of trap insn
+ trap ; trap on failure, but arranged so
+ ; "permissive mode" falls through
+ .Lkcfi_call$N: ; label of call insn
+ call *%target ; actual indirect call
So I sound like a broken record at this point. I see the assertion that
it has to be an atomic unit, but nothing about why.
+ /* Keep a private copy: get_insn_template can return a pointer into a target
+ output routine's static scratch buffer, and nop_insn is GC-allocated, so
we
+ must not cache the raw pointer for the life of the compilation. */
+ kcfi_nop = xstrdup (get_insn_template (code_num, nop_insn));
+ int nop_insn_bytes = get_attr_length (nop_insn);
Note that some ports do not track insn lengths, so you'll consistently
get back 0 on those. I would probably recommend asserting the size is
nonzero so that if you try to use this on a target without lengths
you'll get a compile-time error rather than a runtime surprise.
Also note that length computations are not guaranteed to be correct;
they are guaranteed to conservatively correct for things like
jump/branch relaxing to work. So in that context conservatively correct
means the reported size is no smaller than the actual size.
So you need to make sure you're safe if the port reports back a longer
size than what gets encoded into the final object.
+ int prefix_nop_bytes = prefix_nops * nop_insn_bytes;
+ kcfi_typeid_offset += prefix_nop_bytes;
+
+ /* In the case where the KCFI preamble (and potentially the prefix NOPs)
+ are being used for alternative CFI implementations via live-patching,
+ the __cfi_... label itself needs to be usable as a callable function
+ target, so alignment NOPs may need to be added between the preamble
+ label and the typeid during KCFI preamble emission:
+
+ __cfi_func: // may need to be function entry aligned
+ [alignment padding nops] // may be needed when -falign-functions set
+ [typeid]
+ [prefix nops]
+ func:
+ [entry nops]
+ [function body]
+
+ But we only calculate alignment padding NOPs when -falign-functions
+ has been explicitly set.
+ */
+ if (align_functions.levels[0].log <= 0)
+ return;
So what happens if the port does not align functions to a useful boundary?
+
+ /* Ignore cold partition sections: cold partitions are never indirect call
+ targets. Only skip preambles for cold partitions (has_bb_partition =
true)
+ not for entire cold-attributed functions (has_bb_partition = false). */
+ if (in_cold_section_p && crtl && crtl->has_bb_partition)
+ return;
Is that an design decision in the partitioning code or is it an
implementation detail. If it's the former, than I'm inclined to allow
relying on it. If it's the latter, then it seems we're just setting up
a ticking time bomb. It could well be either, I don't know off the top
of my head.
I'm assuming that things like devirtualizing a call into a direct call
are safe, including the speculative forms.
Given we can pretty easily describe the generic call setup in RTL, how
painful would it be to instead do the instrumentation in a generic way
rather than twiddling each backend?
In that kind of design we'd force the target to provide the two marker
insns which mark the beginning and end of the atomic region. Target
independent code would be used to load the typeid hash from the preamble
(using the offset the target provides), compare it to the expected type
id, branch around a trap-like insn, the trap-like insn itself, then
finally the call. I'm not demanding this rethink, but I do want it to
be at least considered as a design question.
Jeff
