linux-kernel - Re: [PATCH v2 1/2] arm64: cpufeatures: Fix handling of CONFIG

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 Mar 2021 09:33:07 +0000
From:   Will Deacon <will@...nel.org>
To:     Rob Herring <robh@...nel.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Max Uvarov <muvarov@...il.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Doug Anderson <dianders@...omium.org>,
        Tyler Hicks <tyhicks@...ux.microsoft.com>,
        Frank Rowand <frowand.list@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Palmer Dabbelt <palmer@...belt.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Android Kernel Team <kernel-team@...roid.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v2 1/2] arm64: cpufeatures: Fix handling of
 CONFIG_CMDLINE for idreg overrides

On Wed, Mar 03, 2021 at 04:30:21PM -0600, Rob Herring wrote:
> On Wed, Mar 3, 2021 at 7:50 AM Will Deacon <will@...nel.org> wrote:
> >
> > The built-in kernel commandline (CONFIG_CMDLINE) can be configured in
> > three different ways:
> >
> >   1. CMDLINE_FORCE: Use CONFIG_CMDLINE instead of any bootloader args
> >   2. CMDLINE_EXTEND: Append the bootloader args to CONFIG_CMDLINE
> >   3. CMDLINE_FROM_BOOTLOADER: Only use CONFIG_CMDLINE if there aren't
> >      any bootloader args.
> >
> > The early cmdline parsing to detect idreg overrides gets (2) and (3)
> > slightly wrong: in the case of (2) the bootloader args are parsed first
> > and in the case of (3) the CMDLINE is always parsed.
> >
> > Fix these issues by moving the bootargs parsing out into a helper
> > function and following the same logic as that used by the EFI stub.
> >
> > Reviewed-by: Marc Zyngier <maz@...nel.org>
> > Fixes: 33200303553d ("arm64: cpufeature: Add an early command-line cpufeature override facility")
> > Signed-off-by: Will Deacon <will@...nel.org>
> > ---
> >  arch/arm64/kernel/idreg-override.c | 44 +++++++++++++++++-------------
> >  1 file changed, 25 insertions(+), 19 deletions(-)
> 
> It bothers me a bit having command line handling here. It means how
> the command line is assembled in 2 places.

It's more than 2 places, it's also done in the EFI stub; see efi_pe_entry()
in drivers/firmware/efi/libstub/efi-stub.c. That's why I'm pushing on this,
because at the moment the command-line reported by /proc/cmdline doesn't
match the command-line that was parsed there because they stitch it together
in opposite orders when CMDLINE_EXTEND is used.

> I guess if we get rid of ambiguous 'extend' then it's better, but perhaps
> a better implementation would be an api get a specific command line
> parameter.  The main downside would be searching the DT again for each
> parameter if we can't store any data in between calls, but there's ways
> around that. PowerPC also needs similar functionality in
> disabled_on_cmdline().

Christophe's patches at least aim to put the assembling all in one place,
although we'll have to see whether or not it can be used in the EFI stub
environment.

Will