Re: [PATCH 5/5] kcfi: Rename CONFIG_CFI_CLANG to CONFIG_CFI

[Nathan Chancellor <nathan@kernel.org>]

On Wed, Aug 27, 2025 at 12:35:12AM -0700, Randy Dunlap wrote:
> On 8/26/25 6:34 PM, Nathan Chancellor wrote:
> > On Mon, Aug 25, 2025 at 03:31:34PM -0400, Kees Cook wrote:
> >> On August 25, 2025 1:00:22 PM EDT, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> >>> On Mon, Aug 25, 2025 at 5:35 PM Kees Cook <kees@kernel.org> wrote:
> >>>>
> >>>> Yeah, that's a good idea. What the right way to do that?
> >>>>
> >>>> config CFI_CLANG
> >>>>         bool "Use Clang's Control Flow Integrity (CFI)"
> >>>>         depends on ARCH_SUPPORTS_CFI
> >>>>         select CFI
> >>>>
> >>>> ?
> >>>
> >>> I don't recall what is the idiomatic solution for renames, but I
> >>> remember Linus talking about this topic and about avoiding losing old
> >>> values if possible (perhaps getting a new question in `oldconfig` is
> >>> OK as long as the `olddefconfig` respects the old value).
> >>>
> >>> I think your suggestion above will still make it appear twice in
> >>> `menuconfig` -- there may be a way to play with visibility to make it
> >>> better.
> >>>
> >>> A simple possibility I can think of (assuming it works) is having the
> >>> CFI symbol for the time being introduced just as a `def_bool
> >>> CFI_CLANG` for a few releases so that people get the new one in their
> >>> configs.
> >>
> >> Ah, I think this works:
> >>
> >> config CFI_CLANG
> >>     bool
> >>
> >> config CFI
> >>     bool "...."
> >>     default CFI_CLANG
> >>
> >> I will add that for v2.
> > 
> > That does not appear to work for me. I applied
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index c25a45d9aa96..0d3ed03c76c2 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -876,8 +876,12 @@ config ARCH_SUPPORTS_CFI
> >  config ARCH_USES_CFI_TRAPS
> >         bool
> > 
> > +config CFI_CLANG
> > +       bool
> > +
> >  config CFI
> >         bool "Use Kernel Control Flow Integrity (kCFI)"
> > +       default CFI_CLANG
> >         depends on ARCH_SUPPORTS_CFI
> >         depends on $(cc-option,-fsanitize=kcfi)
> >         help
> > 
> > on top of this series and
> > 
> >   CONFIG_CFI_CLANG=y
> >   # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS is not set
> >   # CONFIG_CFI_PERMISSIVE is not set
> > 
> > gets turned into
> > 
> >   # CONFIG_CFI is not set
> > 
> > after olddefconfig. CONFIG_CFI_CLANG has to be user selectable with a
> 
> Could/did you test with 'oldconfig' instead?
> 
> olddefconfig is going to use the default value from the Kconfig file,
> which if CFI_CLANG which is undefined/No/Not set.
> 
> oldconfig will use the old value from the .config file.

I am not sure I understand what you mean here. With the series as it is
or Kees's suggested fix, oldconfig still prompts the user to enable
CONFIG_CFI with CONFIG_CFI_CLANG=y in the old configuration. Both Miguel
and I allude to that being fine but it would be really nice if users
with CONFIG_CFI_CLANG=y were automatically transitioned to CONFIG_CFI=y
without any action on their part. That seems to be in line with how
Linus feels even as recently as this past merge window:

https://lore.kernel.org/CAHk-=wgO0Rx2LcYT4f75Xs46orbJ4JxO2jbAFQnVKDYAjV5HeQ@mail.gmail.com/

Another idea I had to avoid this is introducing CONFIG_CFI_GCC as a user
selectable symbol and making CONFIG_CFI the hidden symbol that both
compiler symbols select. After a couple of releases (or maybe the next
LTS), both CONFIG_CFI_CLANG and CONFIG_CFI_GCC could be eliminated with
CONFIG_CFI becoming user selectable, which would keep things working
since CONFIG_CFI=y will be present in the previous configuration.

Maybe it is not worth optimizing for this situation. I personally check
my configurations into git so that it is easy to deal with losing
things, as I have had my networking broken several times by new symbols
and dependencies that do not get handled well with olddefconfig.

> > prompt but the only solution I can think of at the moment results in a
> > duplicate prompt for Clang.
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index c25a45d9aa96..93bf9b41a9de 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -876,8 +876,17 @@ config ARCH_SUPPORTS_CFI
> >  config ARCH_USES_CFI_TRAPS
> >         bool
> > 
> > +config CFI_CLANG
> > +       bool "Use Kernel Control Flow Integrity (kCFI) - Transitional" if CC_IS_CLANG
> > +       select CFI
> > +       depends on ARCH_SUPPORTS_CFI
> > +       depends on $(cc-option,-fsanitize=kcfi)
> > +       help
> > +         This is a transitional symbol to CONFIG_CFI, see its help text
> > +         for more information.
> > +
> >  config CFI
> > -       bool "Use Kernel Control Flow Integrity (kCFI)"
> > +       bool "Use Kernel Control Flow Integrity (kCFI)" if CC_IS_GCC
> >         depends on ARCH_SUPPORTS_CFI
> >         depends on $(cc-option,-fsanitize=kcfi)
> >         help
> > 
> > Maybe that does not matter for the sake of keeping things working?
> > Otherwise, we could just keep things as they are with the patch set and
> > expect people to actually use oldconfig or diff the results of
> > olddefconfig (which I think is good practice anyways).
> > 
> > Cheers,
> > Nathan
> > 
> 
> -- 
> ~Randy
>