Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params

From: Ard Biesheuvel <ardb@kernel.org>
To: Chester Lin <clin@suse.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Mimi Zohar <zohar@linux.ibm.com>,
	dmitry.kasatkin@gmail.com, Jonathan Corbet <corbet@lwn.net>,
	Mark Rutland <mark.rutland@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	linux-integrity <linux-integrity@vger.kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	"Lee, Chun-Yi" <jlee@suse.com>
Subject: Re: [PATCH 1/6] efistub: pass uefi secureboot flag via fdt params
Date: Mon, 12 Oct 2020 10:20:15 +0200	[thread overview]
Message-ID: <CAMj1kXErMjbvwuP6YhgaZ4M47vzhYkFJb2kYC+h7Odr4Zu3eSA@mail.gmail.com> (raw)
In-Reply-To: <20201005022014.GA5112@linux-8mug>

On Mon, 5 Oct 2020 at 04:20, Chester Lin <clin@suse.com> wrote:
>
> On Mon, Sep 14, 2020 at 04:05:22PM +0800, Chester Lin wrote:
> > Hi Ard,
> >
> > On Fri, Sep 11, 2020 at 06:01:09PM +0300, Ard Biesheuvel wrote:
> > > On Fri, 4 Sep 2020 at 10:29, Chester Lin <clin@suse.com> wrote:
> > > >
> > > > Add a new UEFI parameter: "linux,uefi-secure-boot" in fdt boot params
> > > > as other architectures have done in their own boot data. For example,
> > > > the boot_params->secure_boot in x86.
> > > >
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > >
> > > Why do we need this flag? Can't the OS simply check the variable directly?
> > >
> >
> > In fact, there's a difficulty to achieve this.
> >
> > When linux kernel is booting on ARM, the runtime services are enabled later on.
> > It's done by arm_enable_runtime_services(), which is registered as an early_initcall.
> > Before it calls efi_native_runtime_setup(), all EFI runtime callbacks are still
> > NULL so calling efi.get_variable() will cause NULL pointer dereference.
> >
> > There's a case that arch_ima_get_secureboot() can be called in early boot stage.
> > For example, when you try to set "ima_appraise=off" in kernel command line, it's
> > actually handled early:
> >
> > [    0.000000] Kernel command line: BOOT_IMAGE=/boot/Image-5.9.0-rc3-9.gdd61cda-
> > vanilla root=UUID=a88bfb80-8abb-425c-a0f3-ad317465c28b splash=silent mitigations
> > =auto ignore_loglevel earlycon=pl011,mmio,0x9000000 console=ttyAMA0 ima_appraise=off
> > [    0.000000] ima: Secure boot enabled: ignoring ima_appraise=off boot parameter option
> > [    0.000000] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear)
> >
> > However EFI services are remapped and enabled afterwards.
> >
> > [    0.082286] rcu: Hierarchical SRCU implementation.
> > [    0.089592] Remapping and enabling EFI services.
> > [    0.097509] smp: Bringing up secondary CPUs ...
> >
> > Another problem is that efi_rts_wq is created in subsys_initcall so we have to
> > wait for both EFI services mapping and the workqueue get initiated before calling
> > efi.get_variable() on ARM.
> >
> > The only way I can think of is to put a flag via fdt params. May I have your
> > suggestions? I will appreciate if there's any better approach.
> >
> > Thanks,
> > Chester
>
> Ping. May I have some suggestions here?
>

IMA itself is initialized as a late initcall. The only reason you see
this message early is because this is where the parsing of the command
line parameter happens.

I'll send out a patch with a proposed solution for this issue.