Message-ID: <59c4f103-7f1b-4829-bd82-0d392047fea4@oracle.com>
Date: Mon, 1 Sep 2025 10:34:19 +0200
From: Vegard Nossum <vegard.nossum@...cle.com>
To: Kees Cook <kees@...nel.org>, Nathan Chancellor <nathan@...nel.org>
Cc: Nicolas Schier <nicolas.schier@...ux.dev>,
        Jonathan Corbet <corbet@....net>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Randy Dunlap <rdunlap@...radead.org>, Arnd Bergmann <arnd@...db.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        linux-kbuild@...r.kernel.org, linux-doc@...r.kernel.org,
        Miguel Ojeda <ojeda@...nel.org>,
        Stephen Brennan <stephen.s.brennan@...cle.com>,
        Marco Bonelli <marco@...eim.net>, Petr Vorel <pvorel@...e.cz>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] kconfig: Add transitional symbol attribute for
 migration support


Hi,

Drive-by review... consider it more as "here's some stuff that could be
worth looking at" rather than blocking in any way.

On 30/08/2025 04:01, Kees Cook wrote:
> During kernel option migrations (e.g. CONFIG_CFI_CLANG to CONFIG_CFI),
> existing .config files need to maintain backward compatibility while
> preventing deprecated options from appearing in newly generated
> configurations. This is challenging with existing Kconfig mechanisms
> because:
> 
> 1. Simply removing old options breaks existing .config files.
> 2. Manually listing an option as "deprecated" leaves it needlessly
>     visible and still writes them to new .config files.
> 3. Using any method to remove visibility (.e.g no 'prompt', 'if n',
>     etc) prevents the option from being processed at all.
> 
> Add a "transitional" attribute that creates symbols which are:
> - Processed during configuration (can influence other symbols' defaults)
> - Hidden from user menus (no prompts appear)
> - Omitted from newly written .config files (gets migrated)
> - Restricted to only having help sections (no defaults, selects, etc)
>    making it truly just a "prior value pass-through" option.
> 
> The transitional syntax requires a type argument and prevents type
> redefinition:
> 
>      config OLD_OPTION
>          transitional bool
>          help
>            Transitional config for OLD_OPTION migration.
> 
>      config NEW_OPTION
>          bool "New option"
>          default OLD_OPTION

Can you add this to scripts/kconfig/tests/ + both positive and negative
tests? Tests are run with 'make testconfig' but (AFAICT) doesn't
actually recompile config/mconf/etc. before running the tests, so small
gotcha there.

> This allows seamless migration: olddefconfig processes existing
> CONFIG_OLD_OPTION=y settings to enable CONFIG_NEW_OPTION=y, while
> CONFIG_OLD_OPTION is omitted from newly generated .config files.
> 
> Implementation details:
> - Parser validates transitional symbols can only have help sections
> - Symbol visibility logic updated: usable = (visible != no || transitional)
> - Transitional symbols preserve user values during configuration
> - Type safety enforced to prevent redefinition after transitional declaration
> - Used distinct struct members instead of new flags for readability
> - Documentation added to show the usage
> 
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> With help from Claude Code to show me how to navigate the kconfig parser.
> 
>   v2: fixed human-introduced errors
>   v1: https://lore.kernel.org/all/20250830014438.work.682-kees@kernel.org/
> 
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nicolas Schier <nicolas.schier@...ux.dev>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Randy Dunlap <rdunlap@...radead.org>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Cc: <linux-kbuild@...r.kernel.org>
> Cc: <linux-doc@...r.kernel.org>
> ---
>   scripts/kconfig/expr.h                    | 15 +++++++
>   scripts/kconfig/lexer.l                   |  1 +
>   scripts/kconfig/parser.y                  | 51 +++++++++++++++++++++++
>   scripts/kconfig/symbol.c                  | 11 +++--
>   Documentation/kbuild/kconfig-language.rst | 31 ++++++++++++++
>   5 files changed, 106 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index fe2231e0e6a4..be51574d6c77 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -127,6 +127,21 @@ struct symbol {
>   	/* SYMBOL_* flags */
>   	int flags;
>   
> +	/*
> +	 * Transitional symbol - processed during configuration but hidden from
> +	 * user in menus and omitted from newly written .config files. Used for
> +	 * backward compatibility during config option migrations (e.g.,
> +	 * CFI_CLANG → CFI). Transitional symbols can still influence default
> +	 * expressions of other symbols.
> +	 */
> +	bool transitional:1;
> +
> +	/*
> +	 * Symbol usability - calculated as (visible != no || transitional).
> +	 * Determines if symbol can be used in expressions.
> +	 */
> +	bool usable:1;
> +

It's a bit of a "red flag" to see bitfield bools just after an "int
flags;" member... should these be SYMBOL_ flags?

Speaking of SYMBOL_ flags, there's apparently one that controls whether
a given symbol should be written out to .config:

scripts/kconfig/expr.h:#define SYMBOL_WRITE      0x0200  /* write symbol 
to file (KCONFIG_CONFIG) */

This seems like something you'd like to use somehow -- maybe simply
clear it in sym_calc_value() if it's transitional? Similar to how it's
done for choice values:

         if (sym_is_choice(sym))
                 sym->flags &= ~SYMBOL_WRITE;

>   	/* List of properties. See prop_type. */
>   	struct property *prop;
>   
> diff --git a/scripts/kconfig/lexer.l b/scripts/kconfig/lexer.l
> index 9c2cdfc33c6f..6d2c92c6095d 100644
> --- a/scripts/kconfig/lexer.l
> +++ b/scripts/kconfig/lexer.l
> @@ -126,6 +126,7 @@ n	[A-Za-z0-9_-]
>   "select"		return T_SELECT;
>   "source"		return T_SOURCE;
>   "string"		return T_STRING;
> +"transitional"		return T_TRANSITIONAL;
>   "tristate"		return T_TRISTATE;
>   "visible"		return T_VISIBLE;
>   "||"			return T_OR;
> diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
> index e9c3c664e925..01d2d0f720ce 100644
> --- a/scripts/kconfig/parser.y
> +++ b/scripts/kconfig/parser.y
> @@ -75,6 +75,7 @@ struct menu *current_menu, *current_entry, *current_choice;
>   %token T_SELECT
>   %token T_SOURCE
>   %token T_STRING
> +%token T_TRANSITIONAL
>   %token T_TRISTATE
>   %token T_VISIBLE
>   %token T_EOL
> @@ -205,6 +206,16 @@ config_option: T_PROMPT T_WORD_QUOTE if_expr T_EOL
>   	printd(DEBUG_PARSE, "%s:%d:prompt\n", cur_filename, cur_lineno);
>   };
>   
> +config_option: T_TRANSITIONAL type T_EOL
> +{
> +	if (current_entry->sym->type != S_UNKNOWN)
> +		yyerror("transitional type cannot be set after symbol type is already defined");
> +	menu_set_type($2);
> +	current_entry->sym->transitional = true;
> +	printd(DEBUG_PARSE, "%s:%d:transitional(%u)\n", cur_filename, cur_lineno,
> +		$2);
> +};

You could also consider making this an attribute similar to the
"modules" flags and simplify:

config_option: T_TRANSITIONAL T_EOL
{
        current_entry->sym->transitional = true;
        printd(DEBUG_PARSE, "%s:%d:transitional\n", cur_filename, 
cur_lineno);
};

...it would mean the config options look this way:

config OLD_OPTION
     bool
     transitional

(If not, menu_set_type() does already contain a check for whether the
type has already been set.)

> +
>   config_option: default expr if_expr T_EOL
>   {
>   	menu_add_expr(P_DEFAULT, $2, $3);
> @@ -482,6 +493,43 @@ assign_val:
>   
>   %%
>   
> +/**
> + * transitional_check_sanity - check transitional symbols have no other
> + *			       properties
> + *
> + * @menu: menu of the potentially transitional symbol
> + *
> + * Return: -1 if an error is found, 0 otherwise.
> + */
> +static int transitional_check_sanity(const struct menu *menu)
> +{
> +	struct property *prop;
> +
> +	if (!menu->sym || !menu->sym->transitional)
> +		return 0;
> +
> +	/* Check for depends and visible conditions. */
> +	if ((menu->dep && !expr_is_yes(menu->dep)) ||
> +	    (menu->visibility && !expr_is_yes(menu->visibility))) {
> +		fprintf(stderr, "%s:%d: error: %s",
> +			menu->filename, menu->lineno,
> +			"transitional symbols can only have help sections\n");
> +		return -1;
> +	}
> +
> +	/* Check for any property other than "help". */
> +	for (prop = menu->sym->prop; prop; prop = prop->next) {
> +		if (prop->type != P_COMMENT) {
> +			fprintf(stderr, "%s:%d: error: %s",
> +				prop->filename, prop->lineno,
> +				"transitional symbols can only have help sections\n");
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * choice_check_sanity - check sanity of a choice member
>    *
> @@ -558,6 +606,9 @@ void conf_parse(const char *name)
>   		if (menu->sym && sym_check_deps(menu->sym))
>   			yynerrs++;
>   
> +		if (transitional_check_sanity(menu))
> +			yynerrs++;
> +
>   		if (menu->sym && sym_is_choice(menu->sym)) {
>   			menu_for_each_sub_entry(child, menu)
>   				if (child->sym && choice_check_sanity(child))
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 26ab10c0fd76..b822c0c897e5 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -447,6 +447,9 @@ void sym_calc_value(struct symbol *sym)
>   	if (sym->visible != no)
>   		sym->flags |= SYMBOL_WRITE;
>   
> +	/* Calculate usable flag */
> +	sym->usable = (sym->visible != no || sym->transitional);
> +

Is this actually ever used outside of this function? (IOW could this
just be a local variable instead of a sym-> flag/member?) Or do we need
to set it here because sym_calc_value() calls itself recursively? To me
it looks like we only ever access sym->usable for the "sym" that was
passed as an argument to the function.

>   	/* set default if recursively called */
>   	sym->curr = newval;
>   
> @@ -459,13 +462,15 @@ void sym_calc_value(struct symbol *sym)
>   			sym_calc_choice(choice_menu);
>   			newval.tri = sym->curr.tri;
>   		} else {
> -			if (sym->visible != no) {
> +			if (sym->usable) {
>   				/* if the symbol is visible use the user value
>   				 * if available, otherwise try the default value
>   				 */
>   				if (sym_has_value(sym)) {
> +					tristate value = sym->transitional ?
> +						sym->def[S_DEF_USER].tri : sym->visible;
>   					newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri,
> -							      sym->visible);
> +							      value);

This looks a bit odd to me. Just thinking out loud: your new logic is
there to be able to use a value even though it's not visible. In the
case where it's transitional you use the .config value instead of the
condition that makes it visible.

Could you simply change sym_calc_visibility() instead to always return
'yes' when the symbol is transitional? Wouldn't that simplify everything
in sym_calc_value()?

>   					goto calc_newval;
>   				}
>   			}
> @@ -497,7 +502,7 @@ void sym_calc_value(struct symbol *sym)
>   	case S_STRING:
>   	case S_HEX:
>   	case S_INT:
> -		if (sym->visible != no && sym_has_value(sym)) {
> +		if (sym->usable && sym_has_value(sym)) {
>   			newval.val = sym->def[S_DEF_USER].val;
>   			break;
>   		}

Ok.

> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> index a91abb8f6840..345c334ce680 100644
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -232,6 +232,37 @@ applicable everywhere (see syntax).
>     enables the third modular state for all config symbols.
>     At most one symbol may have the "modules" option set.
>   
> +- transitional attribute: "transitional"
> +  This declares the symbol as transitional, meaning it should be processed
> +  during configuration but omitted from newly written .config files.
> +  Transitional symbols are useful for backward compatibility during config
> +  option migrations - they allow olddefconfig to process existing .config
> +  files while ensuring the old option doesn't appear in new configurations.
> +
> +  A transitional symbol:
> +  - Has no prompt (is not visible to users in menus)
> +  - Is processed normally during configuration (values are read and used)
> +  - Can be referenced in default expressions of other symbols
> +  - Is not written to new .config files
> +  - Cannot have any other properties (it is a pass-through option)
> +
> +  Example migration from OLD_NAME to NEW_NAME::
> +
> +    config NEW_NAME
> +	bool "New option name"
> +	default OLD_NAME
> +	help
> +	  This replaces the old CONFIG_OLD_NAME option.
> +
> +    config OLD_NAME
> +	transitional bool
> +	help
> +	  Transitional config for OLD_NAME to NEW_NAME migration.
> +
> +  With this setup, existing .config files with "CONFIG_OLD_NAME=y" will
> +  result in "CONFIG_NEW_NAME=y" being set, while CONFIG_OLD_NAME will be
> +  omitted from newly written .config files.
> +
>   Menu dependencies
>   -----------------
>   


Vegard

Powered by blists - more mailing lists