From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Ard Biesheuvel <ardb@kernel.org>, linux-efi@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, lersek@redhat.com,
leif@nuviainc.com, pjones@redhat.com, mjg59@google.com,
agraf@csgraf.de, ilias.apalodimas@linaro.org,
daniel.kiper@oracle.com
Subject: Re: [PATCH 2/2] efi/libstub: take noinitrd cmdline argument into account for devpath initrd
Date: Thu, 6 Feb 2020 19:33:08 +0100 [thread overview]
Message-ID: <ea0b44d5-c9e4-943a-da81-8b4c8f1a371d@gmx.de> (raw)
In-Reply-To: <20200206140352.6300-3-ardb@kernel.org>
On 2/6/20 3:03 PM, Ard Biesheuvel wrote:
> One of the advantages of using what basically amounts to a callback
> interface into the bootloader for loading the initrd is that it provides
> a natural place for the bootloader or firmware to measure the initrd
> contents while they are being passed to the kernel.
>
> Unfortunately, this is not a guarantee that the initrd will in fact be
> loaded and its /init invoked by the kernel, since the command line may
> contain the 'noinitrd' option, in which case the initrd is ignored, but
> this will not be reflected in the PCR that covers the initrd measurement.
Does PCR here refer to the TPM Platform Configuration Register?
>
> This could be addressed by measuring the command line as well, and
> including that PCR in the attestation policy, but this locks down the
> command line completely, which may be too restrictive.
>
> So let's take the noinitrd argument into account in the stub, too. This
> forces the PCR that covers the initrd to assume a different value when
> noinitrd is passed, allowing an attestation policy to disregard the
> command line if there is no need to take its measurement into account
> for other reasons.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/firmware/efi/libstub/arm-stub.c | 23 +++++-----
> drivers/firmware/efi/libstub/efi-stub-helper.c | 9 ++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 45 +++++++++++---------
> 4 files changed, 47 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
> index 1db943c1ba2b..5e8f16cf016e 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -256,18 +256,21 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table_arg,
> if (!fdt_addr)
> pr_efi("Generating empty DTB\n");
>
> - max_addr = efi_get_max_initrd_addr(dram_base, *image_addr);
> - status = efi_load_initrd_devpath(&initrd_addr, &initrd_size, max_addr);
> - if (status == EFI_SUCCESS)
> - pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
> - else if (status == EFI_NOT_FOUND) {
> - status = efi_load_initrd(image, ULONG_MAX, max_addr,
> - &initrd_addr, &initrd_size);
> + if (!noinitrd()) {
> + max_addr = efi_get_max_initrd_addr(dram_base, *image_addr);
> + status = efi_load_initrd_devpath(&initrd_addr, &initrd_size,
> + max_addr);
> if (status == EFI_SUCCESS)
> - pr_efi("Loaded initrd from command line option\n");
> + pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
> + else if (status == EFI_NOT_FOUND) {
> + status = efi_load_initrd(image, ULONG_MAX, max_addr,
> + &initrd_addr, &initrd_size);
> + if (status == EFI_SUCCESS)
> + pr_efi("Loaded initrd from command line option\n");
> + }
> + if (status != EFI_SUCCESS)
> + pr_efi_err("Failed to load initrd!\n");
> }
> - if (status != EFI_SUCCESS)
> - pr_efi_err("Failed to load initrd!\n");
>
> efi_random_get_seed();
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index eaf45ea749b3..367575fb8424 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> static bool __efistub_global efi_nochunk;
> static bool __efistub_global efi_nokaslr;
> +static bool __efistub_global efi_noinitrd;
> static bool __efistub_global efi_quiet;
> static bool __efistub_global efi_novamap;
> static bool __efistub_global efi_nosoftreserve;
> @@ -25,6 +26,10 @@ bool __pure nokaslr(void)
> {
> return efi_nokaslr;
> }
> +bool __pure noinitrd(void)
> +{
> + return efi_noinitrd;
> +}
> bool __pure is_quiet(void)
> {
> return efi_quiet;
> @@ -71,6 +76,10 @@ efi_status_t efi_parse_options(char const *cmdline)
> if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
> efi_nokaslr = true;
>
> + str = strstr(cmdline, "noinitrd");
> + if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
> + efi_noinitrd = true;
> +
> str = strstr(cmdline, "quiet");
> if (str == cmdline || (str && str > cmdline && *(str - 1) == ' '))
> efi_quiet = true;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index fbf9f9442eed..29a5d0e9554a 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -44,6 +44,7 @@
>
> extern bool __pure nochunk(void);
> extern bool __pure nokaslr(void);
> +extern bool __pure noinitrd(void);
> extern bool __pure is_quiet(void);
> extern bool __pure novamap(void);
>
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 7f38f95676dd..9d86c0949b3c 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -419,26 +419,28 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
> if (status != EFI_SUCCESS)
> goto fail2;
>
> - /*
> - * The initrd loaded from the Linux initrd vendor device
> - * path should take precedence, as we don't want the
> - * [unverified] command line to override the initrd
> - * supplied by the [potentially verified] firmware.
> - */
> - status = efi_load_initrd_devpath(&ramdisk_addr, &ramdisk_size,
> - above4g ? ULONG_MAX
> - : hdr->initrd_addr_max);
> - if (status == EFI_NOT_FOUND)
> - status = efi_load_initrd(image, hdr->initrd_addr_max,
> - above4g ? ULONG_MAX
> - : hdr->initrd_addr_max,
> - &ramdisk_addr, &ramdisk_size);
> - if (status != EFI_SUCCESS)
> - goto fail2;
> - hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
> - hdr->ramdisk_size = ramdisk_size & 0xffffffff;
> - boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
> - boot_params->ext_ramdisk_size = (u64)ramdisk_size >> 32;
> + if (!noinitrd()) {
> + /*
> + * The initrd loaded from the Linux initrd vendor device
> + * path should take precedence, as we don't want the
> + * [unverified] command line to override the initrd
> + * supplied by the [potentially verified] firmware.
> + */
> + status = efi_load_initrd_devpath(&ramdisk_addr, &ramdisk_size,
> + above4g ? ULONG_MAX
> + : hdr->initrd_addr_max);
> + if (status == EFI_NOT_FOUND)
> + status = efi_load_initrd(image, hdr->initrd_addr_max,
> + above4g ? ULONG_MAX
> + : hdr->initrd_addr_max,
> + &ramdisk_addr, &ramdisk_size);
> + if (status != EFI_SUCCESS)
> + goto fail2;
> + hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
> + hdr->ramdisk_size = ramdisk_size & 0xffffffff;
> + boot_params->ext_ramdisk_image = (u64)ramdisk_addr >> 32;
> + boot_params->ext_ramdisk_size = (u64)ramdisk_size >> 32;
> + }
>
> efi_stub_entry(handle, sys_table, boot_params);
> /* not reached */
> @@ -743,7 +745,8 @@ struct boot_params *efi_main(efi_handle_t handle,
> ((u64)boot_params->ext_cmd_line_ptr << 32));
> efi_parse_options((char *)cmdline_paddr);
>
> - if (!hdr->ramdisk_size && !boot_params->ext_ramdisk_size) {
> + if (!hdr->ramdisk_size && !boot_params->ext_ramdisk_size &&
> + !noinitrd()) {
> unsigned long max = hdr->initrd_addr_max;
> unsigned long addr, size;
>
>
next prev parent reply other threads:[~2020-02-06 18:33 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-06 14:03 [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path Ard Biesheuvel
2020-02-06 18:26 ` Heinrich Schuchardt
2020-02-06 18:46 ` Ilias Apalodimas
2020-02-06 19:15 ` Heinrich Schuchardt
2020-02-06 20:09 ` Ilias Apalodimas
2020-02-06 22:49 ` Heinrich Schuchardt
2020-02-07 7:35 ` Ilias Apalodimas
2020-02-06 22:35 ` Ard Biesheuvel
2020-02-07 0:01 ` Heinrich Schuchardt
2020-02-07 0:21 ` Ard Biesheuvel
2020-02-07 0:57 ` Heinrich Schuchardt
2020-02-07 8:12 ` Ard Biesheuvel
2020-02-07 13:30 ` Heinrich Schuchardt
2020-02-07 13:58 ` Ard Biesheuvel
2020-02-07 14:18 ` Alexander Graf
2020-02-07 15:30 ` Ard Biesheuvel
2020-02-07 15:35 ` Heinrich Schuchardt
2020-02-07 11:09 ` Laszlo Ersek
2020-02-07 11:03 ` Laszlo Ersek
2020-02-07 9:48 ` Laszlo Ersek
2020-02-07 12:36 ` Ard Biesheuvel
2020-02-10 14:26 ` Laszlo Ersek
2020-02-09 6:39 ` Lukas Wunner
2020-02-09 11:35 ` Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 2/2] efi/libstub: take noinitrd cmdline argument into account for devpath initrd Ard Biesheuvel
2020-02-06 18:33 ` Heinrich Schuchardt [this message]
2020-02-06 23:44 ` Ard Biesheuvel
2020-02-12 16:01 ` Peter Jones
2020-02-07 9:09 ` [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Laszlo Ersek
2020-02-07 9:22 ` Laszlo Ersek
2020-02-07 12:23 ` Ard Biesheuvel
2020-02-07 16:20 ` James Bottomley
2020-02-07 18:31 ` Ard Biesheuvel
2020-02-07 19:54 ` James Bottomley
2020-02-07 20:03 ` Ard Biesheuvel
2020-02-07 18:45 ` Arvind Sankar
2020-02-07 19:47 ` Ard Biesheuvel
2020-02-07 20:26 ` Arvind Sankar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ea0b44d5-c9e4-943a-da81-8b4c8f1a371d@gmx.de \
--to=xypron.glpk@gmx.de \
--cc=agraf@csgraf.de \
--cc=ardb@kernel.org \
--cc=daniel.kiper@oracle.com \
--cc=ilias.apalodimas@linaro.org \
--cc=leif@nuviainc.com \
--cc=lersek@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-efi@vger.kernel.org \
--cc=mjg59@google.com \
--cc=pjones@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).