Re: [PATCH v3 6/6] x86/setup: prepend embedded bootconfig cmdline before parse_early_param

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The patch selects `ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG`, but this Kconfig symbol is not defined anywhere in the kernel.
- [High] Unconditionally prepending the embedded bootconfig in `setup_arch()` breaks the precedence rule where an initrd bootconfig must completely override and replace an embedded bootconfig.
- [High] Prepending embedded bootconfig keys in-place to `boot_command_line` can silently truncate user-supplied bootloader arguments.
- [High] The 'bootconfig' command line parameter is parsed differently by `setup_arch()` and `setup_boot_config()`, causing a split-brain state where `bootconfig=0` disables early parameter application but still enables late parameter application.
--

commit 3b4a5034917523d388052b505ac2245c0d663702
Author: Breno Leitao <leitao@debian.org>

x86/setup: prepend embedded bootconfig cmdline before parse_early_param

Call xbc_prepend_embedded_cmdline() in setup_arch() right after the
CONFIG_CMDLINE merge and before strscpy(command_line, ...) so the
build-time-rendered embedded bootconfig "kernel" subtree is part of
boot_command_line by the time parse_early_param() runs. early_param()
handlers (mem=, earlycon=, loglevel=, ...) now see values supplied via
CONFIG_BOOT_CONFIG_EMBED_FILE without parsing bootconfig at runtime.

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0de23e6471973..8ab11199c16d5 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -127,6 +127,7 @@ config X86
>  	select ARCH_SUPPORTS_NUMA_BALANCING	if X86_64
>  	select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP	if NR_CPUS <= 4096
>  	select ARCH_SUPPORTS_CFI		if X86_64
> +	select ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG

[Severity: High]
Does ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG exist? 

This selects a Kconfig symbol that is not defined anywhere in the kernel. This 
is a Kconfig subsystem violation that can cause silent build failures or leave 
required features disabled because the selected symbol has no definition in any 
Kconfig file.

>  	select ARCH_USES_CFI_TRAPS		if X86_64 && CFI
>  	select ARCH_SUPPORTS_LTO_CLANG
>  	select ARCH_SUPPORTS_LTO_CLANG_THIN
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 46882ce79c3a4..003f8651db6c3 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
[ ... ]
> @@ -924,6 +926,20 @@ void __init setup_arch(char **cmdline_p)
>  	builtin_cmdline_added = true;
>  #endif
>  
> +	/*
> +	 * Match the runtime bootconfig parser's opt-in: only fold the
> +	 * embedded kernel.* keys into the cmdline when "bootconfig" is
> +	 * present on the command line, or CONFIG_BOOT_CONFIG_FORCE is set.
> +	 * setup_boot_config() bails out under the same condition, so the
> +	 * early prepend stays in lockstep with what the late runtime parser
> +	 * would have applied. CONFIG_BOOT_CONFIG_FORCE defaults to y when
> +	 * BOOT_CONFIG_EMBED is set, so on the default config the embedded
> +	 * keys are applied unconditionally.
> +	 */
> +	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE) ||
> +	    cmdline_find_option_bool(boot_command_line, "bootconfig"))

[Severity: High]
Could this result in a split-brain state for "bootconfig" parsing?

In setup_arch(), cmdline_find_option_bool("bootconfig") accurately rejects
boolean-false values like bootconfig=0 or bootconfig=off.

However, in init/main.c:setup_boot_config(), the lax pre-existing parser uses 
parse_args() which merely checks for the presence of the "bootconfig" key and
ignores its value. 

Passing bootconfig=0 will therefore skip the early embedded parameter injection
here, but mistakenly apply the embedded bootconfig later at runtime, causing an 
inconsistent system state.

> +		xbc_prepend_embedded_cmdline(boot_command_line, COMMAND_LINE_SIZE);

[Severity: High]
Can this unconditionally prepend the embedded bootconfig, breaking the
precedence rule where an initrd bootconfig must completely override it?

The intended logic in init/main.c:setup_boot_config() allows an initrd 
bootconfig to completely override the embedded one:

init/main.c:setup_boot_config() {
	if (!data) {
		data = xbc_get_embedded_bootconfig(&size);
		from_embedded = true;
	}
    ...
}

By prepending the embedded keys directly to boot_command_line early in 
setup_arch(), these keys bypass the override check and are inevitably processed 
by parse_early_param(). This permanently leaks the embedded keys into the boot 
parameters, causing conflicting or unwanted early parameters to be applied even 
when a valid initrd bootconfig is present.

[Severity: High]
Will prepending embedded bootconfig keys in-place to boot_command_line 
silently truncate user-supplied bootloader arguments?

boot_command_line is a fixed-size buffer bounded by COMMAND_LINE_SIZE. When 
xbc_prepend_embedded_cmdline() prepends the embedded bootconfig string, it must 
push the existing content to the right. 

If the combined length exceeds the buffer, the tail-end arguments 
(such as root= or init=) will be silently truncated, resulting in a kernel panic 
on boot.

>  
>  	strscpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>  	*cmdline_p = command_line;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-bootconfig_using_tools-v3-0-4ddd079a0696@debian.org?part=6