Re: [PATCH v17 2/7] kcfi: Add core Kernel Control Flow Integrity infrastructure

[Kees Cook <kees@kernel.org>]

On Fri, Oct 09, 2026 at 07:35:29AM -0600, Jeff Law wrote:
> > +#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

Ah-ha, understood! I've fixed this for v18 now.

> > +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.

I've fixed this for v18.  (I'm used to the opposite in Linux: everything
has parameter names.)

> > +- 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:
[...]
> 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.

Yes, sorry, and I've answered this in detail now in my reply to 7/7. In
short:

1. The checked target must stay in the checked register until the call:
   a spill and reload in between would let attacker-writable memory
   supply an unchecked target ("pop %target; call %target").
2. The trap isn't noreturn: in the kernel's permissive mode, execution
   continues from the trap into the call, so the trap block can't be
   moved out of line.
3. The kernel depends on the exact layout: its trap handlers decode the
   instructions around the trap, and x86 may rewrite call sites in place
   into FineIBT at boot.

v18 will add all of this to the design comment, so the "why" lives next
to the assertion.

> > +  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.

Yup, I was able to trigger this mismatch with RISC-V's C extension
where I get a length of 4, but the assembler emits c.nop (2 bytes),
so with prefix NOPs the call sites loaded the type ID from the wrong
offset. (The kernel doesn't use prefix NOPs with KCFI on RISC-V, which
is why I never hit this.)  v18 will add a TARGET_KCFI_NOP_SIZE hook for
the size the assembler actually emits; RISC-V will return 2 with C or
Zca, as Clang does. Without the hook, KCFI will fall back to the length
attribute and assert that it's nonzero.

> > +  if (align_functions.levels[0].log <= 0)
> > +    return;
> So what happens if the port does not align functions to a useful boundary?

That padding is only used in Linux for x86's FineIBT live-patching,
which needs the __cfi_ label itself to be callable, so the kernel asks
for it with -falign-functions. But since this padding is supported by
the core KCFI code, it needs to work on all backends.  On RISC-V with
C, and on Thumb at -Os, the type ID could end up only 2-byte aligned,
making the load misaligned. Clang aligns every function that has a KCFI
type ID to at least 4 bytes, on all targets; v18 will do the same now.

> > +  /* 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.

It's a design property of partitioning: the cold part of a function is
never called, only jumped to from its parent. A function's address is
always its hot entry, so there's no valid entry point into the middle
of a function, and we don't want KCFI to create one.

bb-reorder enforces this: sanitize_hot_paths() walks up from every hot
block to the entry and moves that path into the hot partition, so when a
function is split, its entry block is always hot.

And x86 IBT already depends on the same property. The ENDBR inserter
(rest_of_insert_endbr_and_patchable_area() in i386-features.cc) only
puts an ENDBR at the head of the function's first block, never at the
start of the cold partition. An indirect branch into a cold partition
would fault under IBT just as it would fail a KCFI check. For example,
with -O2 -fcf-protection=full -fsanitize=kcfi:

	__cfi_work:
		...
		movl	$0xb8360626, %eax
	work:
		endbr64
		testl	%edi, %edi
		js	.L4
		...
		.section	.text.unlikely
	work.cold:
	.L4:
		subq	$24, %rsp

work gets both the ENDBR and the KCFI preamble; work.cold gets neither,
and is reached only by the direct "js .L4".

> I'm assuming that things like devirtualizing a call into a direct call
> are safe, including the speculative forms.

Yes. A call that becomes direct needs no check, since its target is
immutable. A speculative call keeps the original indirect call as the
fallback, and that is still an indirect call with its function type, so
it gets checked. KCFI is C-only, so in practice the speculative form
comes from indirect call profiling; v18 will add a tree-prof test
(gcc.dg/tree-prof/kcfi-speculative-call.c) that checks the fallback
keeps its KCFI check and that the profiled program runs without
trapping.

> 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.

I did consider it, and tried something like it around v2. Two things
stopped me:

- The marker insns don't make the region atomic in the ways that matter
  (spill code between the check and the call, and the trap block being
  moved), as above.

- Each target's call setup is more than the generic call: the call
  expanders handle calling conventions, sibcalls, PLT and long-call
  forms, BTI and SLS mitigations, and more. KCFI wraps the call pattern
  each target already produces, so all of that keeps working, and the
  per-target part is only the check sequence itself (and the kernel's
  trap handlers decode a different sequence on each architecture
  anyway). I described this to Andrea around v5[1].

Clang made the same choice: generic code marks the indirect calls, and
each target expands its own check sequence just before the call.

Thanks for looking at all this; hopefully I've covered everything.
Let me know if I need to dig in further anywhere or if I've misunderstood
anything. :)

-Kees

[1] https://inbox.sourceware.org/gcc-patches/202510221253.D6B51F950@keescook/

-- 
Kees Cook