Re: [PATCH RFC bootconfig] 2/2] fs/proc: Add /proc/cmdline_image for embedded arguments

Masami Hiramatsu
2023-07-29T14:23:58Z

Hi Paul,

On Thu, 27 Jul 2023 20:37:01 -0700
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> In kernels built with CONFIG_BOOT_CONFIG_FORCE=y, /proc/cmdline will show
> all kernel boot parameters, both those supplied by the boot loader and
> those embedded in the kernel image. This works well for those who just
> want to see all of the kernel boot parameters, but is not helpful to those
> who need to see only those parameters that were embedded into the kernel
> image. This is especially important in situations where there are many
> kernel images for different kernel versions and kernel configurations,
> all of which opens the door to a great deal of human error.

There is /proc/bootconfig file which shows all bootconfig entries and is
formatted as easily filter by grep (or any other line-based commands).
(e.g. `grep ^kernel\\. /proc/cmdline` will filter all kernel cmdline
parameters in the bootconfig)
Could you clarify the reason why you need a dump of bootconfig file?

Thank you,

>
> Therefore, provide a /proc/cmdline_image file that shows only those kernel
> boot parameters that were embedded in the kernel image. The output
> is in boot-image format, which allows easy reconcilation against the
> boot-config source file.
>
> Why put this in /proc? Because it is quite similar to /proc/cmdline, so
> it makes sense to put it in the same place that /proc/cmdline is located.
>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: <linux-fsdevel@vger.kernel.org>
>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---
> fs/proc/cmdline.c | 12 ++++++++++++
> include/linux/init.h | 11 ++++++-----
> init/main.c | 9 +++++++++
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
> index 1d0ef9d2949d..4ab5223198cb 100644
> --- a/fs/proc/cmdline.c
> +++ b/fs/proc/cmdline.c
> @@ -20,6 +20,15 @@ static int cmdline_load_proc_show(struct seq_file *m, void *v)
> return 0;
> }
>
> +static int cmdline_image_proc_show(struct seq_file *m, void *v)
> +{
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> + seq_puts(m, saved_bootconfig_string);
> + seq_putc(m, '\n');
> +#endif
> + return 0;
> +}
> +
> static int __init proc_cmdline_init(void)
> {
> struct proc_dir_entry *pde;
> @@ -31,6 +40,9 @@ static int __init proc_cmdline_init(void)
> pde = proc_create_single("cmdline_load", 0, NULL, cmdline_load_proc_show);
> pde_make_permanent(pde);
> pde->size = strnlen(boot_command_line, COMMAND_LINE_SIZE) + 1;
> + pde = proc_create_single("cmdline_image", 0, NULL, cmdline_image_proc_show);
> + pde_make_permanent(pde);
> + pde->size = strnlen(saved_bootconfig_string, COMMAND_LINE_SIZE) + 1;
> }
> return 0;
> }
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 29e75bbe7984..c075983c5015 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -14,7 +14,7 @@
> #define __noinitretpoline
> #endif
>
> -/* These macros are used to mark some functions or
> +/* These macros are used to mark some functions or
> * initialized data (doesn't apply to uninitialized data)
> * as `initialization' functions. The kernel can take this
> * as hint that the function is used only during the initialization
> @@ -22,7 +22,7 @@
> *
> * Usage:
> * For functions:
> - *
> + *
> * You should add __init immediately before the function name, like:
> *
> * static void __init initme(int x, int y)
> @@ -148,6 +148,7 @@ extern char boot_command_line[];
> extern char *saved_command_line;
> extern unsigned int saved_command_line_len;
> extern unsigned int reset_devices;
> +extern char saved_bootconfig_string[];
>
> /* used by init/main.c */
> void setup_arch(char **);
> @@ -184,7 +185,7 @@ extern void (*late_time_init)(void);
> extern bool initcall_debug;
>
> #endif
> -
> +
> #ifndef MODULE
>
> #ifndef __ASSEMBLY__
> @@ -192,8 +193,8 @@ extern bool initcall_debug;
> /*
> * initcalls are now grouped by functionality into separate
> * subsections. Ordering inside the subsections is determined
> - * by link order.
> - * For backwards compatibility, initcall() puts the call in
> + * by link order.
> + * For backwards compatibility, initcall() puts the call in
> * the device init subsection.
> *
> * The `id' arg to __define_initcall() is needed so that multiple initcalls
> diff --git a/init/main.c b/init/main.c
> index 2121685c479a..981170da0b1c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -146,6 +146,11 @@ static char *extra_command_line;
> /* Extra init arguments */
> static char *extra_init_args;
>
> +/* Untouched boot-config string */
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> +char saved_bootconfig_string[COMMAND_LINE_SIZE] __ro_after_init;
> +#endif
> +
> #ifdef CONFIG_BOOT_CONFIG
> /* Is bootconfig on command line? */
> static bool bootconfig_found;
> @@ -435,6 +440,10 @@ static void __init setup_boot_config(void)
> return;
> }
>
> +#ifdef CONFIG_BOOT_CONFIG_FORCE
> + strncpy(saved_bootconfig_string, data, COMMAND_LINE_SIZE);
> +#endif
> +
> if (size >= XBC_DATA_MAX) {
> pr_err("bootconfig size %ld greater than max size %d\n",
> (long)size, XBC_DATA_MAX);
> --
> 2.40.1
>

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>