Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200424130531.30518-1-ardb@kernel.org> <20200424130531.30518-3-ardb@kernel.org>
In-Reply-To: <20200424130531.30518-3-ardb@kernel.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 24 Apr 2020 15:15:45 +0200
Message-ID: <CAMj1kXHxG_7Om=O5XJCAGrWEUnOBoNLvF20amSc=5JegCdFqew@mail.gmail.com>
Subject: Re: [PATCH 02/33] efi/libstub: Make initrd file loader configurable
To:     linux-efi <linux-efi@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Arvind Sankar <nivedita@alum.mit.edu>,
        Atish Patra <atish.patra@wdc.com>,
        Palmer Dabbelt <palmerdabbelt@google.com>,
        Zou Wei <zou_wei@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 24 Apr 2020 at 15:06, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Loading an initrd passed via the kernel command line is deprecated: it
> is limited to files that reside in the same volume as the one the kernel
> itself was loaded from, and we have more flexible ways to achieve the
> same. So make it configurable so new architectures can decide not to
> enable it.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/Kconfig           | 11 ++++++++
>  drivers/firmware/efi/libstub/efistub.h | 38 ++++++++++++++++++++------
>  drivers/firmware/efi/libstub/file.c    | 32 +++++-----------------
>  3 files changed, 47 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 2a2b2b96a1dc..4e788dd55b03 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -124,6 +124,17 @@ config EFI_ARMSTUB_DTB_LOADER
>           functionality for bootloaders that do not have such support
>           this option is necessary.
>
> +config EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER
> +       bool "Enable the command line initrd loader"
> +       depends on EFI_GENERIC_STUB

Right after hitting send, I realised this should be

depends on EFI_GENERIC_STUB || X86

Care to fix that up before applying, please?


> +       default y
> +       help
> +         Select this config option to add support for the initrd= command
> +         line parameter, allowing an initrd that resides on the same volume
> +         as the kernel image to be loaded into memory.
> +
> +         This method is deprecated.
> +
>  config EFI_BOOTLOADER_CONTROL
>         tristate "EFI Bootloader Control"
>         depends on EFI_VARS
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 67d26949fd26..7517683b31e9 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -651,15 +651,35 @@ efi_status_t efi_parse_options(char const *cmdline);
>  efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
>                            unsigned long size);
>
> -efi_status_t efi_load_dtb(efi_loaded_image_t *image,
> -                         unsigned long *load_addr,
> -                         unsigned long *load_size);
> -
> -efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> -                            unsigned long *load_addr,
> -                            unsigned long *load_size,
> -                            unsigned long soft_limit,
> -                            unsigned long hard_limit);
> +efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> +                                 const efi_char16_t *optstr,
> +                                 int optstr_size,
> +                                 unsigned long soft_limit,
> +                                 unsigned long hard_limit,
> +                                 unsigned long *load_addr,
> +                                 unsigned long *load_size);
> +
> +
> +static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
> +                                       unsigned long *load_addr,
> +                                       unsigned long *load_size)
> +{
> +       return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
> +                                   ULONG_MAX, ULONG_MAX, load_addr, load_size);
> +}
> +
> +static inline efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> +                                          unsigned long *load_addr,
> +                                          unsigned long *load_size,
> +                                          unsigned long soft_limit,
> +                                          unsigned long hard_limit)
> +{
> +       if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER))
> +               return EFI_SUCCESS;
> +
> +       return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
> +                                   soft_limit, hard_limit, load_addr, load_size);
> +}
>
>  efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
>                                       unsigned long *load_size,
> diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
> index ea66b1f16a79..27e014ea4459 100644
> --- a/drivers/firmware/efi/libstub/file.c
> +++ b/drivers/firmware/efi/libstub/file.c
> @@ -121,13 +121,13 @@ static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
>   * We only support loading a file from the same filesystem as
>   * the kernel image.
>   */
> -static efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> -                                        const efi_char16_t *optstr,
> -                                        int optstr_size,
> -                                        unsigned long soft_limit,
> -                                        unsigned long hard_limit,
> -                                        unsigned long *load_addr,
> -                                        unsigned long *load_size)
> +efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> +                                 const efi_char16_t *optstr,
> +                                 int optstr_size,
> +                                 unsigned long soft_limit,
> +                                 unsigned long hard_limit,
> +                                 unsigned long *load_addr,
> +                                 unsigned long *load_size)
>  {
>         const efi_char16_t *cmdline = image->load_options;
>         int cmdline_len = image->load_options_size / 2;
> @@ -239,21 +239,3 @@ static efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
>         efi_free(alloc_size, alloc_addr);
>         return status;
>  }
> -
> -efi_status_t efi_load_dtb(efi_loaded_image_t *image,
> -                         unsigned long *load_addr,
> -                         unsigned long *load_size)
> -{
> -       return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
> -                                   ULONG_MAX, ULONG_MAX, load_addr, load_size);
> -}
> -
> -efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> -                            unsigned long *load_addr,
> -                            unsigned long *load_size,
> -                            unsigned long soft_limit,
> -                            unsigned long hard_limit)
> -{
> -       return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
> -                                   soft_limit, hard_limit, load_addr, load_size);
> -}
> --
> 2.17.1
>