init: keep boot_command_line after init

Message ID 20230726143348.724611-1-arnd@kernel.org
State New
Headers
Series init: keep boot_command_line after init |

Commit Message

Arnd Bergmann July 26, 2023, 2:33 p.m. UTC
  From: Arnd Bergmann <arnd@arndb.de>

The boot command line is not available after the init section gets discarded,
so adding a permanent reference to it causes a link time warning:

WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x2 (section: .text) -> boot_command_line (section: .init.data)
ERROR: modpost: Section mismatches detected.
Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.

Change it to __ro_after_init to prevent it from getting freed.

Fixes: cf9eca90a3397 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/init.h | 2 +-
 init/main.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Nick Desaulniers July 26, 2023, 4:25 p.m. UTC | #1
On Wed, Jul 26, 2023 at 7:33 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The boot command line is not available after the init section gets discarded,
> so adding a permanent reference to it causes a link time warning:
>
> WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x2 (section: .text) -> boot_command_line (section: .init.data)

cmdline_load_proc_show is probably inlined, but should it also be
marked __init?  It's lone call site seems to be __init AFAICT.

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ERROR: modpost: Section mismatches detected.
> Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
>
> Change it to __ro_after_init to prevent it from getting freed.
>
> Fixes: cf9eca90a3397 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/init.h | 2 +-
>  init/main.c          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 9a5973324072b..4e97a7a29a4d7 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -143,7 +143,7 @@ struct file_system_type;
>
>  /* Defined in init/main.c */
>  extern int do_one_initcall(initcall_t fn);
> -extern char __initdata boot_command_line[];
> +extern char boot_command_line[];
>  extern char *saved_command_line;
>  extern unsigned int saved_command_line_len;
>  extern unsigned int reset_devices;
> diff --git a/init/main.c b/init/main.c
> index c946ab87783a1..981170da0b1cd 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
>  void (*__initdata late_time_init)(void);
>
>  /* Untouched command line saved by arch-specific code. */
> -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
>  /* Untouched saved command line (eg. for /proc) */
>  char *saved_command_line __ro_after_init;
>  unsigned int saved_command_line_len __ro_after_init;
> --
> 2.39.2
>
  
Arnd Bergmann July 26, 2023, 5:02 p.m. UTC | #2
On Wed, Jul 26, 2023, at 18:25, Nick Desaulniers wrote:
> On Wed, Jul 26, 2023 at 7:33 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The boot command line is not available after the init section gets discarded,
>> so adding a permanent reference to it causes a link time warning:
>>
>> WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x2 (section: .text) -> boot_command_line (section: .init.data)
>
> cmdline_load_proc_show is probably inlined, but should it also be
> marked __init?  It's lone call site seems to be __init AFAICT.
>

No, that's not what it does: cmdline_load_proc_show() is called
when someone reads /proc/cmdline_load from userspace. It's only
the function that creates this procfs file that is __init, but the
call happens later.

      Arnd
  
Paul E. McKenney July 26, 2023, 6:53 p.m. UTC | #3
On Wed, Jul 26, 2023 at 07:02:55PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 26, 2023, at 18:25, Nick Desaulniers wrote:
> > On Wed, Jul 26, 2023 at 7:33 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> The boot command line is not available after the init section gets discarded,
> >> so adding a permanent reference to it causes a link time warning:
> >>
> >> WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x2 (section: .text) -> boot_command_line (section: .init.data)
> >
> > cmdline_load_proc_show is probably inlined, but should it also be
> > marked __init?  It's lone call site seems to be __init AFAICT.
> >
> 
> No, that's not what it does: cmdline_load_proc_show() is called
> when someone reads /proc/cmdline_load from userspace. It's only
> the function that creates this procfs file that is __init, but the
> call happens later.

Thank you all!  I have declared a tie between Stephen Rothwell and Arnd
Bergmann for this fix.  Please let me know if you are uncomfortable
with these changes being squashed into the original with your guys'
Co-developed-by.  I also added Nick's Reviewed-by, please see below.

							Thanx, Paul

------------------------------------------------------------------------

commit de2f542cfbec295ac0f9b6a832d7b3ba20df391f
Author: Paul E. McKenney <paulmck@kernel.org>
Date:   Fri Jul 21 16:05:38 2023 -0700

    fs/proc: Add /proc/cmdline_load for boot loader arguments
    
    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 supplied by the boot loader.
    This is especially important when these parameters are presented to the
    boot loader by automation that might gather them from diverse sources.
    
    Therefore, provide a /proc/cmdline_load file that shows only those kernel
    boot parameters supplied by the boot loader.
    
    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.
    
    Co-developed-by: Stephen Rothwell <sfr@canb.auug.org.au>
    Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
    Co-developed-by: Arnd Bergmann <arnd@kernel.org>
    Signed-off-by: Arnd Bergmann <arnd@kernel.org>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
    Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
    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>

diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index a6f76121955f..1d0ef9d2949d 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -3,6 +3,7 @@
 #include <linux/init.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <asm/setup.h>
 #include "internal.h"
 
 static int cmdline_proc_show(struct seq_file *m, void *v)
@@ -12,6 +13,13 @@ static int cmdline_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int cmdline_load_proc_show(struct seq_file *m, void *v)
+{
+	seq_puts(m, boot_command_line);
+	seq_putc(m, '\n');
+	return 0;
+}
+
 static int __init proc_cmdline_init(void)
 {
 	struct proc_dir_entry *pde;
@@ -19,6 +27,11 @@ static int __init proc_cmdline_init(void)
 	pde = proc_create_single("cmdline", 0, NULL, cmdline_proc_show);
 	pde_make_permanent(pde);
 	pde->size = saved_command_line_len + 1;
+	if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE)) {
+		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;
+	}
 	return 0;
 }
 fs_initcall(proc_cmdline_init);
diff --git a/include/linux/init.h b/include/linux/init.h
index 266c3e1640d4..c42a277db2da 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,9 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
+#include <linux/cache.h>
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +146,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[] __ro_after_init;
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index ad920fac325c..2121685c479a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;
  

Patch

diff --git a/include/linux/init.h b/include/linux/init.h
index 9a5973324072b..4e97a7a29a4d7 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -143,7 +143,7 @@  struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[];
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index c946ab87783a1..981170da0b1cd 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@  EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;