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

On Thu, Jul 23, 2026 at 06:28:16PM -0700, Andrea Pinski wrote:
> On Wed, Jul 15, 2026 at 4:32 PM Kees Cook <[email protected]> wrote:
> >
> > On Wed, Jul 15, 2026 at 03:34:09PM -0700, Kees Cook wrote:
> > > On Sat, Jun 27, 2026 at 03:57:36PM -0700, Andrea Pinski wrote:
> > > > On Thu, Jun 18, 2026 at 1:45 PM Kees Cook <[email protected]> wrote:
> > > > > [...]
> > > > > +/* KCFI label counter, incremented by KCFI insn emission.  */
> > > > > +static int kcfi_labelno = 0;
> > > >
> > > > I am not 100% sure if this needs a GTY marker or not. I suspect no
> > > > because we should not have emitted assembly code yet.
> > >
> > > If this moves with kcfi_next_labelno into final, I think it's okay
> > > without GTY?
> >
> > This is an int, so GC shouldn't be an issue.
> 
> GTY is used for PCH also and not just GC. This is why I question the
> need for the GTY marker. A GTY marker on an int is to make sure that
> it is restored from the PCH.
> Maybe you can add a couple of PCH testcases to make sure it is working
> correctly.

Ah, gotcha. Yeah, this doesn't appear to be an issue for PCH since it's
used during output only. Regardless, I've added pch tests now as well.

> > > > > +      type_id = (uint32_t) TREE_INT_CST_LOW (value);
> > > > > +    }
> > > > > +  else
> > > > > +    {
> > > > > +      type_id = compute_kcfi_type_id (fn_type);
> > > > > +
> > > > > +      tree type_id_tree = build_int_cst (unsigned_type_node, type_id);
> > > > > +      tree attr = build_tree_list (kcfi_type_id_attr, type_id_tree);
> > > > > +
> > > > > +      TYPE_ATTRIBUTES (fn_type) = chainon (TYPE_ATTRIBUTES (fn_type), attr);
> > > > > +    }
> > > >
> > > > Instead of an attribute there must be a better way of doing this.
> > > > Maybe a hashset instead.
> > >
> > > Perhaps? I will go examine this vs LTO, etc.
> >
> > Tracking this with lifetime tied to the fndecl is going to be more pain
> > from what I can find. The attribute is stable and doesn't cause problems
> > for LTO: I've tested with 2 TUs, and this all appears to happen
> > post-merge? Anyway, if there is something I've missed here, I'm happy to
> > find a new solution, but I can't induce any problems so far.
> 
> So maybe we add a field for FUNCTION_TYPE for this instead of an
> attribute. But that requires extra code for streaming the LTO and
> such. But it will reduce the overall overhead in general.

Okay, I've replaced the attribute with a hashset, which you'd suggested
before. This keeps the mapping entirely within kcfi.cc, and doesn't
bloat the FUNCTION_TYPE object with a new field that would only be used
for kcfi.

I'll get v15 sent shortly. :) Thanks!

-Kees

-- 
Kees Cook