Messages in this thread |  | | Date | Wed, 5 Feb 2020 09:06:10 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH v3] bootconfig: Only load bootconfig if "bootconfig" is on the kernel cmdline |
| |
On Tue, 4 Feb 2020 11:04:46 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > As the bootconfig is appended to the initrd it is not as easy to modify as > the kernel command line. If there's some issue with the kernel, and the > developer wants to boot a pristine kernel, it should not be needed to modify > the initrd to remove the bootconfig for a single boot. > > As bootconfig is silently added (if the admin does not know where to look > they may not know it's being loaded). It should be explicitly added to the > kernel cmdline. The loading of the bootconfig is only done if "bootconfig" > is on the kernel command line. This will let admins know that the kernel > command line is extended. > > Note, after adding printk()s for when the size is too great or the checksum > is wrong, exposed that the current method always looked for the boot config, > and if this size and checksum matched, it would parse it (as if either is > wrong a printk has been added to show this). It's better to only check this > if the boot config is asked to be looked for.
Thanks for adding this feature :) This looks good to me. Please add my ack when you fix the error message of "config=bootconfig".
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> > Link: https://lore.kernel.org/r/CAHk-=wjfjO+h6bQzrTf=YCZA53Y3EDyAs3Z4gEsT7icA3u_Psw@mail.gmail.com > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > Changes from v2: > - Use "bootconfig" instead of "config=bootconifg" > > Documentation/admin-guide/bootconfig.rst | 2 ++ > .../admin-guide/kernel-parameters.txt | 6 ++++ > init/main.c | 28 ++++++++++++++----- > 3 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst > index 4d617693c0c8..b342a6796392 100644 > --- a/Documentation/admin-guide/bootconfig.rst > +++ b/Documentation/admin-guide/bootconfig.rst > @@ -123,6 +123,8 @@ To remove the config from the image, you can use -d option as below:: > > # tools/bootconfig/bootconfig -d /boot/initrd.img-X.Y.Z > > +Then add "bootconfig" on the normal kernel command line to tell the > +kernel to look for the bootconfig at the end of the initrd file. > > Config File Limitation > ====================== > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index ade4e6ec23e0..b48c70ba9841 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -437,6 +437,12 @@ > no delay (0). > Format: integer > > + bootconfig [KNL] > + Extended command line options can be added to an initrd > + and this will cause the kernel to look for it. > + > + See Documentation/admin-guide/bootconfig.rst > + > bert_disable [ACPI] > Disable BERT OS support on buggy BIOSes. > > diff --git a/init/main.c b/init/main.c > index dd7da62d99a5..1465fb2d7277 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -336,28 +336,39 @@ u32 boot_config_checksum(unsigned char *p, u32 size) > return ret; > } > > -static void __init setup_boot_config(void) > +static void __init setup_boot_config(const char *cmdline) > { > u32 size, csum; > char *data, *copy; > + const char *p; > u32 *hdr; > > - if (!initrd_end) > + p = strstr(cmdline, "bootconfig"); > + if (!p || (p != cmdline && !isspace(*(p-1))) || > + (p[10] && !isspace(p[10]))) > return; > > + if (!initrd_end) > + goto not_found; > + > hdr = (u32 *)(initrd_end - 8); > size = hdr[0]; > csum = hdr[1]; > > - if (size >= XBC_DATA_MAX) > + if (size >= XBC_DATA_MAX) { > + pr_err("bootconfig size %d greater than max size %d\n", > + size, XBC_DATA_MAX); > return; > + } > > data = ((void *)hdr) - size; > if ((unsigned long)data < initrd_start) > - return; > + goto not_found; > > - if (boot_config_checksum((unsigned char *)data, size) != csum) > + if (boot_config_checksum((unsigned char *)data, size) != csum) { > + pr_err("bootconfig checksum failed\n"); > return; > + } > > copy = memblock_alloc(size + 1, SMP_CACHE_BYTES); > if (!copy) { > @@ -377,9 +388,12 @@ static void __init setup_boot_config(void) > /* Also, "init." keys are init arguments */ > extra_init_args = xbc_make_cmdline("init"); > } > + return; > +not_found: > + pr_err("config=bootconfig on command line, but no bootconfig found\n"); > } > #else > -#define setup_boot_config() do { } while (0) > +#define setup_boot_config(cmdline) do { } while (0) > #endif > > /* Change NUL term back to "=", to make "param" the whole string. */ > @@ -760,7 +774,7 @@ asmlinkage __visible void __init start_kernel(void) > pr_notice("%s", linux_banner); > early_security_init(); > setup_arch(&command_line); > - setup_boot_config(); > + setup_boot_config(command_line); > setup_command_line(command_line); > setup_nr_cpu_ids(); > setup_per_cpu_areas(); > -- > 2.20.1 >
-- Masami Hiramatsu <mhiramat@kernel.org>
|  |