| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1564698745.4914.14.camel@alliedtelesis.co.nz>
Date: Thu, 1 Aug 2019 22:32:25 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: "christophe.leroy@....fr" <christophe.leroy@....fr>,
"paulus@...ba.org" <paulus@...ba.org>,
"mpe@...erman.id.au" <mpe@...erman.id.au>,
"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
"malat@...ian.org" <malat@...ian.org>
CC: "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] powerpc: Support CMDLINE_EXTEND
On Thu, 2019-08-01 at 08:14 +0200, Christophe Leroy wrote:
>
> Le 01/08/2019 à 04:12, Chris Packham a écrit :
> >
> > Bring powerpc in line with other architectures that support
> > extending or
> > overriding the bootloader provided command line.
> >
> > The current behaviour is most like CMDLINE_FROM_BOOTLOADER where
> > the
> > bootloader command line is preferred but the kernel config can
> > provide a
> > fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND
> > can
> > be used to append the CMDLINE from the kernel config to the one
> > provided
> > by the bootloader.
> >
> > Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> > ---
> > While I'm at it does anyone think it's worth getting rid of the
> > default CMDLINE
> > value if CMDLINE_BOOL and maybe CMDLINE_BOOL? Every defconfig in
> > the kernel
> > that sets CMDLINE_BOOL=y also sets CMDLINE to something other than
> > "console=ttyS0,9600 console=tty0 root=/dev/sda2". Removing
> > CMDLINE_BOOL and
> > unconditionally setting the default value of CMDLINE to "" would
> > clean up the
> > Kconfig even more.
> Note
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
> mmit/?id=cbe46bd4f5104552b612505b73d366f66efc2341
> which is already a step forward.
>
> I guess that default is for users selecting this option manually to
> get
> a first sensitive CMDLINE. But is it really worth it ?
>
I'm not even sure if it is working as intended right now. Even without
my changes if I use menuconfig and select CMDLINE_BOOL I end up with
CONFIG_CMDLINE="" in the resulting .config.
> >
> >
> > Changes in v2:
> > - incorporate ideas from Christope's patch https://patchwork.ozlabs
> > .org/patch/1074126/
> >
> > arch/powerpc/Kconfig | 20 +++++++++++++++++++-
> > arch/powerpc/kernel/prom_init.c | 26 +++++++++++++++++++++++++-
> > 2 files changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 77f6ebf97113..d413fe1b4058 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -852,15 +852,33 @@ config CMDLINE
> > some command-line options at build time by entering
> > them here. In
> > most cases you will need to specify the root device
> > here.
> >
> > +choice
> > + prompt "Kernel command line type" if CMDLINE != ""
> > + default CMDLINE_FROM_BOOTLOADER
> > +
> > +config CMDLINE_FROM_BOOTLOADER
> > + bool "Use bootloader kernel arguments if available"
> > + help
> > + Uses the command-line options passed by the boot loader.
> > If
> > + the boot loader doesn't provide any, the default kernel
> > command
> > + string provided in CMDLINE will be used.
> > +
> > +config CMDLINE_EXTEND
> > + bool "Extend bootloader kernel arguments"
> > + help
> > + The command-line arguments provided by the boot loader
> > will be
> > + appended to the default kernel command string.
> > +
> > config CMDLINE_FORCE
> > bool "Always use the default kernel command string"
> > - depends on CMDLINE_BOOL
> > help
> > Always use the default kernel command string, even if
> > the boot
> > loader passes other arguments to the kernel.
> > This is useful if you cannot or don't want to change
> > the
> > command-line options your boot loader passes to the
> > kernel.
> >
> > +endchoice
> > +
> > config EXTRA_TARGETS
> > string "Additional default image types"
> > help
> > diff --git a/arch/powerpc/kernel/prom_init.c
> > b/arch/powerpc/kernel/prom_init.c
> > index 514707ef6779..df29f141dbd2 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -310,6 +310,25 @@ static size_t __init prom_strlcpy(char *dest,
> > const char *src, size_t size)
> > return ret;
> > }
> >
> > +static size_t __init prom_strlcat(char *dest, const char *src,
> > size_t count)
> > +{
> > + size_t dsize = prom_strlen(dest);
> > + size_t len = prom_strlen(src);
> > + size_t res = dsize + len;
> > +
> > + /* This would be a bug */
> > + BUG_ON(dsize >= count);
> Has you pointed in another mail, BUG_ON() should be avoided here.
> I guess if the destination is already full, just return count:
>
> if (dsize >= count)
> return count;
>
Will fix in v3.
> >
> > +
> > + dest += dsize;
> > + count -= dsize;
> > + if (len >= count)
> > + len = count-1;
> > + memcpy(dest, src, len);
> > + dest[len] = 0;
> > + return res;
> > +
> > +}
> > +
> > #ifdef CONFIG_PPC_PSERIES
> > static int __init prom_strtobool(const char *s, bool *res)
> > {
> > @@ -761,8 +780,13 @@ static void __init early_cmdline_parse(void)
> > p = prom_cmd_line;
> > if ((long)prom.chosen > 0)
> > l = prom_getprop(prom.chosen, "bootargs", p,
> > COMMAND_LINE_SIZE-1);
> The above is pointless in case CONFIG_CMDLINE_FORCE is selected.
>
Will fix in v3.
> >
> > - if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] ==
> > '\0')) /* dbl check */
> > +
> > + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || l <= 0 || p[0] ==
> > '\0')
> > prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE,
> > sizeof(prom_cmd_line));
> If we can ensure that prom_cmd_line remains empty when
> CONFIG_CMDLINE_FORCE is selected (see above comment), then
> prom_strlcat() can be used in lieu of prom_strlcpy()
>
> >
> > + else if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
> > + prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
> > + sizeof(prom_cmd_line));
> > +
> > prom_printf("command line: %s\n", prom_cmd_line);
> >
> > #ifdef CONFIG_PPC64
> >
> Christophe
Powered by blists - more mailing lists