Messages in this thread | | | Date | Sat, 11 Feb 2017 21:06:03 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH] Staging: setup.c : boot loader kernel arguments are secured over cmdline.txt arguments for some systems |
| |
On Sun, 12 Feb 2017, Kishore Karanala wrote: 1;2802;0c The proper subsystem for x86 is x86 and not staging.
Also your subject line is a way too long sentence instead of a short and precise summary of the change.
> boot loader kernel arguments are secured over cmdline.txt > arguments for some systems
Repeating it does not make it any better.
What's missing here is an explanation WHY this changes is needed and which problem it solves.
> Signed-off-by: Kishore Karanala <kishore.karanala@gmail.com> > --- > arch/x86/Kconfig | 3 +++ > arch/x86/kernel/setup.c | 7 ++++++- > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index e487493..3077fb0 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -2224,6 +2224,9 @@ config CMDLINE > In most cases, the command line (whether built-in or provided > by the boot loader) should specify the device for the root > file system. > + In some of the systems boot loader arguments needs dominated over > + cmdline arguments in systems like automotive , this can be done using > + CMDLINE="!root=/dev/mmcblk0p1 ro"
I really have no idea what that sentence means. Also it does not explain what the exclamation mark stands for.
Aside of that the indentation of that paragraph is wrong.
> config CMDLINE_OVERRIDE > bool "Built-in command line overrides boot loader arguments" > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 4cfba94..5fa3194 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -962,11 +962,16 @@ void __init setup_arch(char **cmdline_p) > #ifdef CONFIG_CMDLINE_OVERRIDE > strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > #else > - if (builtin_cmdline[0]) { > + if (builtin_cmdline[0] != '!') { > /* append boot loader cmdline to builtin */ > strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE); > strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE); > strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE); > + } else { > + /* This will provide additional secuirty to cmdline */
I assume you mean security. What has this to do with security? The empty changelog does not explain it neither does this comment.
> + /* arguments not overriding bootloader arguments */
Multiline comments are formatted like this
/* * This is the first line of a multiline comment which * continues on the second line. */
> + strlcat(boot_command_line, " ", COMMAND_LINE_SIZE); > + strlcat(boot_command_line, &builtin_cmdline[1], COMMAND_LINE_SIZE);
So this is just the reverse order of the exsiting mechanism. What's the point of this exercise?
Thanks,
tglx
| |