Re: [PATCH v5 7/7] x86/setup: prepend embedded bootconfig cmdline before parse_early_param

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Split-brain and silent loss of embedded bootconfig when an initrd bootconfig is present.
- [High] The check for 'bootconfig' in setup_arch() does not stop at the '--' separator, leading to a split-brain state if 'bootconfig' is passed strictly as an init argument.
--

commit 8df914967d65efb57f129f5003c1c8d584ad9df9
Author: Breno Leitao <leitao@debian.org>

x86/setup: prepend embedded bootconfig cmdline before parse_early_param

This patch modifies the x86 boot sequence to prepend embedded bootconfig
parameters to the kernel command line early in setup_arch(). It manually
checks for the "bootconfig" parameter before parse_early_param() runs,
and adjusts setup_boot_config() to avoid duplicating the command line
prepend later.

> The prepend necessarily runs before setup_boot_config() detects an
> initrd bootconfig, so an initrd cannot override the embedded "kernel"
> keys for early_param(). This is intentional: the embedded cmdline acts
> like a build-time CONFIG_CMDLINE. An initrd bootconfig's "kernel" keys
> never reached early_param() anyway (they apply late via
> extra_command_line), so nothing is lost -- the initrd keys still apply
> late, with last-wins keeping the embedded values in effect.

[Severity: High]
Does this accurately reflect the behavior when an initrd bootconfig is
present?

If both an embedded bootconfig and an initrd bootconfig are present,
setup_boot_config() handles it like this:

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

This means the embedded bootconfig data is entirely ignored for the runtime
tree, silently discarding any tree-structured configuration (like ftrace or
IMA) from the embedded bootconfig.

Furthermore, extra_command_line from the initrd is placed before
boot_command_line in saved_command_line. Since the embedded keys were
already prepended to boot_command_line in setup_arch(), they appear
after the initrd keys.

With last-wins semantics, the embedded kernel parameters override the
initrd kernel parameters, while the tree data comes exclusively from the
initrd.

> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 46882ce79c3a4..d69ba84c203f1 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c

[ ... ]

> @@ -924,6 +926,31 @@ 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()'s parse_args() loop treats any presence of
> +	 * the "bootconfig" key as opt-in (bare, =0, =1, ...), so check both
> +	 * forms here: cmdline_find_option_bool() matches the bare key,
> +	 * cmdline_find_option() matches "bootconfig=<anything>". Without
> +	 * the second check, "bootconfig=0" would skip the early prepend
> +	 * but still trigger the late runtime apply -- a split-brain state.
> +	 * CONFIG_BOOT_CONFIG_FORCE defaults to y when BOOT_CONFIG_EMBED is
> +	 * set, so on the default config the embedded keys are applied
> +	 * unconditionally.
> +	 */
> +	{
> +		char buf[8];
> +
> +		if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE) ||
> +		    cmdline_find_option_bool(boot_command_line, "bootconfig") ||
> +		    cmdline_find_option(boot_command_line, "bootconfig",
> +					buf, sizeof(buf)) >= 0)
> +			xbc_prepend_embedded_cmdline(boot_command_line,
> +						     COMMAND_LINE_SIZE);
> +	}

[Severity: High]
Could this create a split-brain state if "bootconfig" is passed strictly as
an init argument (e.g., after the "--" separator)?

The cmdline_find_option_bool() and cmdline_find_option() functions search
the entire command-line string and do not stop at the "--" separator.

If a user passes "-- bootconfig", setup_arch() will detect it and
prematurely prepend the embedded kernel parameters to the command line.

Later, the runtime parser parse_args() in setup_boot_config() strictly
stops parsing at "--". It will fail to find "bootconfig" and exit early
without initializing the bootconfig subsystem.

This leaves the system in a state where the embedded kernel parameters are
fully applied, but the bootconfig tree itself remains completely
uninitialized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-bootconfig_using_tools-v5-0-fd589a9cc5e3@debian.org?part=7