| 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: <20210325193216.GC16123@willie-the-truck>
Date: Thu, 25 Mar 2021 19:32:17 +0000
From: Will Deacon <will@...nel.org>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Michael Ellerman <mpe@...erman.id.au>, danielwa@...co.com,
robh@...nel.org, daniel@...pelevich.san-francisco.ca.us,
linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-arch@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use
generically defined boot cmdline manipulation
On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote:
>
>
> Le 03/03/2021 à 18:57, Will Deacon a écrit :
> > On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote:
> > > Most architectures have similar boot command line manipulation
> > > options. This patchs adds the definition in init/Kconfig, gated by
> > > CONFIG_HAVE_CMDLINE that the architectures can select to use them.
> > >
> > > In order to use this, a few architectures will have to change their
> > > CONFIG options:
> > > - riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER
> > > - architectures using CONFIG_CMDLINE_OVERRIDE or
> > > CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE.
> > >
> > > Architectures also have to define CONFIG_DEFAULT_CMDLINE.
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> > > ---
> > > init/Kconfig | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > >
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index 22946fe5ded9..a0f2ad9467df 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT
> > > Maximum of each of the number of arguments and environment
> > > variables passed to init from the kernel command line.
> > > +config HAVE_CMDLINE
> > > + bool
> > > +
> > > +config CMDLINE_BOOL
> > > + bool "Default bootloader kernel arguments"
> > > + depends on HAVE_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter
> > will use CONFIG_CMDLINE if it fails to get anything from the bootloader,
> > which sounds like the same scenario.
> >
> > > +config CMDLINE
> > > + string "Initial kernel command string"
> >
> > s/Initial/Default
> >
> > which is then consistent with the rest of the text here.
> >
> > > + depends on CMDLINE_BOOL
> >
> > Ah, so this is a bit different and I don't think lines-up with the
> > CMDLINE_BOOL help text.
> >
> > > + default DEFAULT_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > (same stale text)
> >
> > > +choice
> > > + prompt "Kernel command line type" if CMDLINE != ""
> > > + default CMDLINE_FROM_BOOTLOADER
> > > + help
> > > + Selects the way you want to use the default kernel arguments.
> >
> > How about:
> >
> > "Determines how the default kernel arguments are combined with any
> > arguments passed by the 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
> >
> > Can we rename this to CMDLINE_APPEND, please? There is code in the tree
> > which disagrees about what CMDLINE_EXTEND means, so that will need be
> > to be updated to be consistent (e.g. the EFI stub parsing order). Having
> > the generic option with a different name means we won't accidentally end
> > up with the same inconsistent behaviours.
>
> Argh, yes. Seems like the problem is even larger than that IIUC:
>
> - For ARM it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For Powerpc it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For SH it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For EFI it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For OF it means to append the CONFIG_CMDLINE to the bootloader arguments
>
> So what happens on ARM for instance when it selects CONFIG_OF for instance ?
I think ARM gets different behaviour depending on whether it uses ATAGs or
FDT.
> Or should we consider that EXTEND means APPEND or PREPEND, no matter which ?
> Because EXTEND is for instance used for:
>
> config INITRAMFS_FORCE
> bool "Ignore the initramfs passed by the bootloader"
> depends on CMDLINE_EXTEND || CMDLINE_FORCE
Oh man, I didn't spot that one :(
I think I would make the generic options explicit: either APPEND or PREPEND.
Then architectures which choose to define CMDLINE_EXTEND in their Kconfigs
can select the generic option that matches their behaviour.
INITRAMFS_FORCE sounds like it should depend on APPEND (assuming that means
CONFIG_CMDLINE is appended to the bootloader arguments).
Will
Powered by blists - more mailing lists