diff mbox series

[1/2] efi_loader: Implement FileLoad2 for initramfs loading

Message ID 20200221075546.864563-1-ilias.apalodimas@linaro.org
State New
Headers show
Series [1/2] efi_loader: Implement FileLoad2 for initramfs loading | expand

Commit Message

Ilias Apalodimas Feb. 21, 2020, 7:55 a.m. UTC
Following kernel's proposal for an arch-agnostic initrd loading
mechanism [1] let's implement the U-boot counterpart.
This new approach has a number of advantages compared to what we did up
to now. The file is loaded into memory only when requested limiting the
area of TOCTOU attacks. Users will be allowed to place the initramfs
file on any u-boot accessible partition instead of just the ESP one.
Finally this is an attempt of a generic interface across architectures
in the linux kernel so it makes sense to support that.

The file location is intentionally only supported as a config option
argument(CONFIG_EFI_INITRD_FILESPEC), in an effort to enhance security.
Although U-boot is not responsible for verifying the integrity of the
initramfs, we can enhance the offered security by only accepting a
built-in option, which will be naturally verified by UEFI Secure Boot.
This can easily change in the future if needed and configure that via ENV
or UEFI variable.

[1] https://lore.kernel.org/linux-efi/20200207202637.GA3464906 at rani.riverdale.lan/T/#m4a25eb33112fab7a22faa0fd65d4d663209af32f

Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 cmd/efidebug.c                   |   4 +
 include/efi_api.h                |  17 +++
 include/efi_load_initrd.h        |  25 ++++
 include/efi_loader.h             |   1 +
 lib/efi_loader/Kconfig           |  15 +++
 lib/efi_loader/Makefile          |   1 +
 lib/efi_loader/efi_load_initrd.c | 198 +++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c       |   5 +
 8 files changed, 266 insertions(+)
 create mode 100644 include/efi_load_initrd.h
 create mode 100644 lib/efi_loader/efi_load_initrd.c

Comments

AKASHI Takahiro March 12, 2020, 5:28 a.m. UTC | #1
Ilias,

I haven't followed this thread.

On Fri, Feb 21, 2020 at 09:55:45AM +0200, Ilias Apalodimas wrote:
> Following kernel's proposal for an arch-agnostic initrd loading
> mechanism [1] let's implement the U-boot counterpart.
> This new approach has a number of advantages compared to what we did up
> to now. The file is loaded into memory only when requested limiting the
> area of TOCTOU attacks. Users will be allowed to place the initramfs
> file on any u-boot accessible partition instead of just the ESP one.
> Finally this is an attempt of a generic interface across architectures
> in the linux kernel so it makes sense to support that.
> 
> The file location is intentionally only supported as a config option
> argument(CONFIG_EFI_INITRD_FILESPEC), in an effort to enhance security.
> Although U-boot is not responsible for verifying the integrity of the
> initramfs, we can enhance the offered security by only accepting a
> built-in option, which will be naturally verified by UEFI Secure Boot.
> This can easily change in the future if needed and configure that via ENV
> or UEFI variable.

What if we have several bootable images with different initrd
required at the same time? For example, we may install ubuntu 16.04,
18.04 etc to the same device (and then boot one of them via grub).

Since the guid is unique, U-Boot doesn't have any measure to identify which
initrd be exported via FILE2_PROTOCOL.
In this sense, I don't think using a config option is a good idea in general.

> [1] https://lore.kernel.org/linux-efi/20200207202637.GA3464906 at rani.riverdale.lan/T/#m4a25eb33112fab7a22faa0fd65d4d663209af32f
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  cmd/efidebug.c                   |   4 +
>  include/efi_api.h                |  17 +++
>  include/efi_load_initrd.h        |  25 ++++
>  include/efi_loader.h             |   1 +
>  lib/efi_loader/Kconfig           |  15 +++
>  lib/efi_loader/Makefile          |   1 +
>  lib/efi_loader/efi_load_initrd.c | 198 +++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_setup.c       |   5 +
>  8 files changed, 266 insertions(+)
>  create mode 100644 include/efi_load_initrd.h
>  create mode 100644 lib/efi_loader/efi_load_initrd.c
> 
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 576e95b395dc..f450518c9e83 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -264,6 +264,10 @@ static const struct {
>  		"SMBIOS table",
>  		SMBIOS_TABLE_GUID,
>  	},
> +	{
> +		"Load File2",
> +		EFI_LOAD_FILE2_PROTOCOL_GUID,
> +	},
>  };
>  
>  /**
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 22396172e15f..8ffed1334b84 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -319,6 +319,14 @@ struct efi_runtime_services {
>  	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>  		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>  
> +#define EFI_LOAD_FILE_PROTOCOL_GUID \
> +	EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
> +		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)

Do you need this definition?

> +#define EFI_LOAD_FILE2_PROTOCOL_GUID \
> +	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
> +		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
> +
>  struct efi_configuration_table {
>  	efi_guid_t guid;
>  	void *table;
> @@ -474,6 +482,7 @@ struct efi_device_path_nvme {
>  #define DEVICE_PATH_TYPE_MEDIA_DEVICE		0x04
>  #  define DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH	0x01
>  #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH	0x02
> +#  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH	0x03
>  #  define DEVICE_PATH_SUB_TYPE_FILE_PATH	0x04
>  
>  struct efi_device_path_hard_drive_path {
> @@ -1607,6 +1616,14 @@ struct efi_unicode_collation_protocol {
>  	char *supported_languages;
>  };
>  
> +struct efi_load_file_protocol {
> +	efi_status_t (EFIAPI *load_file)(struct efi_load_file_protocol *this,
> +					 struct efi_device_path *file_path,
> +					 bool boot_policy,
> +					 efi_uintn_t *buffer_size,
> +					 void *buffer);
> +};
> +
>  /* Boot manager load options */
>  #define LOAD_OPTION_ACTIVE		0x00000001
>  #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
> diff --git a/include/efi_load_initrd.h b/include/efi_load_initrd.h
> new file mode 100644
> index 000000000000..478ae807c685
> --- /dev/null
> +++ b/include/efi_load_initrd.h

It might be unlikely, but the file name should be more generic
for potential enhancement.

> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_LOAD_INITRD_H_
> +#define _EFI_LOAD_INITRD_H_
> +
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +/*
> + * Vendor GUID used by Linux to identify the handle with the
> + * EFI_LOAD_FILE2_PROTOCOL and load an initial ramdisk.
> + */
> +#define EFI_INITRD_MEDIA_GUID \
> +	EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, \
> +		 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> +
> +struct efi_initrd_dp {
> +	struct efi_device_path_vendor vendor;
> +	struct efi_device_path end;
> +} __packed;
> +
> +#endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d4c59b54c48b..8e3437983391 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -378,6 +378,7 @@ efi_status_t efi_gop_register(void);
>  efi_status_t efi_net_register(void);
>  /* Called by bootefi to make the watchdog available */
>  efi_status_t efi_watchdog_register(void);
> +efi_status_t efi_initrd_register(void);
>  /* Called by bootefi to make SMBIOS tables available */
>  /**
>   * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index a7afa3f29e88..50d4f81e6207 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -130,4 +130,19 @@ config EFI_RNG_PROTOCOL
>  	  "Support for EFI_RNG_PROTOCOL implementation. Uses the rng
>  	   device on the platform"
>  
> +config EFI_LOAD_FILE2_INITRD
> +	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
> +	default n
> +	help
> +	  Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
> +	  use to load the initial ramdisk. Once this is enabled using
> +	  initrd=<ramdisk> will stop working.
> +
> +config EFI_INITRD_FILESPEC
> +	string "initramfs path"
> +	default "host 0:1 initrd"

"host" interface makes sense only on sandbox, which will not be expected
to be able to boot any linux kernel.
So this default value is inappropriate anyway.

> +	depends on EFI_LOAD_FILE2_INITRD
> +	help
> +	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> +
>  endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 04dc8648512b..9b3b70447336 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -43,3 +43,4 @@ obj-$(CONFIG_NET) += efi_net.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> +obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> new file mode 100644
> index 000000000000..574a83d7e308
> --- /dev/null
> +++ b/lib/efi_loader/efi_load_initrd.c

I'd prefer efi_file_initrd.c for similarity to efi_file.c, or
more specifically efi_linux.c.

> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <env.h>
> +#include <malloc.h>
> +#include <mapmem.h>
> +#include <dm.h>

Do you need env.h and dm.h?

> +#include <fs.h>
> +#include <efi_loader.h>
> +#include <efi_load_initrd.h>
> +
> +static const efi_guid_t efi_guid_load_file2_protocol =
> +		EFI_LOAD_FILE2_PROTOCOL_GUID;
> +
> +static efi_status_t EFIAPI
> +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> +		      struct efi_device_path *file_path, bool boot_policy,
> +		      efi_uintn_t *buffer_size, void *buffer);
> +
> +static const struct efi_load_file_protocol efi_lf2_protocol = {
> +	.load_file = efi_load_file2_initrd,
> +};
> +
> +/*
> + * Device path defined by Linux to identify the handle providing the
> + * EFI_LOAD_FILE2_PROTOCOL used for loading the initial ramdisk.
> + */
> +static const struct efi_initrd_dp dp = {
> +	.vendor = {
> +		{
> +		   DEVICE_PATH_TYPE_MEDIA_DEVICE,
> +		   DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> +		   sizeof(dp.vendor),
> +		},
> +		EFI_INITRD_MEDIA_GUID,
> +	},
> +	.end = {
> +		DEVICE_PATH_TYPE_END,
> +		DEVICE_PATH_SUB_TYPE_END,
> +		sizeof(dp.end),
> +	}
> +};
> +
> +/**
> + * get_file_size() - retrieve the size of initramfs, set efi status on error
> + *
> + * @dev:			device to read from. i.e "mmc"
> + * @part:			device partition. i.e "0:1"
> + * @file:			name fo file
> + * @status:			EFI exit code in case of failure
> + *
> + * Return:			size of file
> + */
> +static loff_t get_file_size(const char *dev, const char *part, const char *file,
> +			    efi_status_t *status)
> +{
> +	loff_t sz = 0;
> +	int ret;
> +
> +	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> +	if (ret) {
> +		*status = EFI_NO_MEDIA;
> +		goto out;
> +	}
> +
> +	ret = fs_size(file, &sz);
> +	if (ret) {
> +		sz = 0;
> +		*status = EFI_NOT_FOUND;
> +		goto out;
> +	}
> +
> +out:
> +	return sz;
> +}
> +
> +/**
> + * load_file2() - get information about random number generation

Please correct the message.

> + *
> + * This function implement the LoadFile2() service in order to load an initram
> + * disk requested by the Linux kernel stub.
> + * See the UEFI spec for details.
> + *
> + * @this:			loadfile2 protocol instance
> + * @file_path:			relative path of the file. "" in this case
> + * @boot_policy:		must be false for Loadfile2
> + * @buffer_size:		size of allocated buffer
> + * @buffer:			buffer to load the file
> + *
> + * Return:			status code
> + */
> +static efi_status_t EFIAPI
> +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> +		      struct efi_device_path *file_path, bool boot_policy,
> +		      efi_uintn_t *buffer_size, void *buffer)
> +{
> +	const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
> +	efi_status_t status = EFI_NOT_FOUND;
> +	loff_t file_sz = 0, read_sz = 0;
> +	char *dev, *part, *file;
> +	char *s;
> +	int ret;
> +
> +	EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> +		  buffer_size, buffer);
> +
> +	s = strdup(filespec);
> +	if (!s)
> +		goto out;

I think that we should return a better error code rather than NOT_FOUND.

> +
> +	if (!this || this != &efi_lf2_protocol ||

Please use guidcmp() here.

> +	    !buffer_size) {
> +		status = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	if (file_path->type != dp.end.type ||
> +	    file_path->sub_type != dp.end.sub_type) {
> +		status = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	if (boot_policy) {
> +		status = EFI_UNSUPPORTED;
> +		goto out;
> +	}
> +
> +	/* expect something like 'mmc 0:1 initrd.cpio.gz' */
> +	dev = strsep(&s, " ");
> +	if (!dev)
> +		goto out;
> +	part = strsep(&s, " ");
> +	if (!part)
> +		goto out;
> +	file = strsep(&s, " ");
> +	if (!file)
> +		goto out;
> +
> +	file_sz = get_file_size(dev, part, file, &status);
> +	if (!file_sz)
> +		goto out;

ditto.

> +
> +	if (!buffer || *buffer_size < file_sz) {
> +		status = EFI_BUFFER_TOO_SMALL;
> +		*buffer_size = file_sz;
> +	} else {
> +		ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> +		if (ret) {
> +			status = EFI_NO_MEDIA;
> +			goto out;
> +		}
> +
> +		ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
> +			      &read_sz);
> +		if (ret || read_sz != file_sz)
> +			goto out;
> +		*buffer_size = read_sz;
> +
> +		status = EFI_SUCCESS;
> +	}
> +
> +out:
> +	free(s);
> +	return EFI_EXIT(status);
> +}
> +
> +/**
> + * efi_initrd_register() - Register a handle and loadfile2 protocol
> + *
> + * This function creates a new handle and installs a linux specific GUID
> + * to handle initram disk loading during boot.
> + * See the UEFI spec for details.
> + *
> + * Return:			status code
> + */
> +efi_status_t efi_initrd_register(void)
> +{
> +	efi_handle_t efi_initrd_handle = NULL;
> +	efi_status_t ret;
> +
> +	/*
> +	 * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
> +	 * use to load the initial ramdisk.
> +	 */
> +	ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> +		       (&efi_initrd_handle,
> +			/* initramfs */
> +			&efi_guid_device_path, &dp,
> +			/* LOAD_FILE2 */
> +			&efi_guid_load_file2_protocol,
> +			(void *)&efi_lf2_protocol,
> +			NULL));
> +
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index de7b616c6daa..f03685b11559 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -155,6 +155,11 @@ efi_status_t efi_init_obj_list(void)
>  	if (ret != EFI_SUCCESS)
>  		goto out;
>  #endif
> +#ifdef CONFIG_EFI_LOAD_FILE2_INITRD
> +	ret = efi_initrd_register();
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +#endif
>  #ifdef CONFIG_NET
>  	ret = efi_net_register();
>  	if (ret != EFI_SUCCESS)
> -- 
> 2.25.1
>
Ilias Apalodimas March 12, 2020, 6:29 a.m. UTC | #2
Akashi-san,

On Thu, Mar 12, 2020 at 02:28:45PM +0900, AKASHI Takahiro wrote:
> Ilias,
> 
> I haven't followed this thread.
> 
> On Fri, Feb 21, 2020 at 09:55:45AM +0200, Ilias Apalodimas wrote:
> > Following kernel's proposal for an arch-agnostic initrd loading
> > mechanism [1] let's implement the U-boot counterpart.
> > This new approach has a number of advantages compared to what we did up
> > to now. The file is loaded into memory only when requested limiting the
> > area of TOCTOU attacks. Users will be allowed to place the initramfs
> > file on any u-boot accessible partition instead of just the ESP one.
> > Finally this is an attempt of a generic interface across architectures
> > in the linux kernel so it makes sense to support that.
> > 
> > The file location is intentionally only supported as a config option
> > argument(CONFIG_EFI_INITRD_FILESPEC), in an effort to enhance security.
> > Although U-boot is not responsible for verifying the integrity of the
> > initramfs, we can enhance the offered security by only accepting a
> > built-in option, which will be naturally verified by UEFI Secure Boot.
> > This can easily change in the future if needed and configure that via ENV
> > or UEFI variable.
> 
> What if we have several bootable images with different initrd
> required at the same time? For example, we may install ubuntu 16.04,
> 18.04 etc to the same device (and then boot one of them via grub).
> 
> Since the guid is unique, U-Boot doesn't have any measure to identify which
> initrd be exported via FILE2_PROTOCOL.
> In this sense, I don't think using a config option is a good idea in general.

Can't you concatenate multiple cpio archives for this? I haven't checked this
personally but i remember distros doing it to add firmware files they need for
example. It might not be ideal because you may need different user-space
program versions or different versions of the same script(??) but the linux 
module loading part, of different kernel revisions, should be there.

I don't have any strong preference on this anyway hence my commit
message, it's just another naive protection, since the user can always replace
the initramfs using the same name and break out of this. I never needed multiple
initrd files so I preferred the config option. If people need a config option
it's easily extensible.

> > +#define EFI_LOAD_FILE_PROTOCOL_GUID \
[...]
> > +	EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
> > +		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> 
> Do you need this definition?
> 

Not really it just felt weird adding LOAD_FILE2 without the LOAD_FILE
definition.

> > +#define EFI_LOAD_FILE2_PROTOCOL_GUID \
> > +	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
> > +		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
> > +
> > --- /dev/null
[...]
> > +++ b/include/efi_load_initrd.h
> 
> It might be unlikely, but the file name should be more generic
> for potential enhancement.
> 

This is defining a very specific GUID for initramfs that Linux consumes to load
the file, I'd rather keep it as is.

> > @@ -0,0 +1,25 @@
[...]
> > +
> > +config EFI_INITRD_FILESPEC
> > +	string "initramfs path"
> > +	default "host 0:1 initrd"
> 
> "host" interface makes sense only on sandbox, which will not be expected
> to be able to boot any linux kernel.
> So this default value is inappropriate anyway.
> 

Not really. The user *must* have an idea on what he is doing and this is the
default config we need to run on the sandbox. So it serves both as an example
and a defult config for the sandbox testing.

> > +	depends on EFI_LOAD_FILE2_INITRD
> > +	help
> > +	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> > +
> >  endif
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 04dc8648512b..9b3b70447336 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -43,3 +43,4 @@ obj-$(CONFIG_NET) += efi_net.o
> >  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> > +obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> > diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> > new file mode 100644
> > index 000000000000..574a83d7e308
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_load_initrd.c
> 
> I'd prefer efi_file_initrd.c for similarity to efi_file.c, or
> more specifically efi_linux.c.
> 

I can send a patch renaming those but i think it's pointless.
The filename describes exactly what's the intent imho.

> > @@ -0,0 +1,198 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2020, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <env.h>
> > +#include <malloc.h>
> > +#include <mapmem.h>
> > +#include <dm.h>
> 
> Do you need env.h and dm.h?

No, I'll remove those.

> 
> > +#include <fs.h>
> > +#include <efi_loader.h>
> > +#include <efi_load_initrd.h>
> > +
> > +static const efi_guid_t efi_guid_load_file2_protocol =
> > +		EFI_LOAD_FILE2_PROTOCOL_GUID;
> > +
> > +static efi_status_t EFIAPI
> > +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> > +		      struct efi_device_path *file_path, bool boot_policy,
> > +		      efi_uintn_t *buffer_size, void *buffer);
> > +
> > +static const struct efi_load_file_protocol efi_lf2_protocol = {
> > +	.load_file = efi_load_file2_initrd,
> > +};
> > +
> > +/*
> > + * Device path defined by Linux to identify the handle providing the
> > + * EFI_LOAD_FILE2_PROTOCOL used for loading the initial ramdisk.
> > + */
> > +static const struct efi_initrd_dp dp = {
> > +	.vendor = {
> > +		{
> > +		   DEVICE_PATH_TYPE_MEDIA_DEVICE,
> > +		   DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> > +		   sizeof(dp.vendor),
> > +		},
> > +		EFI_INITRD_MEDIA_GUID,
> > +	},
> > +	.end = {
> > +		DEVICE_PATH_TYPE_END,
> > +		DEVICE_PATH_SUB_TYPE_END,
> > +		sizeof(dp.end),
> > +	}
> > +};
> > +
> > +/**
> > + * get_file_size() - retrieve the size of initramfs, set efi status on error
> > + *
> > + * @dev:			device to read from. i.e "mmc"
> > + * @part:			device partition. i.e "0:1"
> > + * @file:			name fo file
> > + * @status:			EFI exit code in case of failure
> > + *
> > + * Return:			size of file
> > + */
> > +static loff_t get_file_size(const char *dev, const char *part, const char *file,
> > +			    efi_status_t *status)
> > +{
> > +	loff_t sz = 0;
> > +	int ret;
> > +
> > +	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> > +	if (ret) {
> > +		*status = EFI_NO_MEDIA;
> > +		goto out;
> > +	}
> > +
> > +	ret = fs_size(file, &sz);
> > +	if (ret) {
> > +		sz = 0;
> > +		*status = EFI_NOT_FOUND;
> > +		goto out;
> > +	}
> > +
> > +out:
> > +	return sz;
> > +}
> > +
> > +/**
> > + * load_file2() - get information about random number generation
> 
> Please correct the message.
> 

Ooops

> > + *
> > + * This function implement the LoadFile2() service in order to load an initram
> > + * disk requested by the Linux kernel stub.
> > + * See the UEFI spec for details.
> > + *
> > + * @this:			loadfile2 protocol instance
> > + * @file_path:			relative path of the file. "" in this case
> > + * @boot_policy:		must be false for Loadfile2
> > + * @buffer_size:		size of allocated buffer
> > + * @buffer:			buffer to load the file
> > + *
> > + * Return:			status code
> > + */
> > +static efi_status_t EFIAPI
> > +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> > +		      struct efi_device_path *file_path, bool boot_policy,
> > +		      efi_uintn_t *buffer_size, void *buffer)
> > +{
> > +	const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
> > +	efi_status_t status = EFI_NOT_FOUND;
> > +	loff_t file_sz = 0, read_sz = 0;
> > +	char *dev, *part, *file;
> > +	char *s;
> > +	int ret;
> > +
> > +	EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> > +		  buffer_size, buffer);
> > +
> > +	s = strdup(filespec);
> > +	if (!s)
> > +		goto out;
> 
> I think that we should return a better error code rather than NOT_FOUND.
> 

Like? The spec has a specific set of returns codes you can use there, 
EFI_UNSUPPORTED
EFI_INVALID_PARAMETER
EFI_NO_MEDIA
EFI_DEVICE_ERROR
EFI_NO_RESPONSE
EFI_NOT_FOUND
EFI_ABORTED
EFI_BUFFER_TOO_SMALL

Is there anything you like better?

> > +
> > +	if (!this || this != &efi_lf2_protocol ||
> 
> Please use guidcmp() here.
> 

Sure

> > +	    !buffer_size) {
> > +	if (!file_sz)
[...]
> > +		goto out;
> 
> ditto.
> 

Sure

> > +
> > +	if (!buffer || *buffer_size < file_sz) {
[...]
> >  	if (ret != EFI_SUCCESS)
> > -- 
> > 2.25.1
> > 


Regards
/Ilias
AKASHI Takahiro March 12, 2020, 6:58 a.m. UTC | #3
On Thu, Mar 12, 2020 at 08:29:38AM +0200, Ilias Apalodimas wrote:
> Akashi-san,
> 
> On Thu, Mar 12, 2020 at 02:28:45PM +0900, AKASHI Takahiro wrote:
> > Ilias,
> > 
> > I haven't followed this thread.
> > 
> > On Fri, Feb 21, 2020 at 09:55:45AM +0200, Ilias Apalodimas wrote:
> > > Following kernel's proposal for an arch-agnostic initrd loading
> > > mechanism [1] let's implement the U-boot counterpart.
> > > This new approach has a number of advantages compared to what we did up
> > > to now. The file is loaded into memory only when requested limiting the
> > > area of TOCTOU attacks. Users will be allowed to place the initramfs
> > > file on any u-boot accessible partition instead of just the ESP one.
> > > Finally this is an attempt of a generic interface across architectures
> > > in the linux kernel so it makes sense to support that.
> > > 
> > > The file location is intentionally only supported as a config option
> > > argument(CONFIG_EFI_INITRD_FILESPEC), in an effort to enhance security.
> > > Although U-boot is not responsible for verifying the integrity of the
> > > initramfs, we can enhance the offered security by only accepting a
> > > built-in option, which will be naturally verified by UEFI Secure Boot.
> > > This can easily change in the future if needed and configure that via ENV
> > > or UEFI variable.
> > 
> > What if we have several bootable images with different initrd
> > required at the same time? For example, we may install ubuntu 16.04,
> > 18.04 etc to the same device (and then boot one of them via grub).
> > 
> > Since the guid is unique, U-Boot doesn't have any measure to identify which
> > initrd be exported via FILE2_PROTOCOL.
> > In this sense, I don't think using a config option is a good idea in general.
> 
> Can't you concatenate multiple cpio archives for this? I haven't checked this
> personally but i remember distros doing it to add firmware files they need for
> example. It might not be ideal because you may need different user-space
> program versions or different versions of the same script(??)

I wonder think why it is not a problem.

> but the linux 
> module loading part, of different kernel revisions, should be there.

Can we assume that this is only the use of initrd?

> I don't have any strong preference on this anyway hence my commit
> message, it's just another naive protection, since the user can always replace
> the initramfs using the same name and break out of this. I never needed multiple
> initrd files so I preferred the config option. If people need a config option
> it's easily extensible.

How? I don't think it's trivial.

> > > +#define EFI_LOAD_FILE_PROTOCOL_GUID \
> [...]
> > > +	EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
> > > +		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> > 
> > Do you need this definition?
> > 
> 
> Not really it just felt weird adding LOAD_FILE2 without the LOAD_FILE
> definition.
> 
> > > +#define EFI_LOAD_FILE2_PROTOCOL_GUID \
> > > +	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
> > > +		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
> > > +
> > > --- /dev/null
> [...]
> > > +++ b/include/efi_load_initrd.h
> > 
> > It might be unlikely, but the file name should be more generic
> > for potential enhancement.
> > 
> 
> This is defining a very specific GUID for initramfs that Linux consumes to load
> the file, I'd rather keep it as is.

I didn't intent the guid, but a file name.

> > > @@ -0,0 +1,25 @@
> [...]
> > > +
> > > +config EFI_INITRD_FILESPEC
> > > +	string "initramfs path"
> > > +	default "host 0:1 initrd"
> > 
> > "host" interface makes sense only on sandbox, which will not be expected
> > to be able to boot any linux kernel.
> > So this default value is inappropriate anyway.
> > 
> 
> Not really. The user *must* have an idea on what he is doing and this is the
> default config we need to run on the sandbox. So it serves both as an example
> and a defult config for the sandbox testing.

I don't think we can boot a linux kernel from U-Boot on sandbox.

> > > +	depends on EFI_LOAD_FILE2_INITRD
> > > +	help
> > > +	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> > > +
> > >  endif
> > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > index 04dc8648512b..9b3b70447336 100644
> > > --- a/lib/efi_loader/Makefile
> > > +++ b/lib/efi_loader/Makefile
> > > @@ -43,3 +43,4 @@ obj-$(CONFIG_NET) += efi_net.o
> > >  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> > >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> > >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> > > +obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> > > diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> > > new file mode 100644
> > > index 000000000000..574a83d7e308
> > > --- /dev/null
> > > +++ b/lib/efi_loader/efi_load_initrd.c
> > 
> > I'd prefer efi_file_initrd.c for similarity to efi_file.c, or
> > more specifically efi_linux.c.
> > 
> 
> I can send a patch renaming those but i think it's pointless.
> The filename describes exactly what's the intent imho.

What if we will want to add another FILE2_PROTOCOL or linux-specifc
hack?
Now I believe that OS (or platform)-dependent implementation
should go into a separate directory, like lib/efi_linux/...
for clarification.

> > > @@ -0,0 +1,198 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2020, Linaro Limited
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <env.h>
> > > +#include <malloc.h>
> > > +#include <mapmem.h>
> > > +#include <dm.h>
> > 
> > Do you need env.h and dm.h?
> 
> No, I'll remove those.
> 
> > 
> > > +#include <fs.h>
> > > +#include <efi_loader.h>
> > > +#include <efi_load_initrd.h>
> > > +
> > > +static const efi_guid_t efi_guid_load_file2_protocol =
> > > +		EFI_LOAD_FILE2_PROTOCOL_GUID;
> > > +
> > > +static efi_status_t EFIAPI
> > > +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> > > +		      struct efi_device_path *file_path, bool boot_policy,
> > > +		      efi_uintn_t *buffer_size, void *buffer);
> > > +
> > > +static const struct efi_load_file_protocol efi_lf2_protocol = {
> > > +	.load_file = efi_load_file2_initrd,
> > > +};
> > > +
> > > +/*
> > > + * Device path defined by Linux to identify the handle providing the
> > > + * EFI_LOAD_FILE2_PROTOCOL used for loading the initial ramdisk.
> > > + */
> > > +static const struct efi_initrd_dp dp = {
> > > +	.vendor = {
> > > +		{
> > > +		   DEVICE_PATH_TYPE_MEDIA_DEVICE,
> > > +		   DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> > > +		   sizeof(dp.vendor),
> > > +		},
> > > +		EFI_INITRD_MEDIA_GUID,
> > > +	},
> > > +	.end = {
> > > +		DEVICE_PATH_TYPE_END,
> > > +		DEVICE_PATH_SUB_TYPE_END,
> > > +		sizeof(dp.end),
> > > +	}
> > > +};
> > > +
> > > +/**
> > > + * get_file_size() - retrieve the size of initramfs, set efi status on error
> > > + *
> > > + * @dev:			device to read from. i.e "mmc"
> > > + * @part:			device partition. i.e "0:1"
> > > + * @file:			name fo file
> > > + * @status:			EFI exit code in case of failure
> > > + *
> > > + * Return:			size of file
> > > + */
> > > +static loff_t get_file_size(const char *dev, const char *part, const char *file,
> > > +			    efi_status_t *status)
> > > +{
> > > +	loff_t sz = 0;
> > > +	int ret;
> > > +
> > > +	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> > > +	if (ret) {
> > > +		*status = EFI_NO_MEDIA;
> > > +		goto out;
> > > +	}
> > > +
> > > +	ret = fs_size(file, &sz);
> > > +	if (ret) {
> > > +		sz = 0;
> > > +		*status = EFI_NOT_FOUND;
> > > +		goto out;
> > > +	}
> > > +
> > > +out:
> > > +	return sz;
> > > +}
> > > +
> > > +/**
> > > + * load_file2() - get information about random number generation
> > 
> > Please correct the message.
> > 
> 
> Ooops
> 
> > > + *
> > > + * This function implement the LoadFile2() service in order to load an initram
> > > + * disk requested by the Linux kernel stub.
> > > + * See the UEFI spec for details.
> > > + *
> > > + * @this:			loadfile2 protocol instance
> > > + * @file_path:			relative path of the file. "" in this case
> > > + * @boot_policy:		must be false for Loadfile2
> > > + * @buffer_size:		size of allocated buffer
> > > + * @buffer:			buffer to load the file
> > > + *
> > > + * Return:			status code
> > > + */
> > > +static efi_status_t EFIAPI
> > > +efi_load_file2_initrd(struct efi_load_file_protocol *this,
> > > +		      struct efi_device_path *file_path, bool boot_policy,
> > > +		      efi_uintn_t *buffer_size, void *buffer)
> > > +{
> > > +	const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
> > > +	efi_status_t status = EFI_NOT_FOUND;
> > > +	loff_t file_sz = 0, read_sz = 0;
> > > +	char *dev, *part, *file;
> > > +	char *s;
> > > +	int ret;
> > > +
> > > +	EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> > > +		  buffer_size, buffer);
> > > +
> > > +	s = strdup(filespec);
> > > +	if (!s)
> > > +		goto out;
> > 
> > I think that we should return a better error code rather than NOT_FOUND.
> > 
> 
> Like? The spec has a specific set of returns codes you can use there, 
> EFI_UNSUPPORTED
> EFI_INVALID_PARAMETER
> EFI_NO_MEDIA
> EFI_DEVICE_ERROR
> EFI_NO_RESPONSE
> EFI_NOT_FOUND
> EFI_ABORTED
> EFI_BUFFER_TOO_SMALL
> 
> Is there anything you like better?

In most of all cases, OUT_OF_RESOURCES.

Thanks,
-Takahiro Akashi

> > > +
> > > +	if (!this || this != &efi_lf2_protocol ||
> > 
> > Please use guidcmp() here.
> > 
> 
> Sure
> 
> > > +	    !buffer_size) {
> > > +	if (!file_sz)
> [...]
> > > +		goto out;
> > 
> > ditto.
> > 
> 
> Sure
> 
> > > +
> > > +	if (!buffer || *buffer_size < file_sz) {
> [...]
> > >  	if (ret != EFI_SUCCESS)
> > > -- 
> > > 2.25.1
> > > 
> 
> 
> Regards
> /Ilias
Ilias Apalodimas March 12, 2020, 7:17 a.m. UTC | #4
On Thu, Mar 12, 2020 at 03:58:03PM +0900, AKASHI Takahiro wrote:
> On Thu, Mar 12, 2020 at 08:29:38AM +0200, Ilias Apalodimas wrote:
> > Akashi-san,
> > 
> > On Thu, Mar 12, 2020 at 02:28:45PM +0900, AKASHI Takahiro wrote:
> > > Ilias,
> > > 
> > > I haven't followed this thread.
> > > 
> > > On Fri, Feb 21, 2020 at 09:55:45AM +0200, Ilias Apalodimas wrote:
> > > > Following kernel's proposal for an arch-agnostic initrd loading
> > > > mechanism [1] let's implement the U-boot counterpart.
> > > > This new approach has a number of advantages compared to what we did up
> > > > to now. The file is loaded into memory only when requested limiting the
> > > > area of TOCTOU attacks. Users will be allowed to place the initramfs
> > > > file on any u-boot accessible partition instead of just the ESP one.
> > > > Finally this is an attempt of a generic interface across architectures
> > > > in the linux kernel so it makes sense to support that.
> > > > 
> > > > The file location is intentionally only supported as a config option
> > > > argument(CONFIG_EFI_INITRD_FILESPEC), in an effort to enhance security.
> > > > Although U-boot is not responsible for verifying the integrity of the
> > > > initramfs, we can enhance the offered security by only accepting a
> > > > built-in option, which will be naturally verified by UEFI Secure Boot.
> > > > This can easily change in the future if needed and configure that via ENV
> > > > or UEFI variable.
> > > 
> > > What if we have several bootable images with different initrd
> > > required at the same time? For example, we may install ubuntu 16.04,
> > > 18.04 etc to the same device (and then boot one of them via grub).
> > > 
> > > Since the guid is unique, U-Boot doesn't have any measure to identify which
> > > initrd be exported via FILE2_PROTOCOL.
> > > In this sense, I don't think using a config option is a good idea in general.
> > 
> > Can't you concatenate multiple cpio archives for this? I haven't checked this
> > personally but i remember distros doing it to add firmware files they need for
> > example. It might not be ideal because you may need different user-space
> > program versions or different versions of the same script(??)
> 
> I wonder think why it is not a problem.
> 

I never said it wasn't, on the contrary, i pointed out cases were
concatenating cpio's might not be enough.

> > but the linux 
> > module loading part, of different kernel revisions, should be there.
> 
> Can we assume that this is only the use of initrd?
> 

Of course not.

> > I don't have any strong preference on this anyway hence my commit
> > message, it's just another naive protection, since the user can always replace
> > the initramfs using the same name and break out of this. I never needed multiple
> > initrd files so I preferred the config option. If people need a config option
> > it's easily extensible.
> 
> How? I don't think it's trivial.

Parse an env variable and override the config option?
Something like:
setenv efi_initrd 'mmc 0:1 foo.cpio.gz' -> env_get("efi_initrd")
wouldn't work?

> 
> > > > +#define EFI_LOAD_FILE_PROTOCOL_GUID \
> > [...]
> > > > +	EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
> > > > +		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> > > 
> > > Do you need this definition?
> > > 
> > 
> > Not really it just felt weird adding LOAD_FILE2 without the LOAD_FILE
> > definition.
> > 
> > > > +#define EFI_LOAD_FILE2_PROTOCOL_GUID \
> > > > +	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
> > > > +		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
> > > > +
> > > > --- /dev/null
> > [...]
> > > > +++ b/include/efi_load_initrd.h
> > > 
> > > It might be unlikely, but the file name should be more generic
> > > for potential enhancement.
> > > 
> > 
> > This is defining a very specific GUID for initramfs that Linux consumes to load
> > the file, I'd rather keep it as is.
> 
> I didn't intent the guid, but a file name.

Yes my response is for the file name. It serves a sole purpose, define the 
explicit GUID linux expects.

> > > > @@ -0,0 +1,25 @@
> > [...]
> > > > +
> > > > +config EFI_INITRD_FILESPEC
> > > > +	string "initramfs path"
> > > > +	default "host 0:1 initrd"
> > > 
> > > "host" interface makes sense only on sandbox, which will not be expected
> > > to be able to boot any linux kernel.
> > > So this default value is inappropriate anyway.
> > > 
> > 
> > Not really. The user *must* have an idea on what he is doing and this is the
> > default config we need to run on the sandbox. So it serves both as an example
> > and a defult config for the sandbox testing.
> 
> I don't think we can boot a linux kernel from U-Boot on sandbox.
> 

We run the sefltest though, which is replicating the Linux efi stub 
functionality.

> > > > +	depends on EFI_LOAD_FILE2_INITRD
> > > > +	help
> > > > +	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> > > > +
> > > >  endif
> > > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > > > index 04dc8648512b..9b3b70447336 100644
> > > > --- a/lib/efi_loader/Makefile
> > > > +++ b/lib/efi_loader/Makefile
> > > > @@ -43,3 +43,4 @@ obj-$(CONFIG_NET) += efi_net.o
> > > >  obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> > > >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> > > >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> > > > +obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> > > > diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> > > > new file mode 100644
> > > > index 000000000000..574a83d7e308
> > > > --- /dev/null
> > > > +++ b/lib/efi_loader/efi_load_initrd.c
> > > 
> > > I'd prefer efi_file_initrd.c for similarity to efi_file.c, or
> > > more specifically efi_linux.c.
> > > 
> > 
> > I can send a patch renaming those but i think it's pointless.
> > The filename describes exactly what's the intent imho.
> 
> What if we will want to add another FILE2_PROTOCOL or linux-specifc
> hack?
> Now I believe that OS (or platform)-dependent implementation
> should go into a separate directory, like lib/efi_linux/...
> for clarification.
> 

I am not against that, i just think the tons of comments are already enough. 
But i see the point in the long run, assuming we'll add enough OS-specific
options to justify this.

> > > > @@ -0,0 +1,198 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*

[...]

> > > I think that we should return a better error code rather than NOT_FOUND.
> > > 
> > 
> > Like? The spec has a specific set of returns codes you can use there, 
> > EFI_UNSUPPORTED
> > EFI_INVALID_PARAMETER
> > EFI_NO_MEDIA
> > EFI_DEVICE_ERROR
> > EFI_NO_RESPONSE
> > EFI_NOT_FOUND
> > EFI_ABORTED
> > EFI_BUFFER_TOO_SMALL
> > 
> > Is there anything you like better?
> 
> In most of all cases, OUT_OF_RESOURCES.

If i am not reading the spec wrong you *can't* return that here and even if you
could, the majority of error cases fail if the file was not found.
Maybe apart from this:
ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
if (ret) {
	status = EFI_NO_MEDIA;
   	goto out;
}


Thanks
/Ilias
diff mbox series

Patch

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 576e95b395dc..f450518c9e83 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -264,6 +264,10 @@  static const struct {
 		"SMBIOS table",
 		SMBIOS_TABLE_GUID,
 	},
+	{
+		"Load File2",
+		EFI_LOAD_FILE2_PROTOCOL_GUID,
+	},
 };
 
 /**
diff --git a/include/efi_api.h b/include/efi_api.h
index 22396172e15f..8ffed1334b84 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -319,6 +319,14 @@  struct efi_runtime_services {
 	EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
 		 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
+#define EFI_LOAD_FILE_PROTOCOL_GUID \
+	EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
+		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
+
+#define EFI_LOAD_FILE2_PROTOCOL_GUID \
+	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
+		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
+
 struct efi_configuration_table {
 	efi_guid_t guid;
 	void *table;
@@ -474,6 +482,7 @@  struct efi_device_path_nvme {
 #define DEVICE_PATH_TYPE_MEDIA_DEVICE		0x04
 #  define DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH	0x01
 #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH	0x02
+#  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH	0x03
 #  define DEVICE_PATH_SUB_TYPE_FILE_PATH	0x04
 
 struct efi_device_path_hard_drive_path {
@@ -1607,6 +1616,14 @@  struct efi_unicode_collation_protocol {
 	char *supported_languages;
 };
 
+struct efi_load_file_protocol {
+	efi_status_t (EFIAPI *load_file)(struct efi_load_file_protocol *this,
+					 struct efi_device_path *file_path,
+					 bool boot_policy,
+					 efi_uintn_t *buffer_size,
+					 void *buffer);
+};
+
 /* Boot manager load options */
 #define LOAD_OPTION_ACTIVE		0x00000001
 #define LOAD_OPTION_FORCE_RECONNECT	0x00000002
diff --git a/include/efi_load_initrd.h b/include/efi_load_initrd.h
new file mode 100644
index 000000000000..478ae807c685
--- /dev/null
+++ b/include/efi_load_initrd.h
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_LOAD_INITRD_H_
+#define _EFI_LOAD_INITRD_H_
+
+#include <efi.h>
+#include <efi_api.h>
+
+/*
+ * Vendor GUID used by Linux to identify the handle with the
+ * EFI_LOAD_FILE2_PROTOCOL and load an initial ramdisk.
+ */
+#define EFI_INITRD_MEDIA_GUID \
+	EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, \
+		 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
+
+struct efi_initrd_dp {
+	struct efi_device_path_vendor vendor;
+	struct efi_device_path end;
+} __packed;
+
+#endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d4c59b54c48b..8e3437983391 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -378,6 +378,7 @@  efi_status_t efi_gop_register(void);
 efi_status_t efi_net_register(void);
 /* Called by bootefi to make the watchdog available */
 efi_status_t efi_watchdog_register(void);
+efi_status_t efi_initrd_register(void);
 /* Called by bootefi to make SMBIOS tables available */
 /**
  * efi_acpi_register() - write out ACPI tables
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index a7afa3f29e88..50d4f81e6207 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -130,4 +130,19 @@  config EFI_RNG_PROTOCOL
 	  "Support for EFI_RNG_PROTOCOL implementation. Uses the rng
 	   device on the platform"
 
+config EFI_LOAD_FILE2_INITRD
+	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
+	default n
+	help
+	  Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
+	  use to load the initial ramdisk. Once this is enabled using
+	  initrd=<ramdisk> will stop working.
+
+config EFI_INITRD_FILESPEC
+	string "initramfs path"
+	default "host 0:1 initrd"
+	depends on EFI_LOAD_FILE2_INITRD
+	help
+	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
+
 endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 04dc8648512b..9b3b70447336 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -43,3 +43,4 @@  obj-$(CONFIG_NET) += efi_net.o
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
 obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
+obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
new file mode 100644
index 000000000000..574a83d7e308
--- /dev/null
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -0,0 +1,198 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#include <common.h>
+#include <env.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <dm.h>
+#include <fs.h>
+#include <efi_loader.h>
+#include <efi_load_initrd.h>
+
+static const efi_guid_t efi_guid_load_file2_protocol =
+		EFI_LOAD_FILE2_PROTOCOL_GUID;
+
+static efi_status_t EFIAPI
+efi_load_file2_initrd(struct efi_load_file_protocol *this,
+		      struct efi_device_path *file_path, bool boot_policy,
+		      efi_uintn_t *buffer_size, void *buffer);
+
+static const struct efi_load_file_protocol efi_lf2_protocol = {
+	.load_file = efi_load_file2_initrd,
+};
+
+/*
+ * Device path defined by Linux to identify the handle providing the
+ * EFI_LOAD_FILE2_PROTOCOL used for loading the initial ramdisk.
+ */
+static const struct efi_initrd_dp dp = {
+	.vendor = {
+		{
+		   DEVICE_PATH_TYPE_MEDIA_DEVICE,
+		   DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
+		   sizeof(dp.vendor),
+		},
+		EFI_INITRD_MEDIA_GUID,
+	},
+	.end = {
+		DEVICE_PATH_TYPE_END,
+		DEVICE_PATH_SUB_TYPE_END,
+		sizeof(dp.end),
+	}
+};
+
+/**
+ * get_file_size() - retrieve the size of initramfs, set efi status on error
+ *
+ * @dev:			device to read from. i.e "mmc"
+ * @part:			device partition. i.e "0:1"
+ * @file:			name fo file
+ * @status:			EFI exit code in case of failure
+ *
+ * Return:			size of file
+ */
+static loff_t get_file_size(const char *dev, const char *part, const char *file,
+			    efi_status_t *status)
+{
+	loff_t sz = 0;
+	int ret;
+
+	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
+	if (ret) {
+		*status = EFI_NO_MEDIA;
+		goto out;
+	}
+
+	ret = fs_size(file, &sz);
+	if (ret) {
+		sz = 0;
+		*status = EFI_NOT_FOUND;
+		goto out;
+	}
+
+out:
+	return sz;
+}
+
+/**
+ * load_file2() - get information about random number generation
+ *
+ * This function implement the LoadFile2() service in order to load an initram
+ * disk requested by the Linux kernel stub.
+ * See the UEFI spec for details.
+ *
+ * @this:			loadfile2 protocol instance
+ * @file_path:			relative path of the file. "" in this case
+ * @boot_policy:		must be false for Loadfile2
+ * @buffer_size:		size of allocated buffer
+ * @buffer:			buffer to load the file
+ *
+ * Return:			status code
+ */
+static efi_status_t EFIAPI
+efi_load_file2_initrd(struct efi_load_file_protocol *this,
+		      struct efi_device_path *file_path, bool boot_policy,
+		      efi_uintn_t *buffer_size, void *buffer)
+{
+	const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
+	efi_status_t status = EFI_NOT_FOUND;
+	loff_t file_sz = 0, read_sz = 0;
+	char *dev, *part, *file;
+	char *s;
+	int ret;
+
+	EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
+		  buffer_size, buffer);
+
+	s = strdup(filespec);
+	if (!s)
+		goto out;
+
+	if (!this || this != &efi_lf2_protocol ||
+	    !buffer_size) {
+		status = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (file_path->type != dp.end.type ||
+	    file_path->sub_type != dp.end.sub_type) {
+		status = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (boot_policy) {
+		status = EFI_UNSUPPORTED;
+		goto out;
+	}
+
+	/* expect something like 'mmc 0:1 initrd.cpio.gz' */
+	dev = strsep(&s, " ");
+	if (!dev)
+		goto out;
+	part = strsep(&s, " ");
+	if (!part)
+		goto out;
+	file = strsep(&s, " ");
+	if (!file)
+		goto out;
+
+	file_sz = get_file_size(dev, part, file, &status);
+	if (!file_sz)
+		goto out;
+
+	if (!buffer || *buffer_size < file_sz) {
+		status = EFI_BUFFER_TOO_SMALL;
+		*buffer_size = file_sz;
+	} else {
+		ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
+		if (ret) {
+			status = EFI_NO_MEDIA;
+			goto out;
+		}
+
+		ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
+			      &read_sz);
+		if (ret || read_sz != file_sz)
+			goto out;
+		*buffer_size = read_sz;
+
+		status = EFI_SUCCESS;
+	}
+
+out:
+	free(s);
+	return EFI_EXIT(status);
+}
+
+/**
+ * efi_initrd_register() - Register a handle and loadfile2 protocol
+ *
+ * This function creates a new handle and installs a linux specific GUID
+ * to handle initram disk loading during boot.
+ * See the UEFI spec for details.
+ *
+ * Return:			status code
+ */
+efi_status_t efi_initrd_register(void)
+{
+	efi_handle_t efi_initrd_handle = NULL;
+	efi_status_t ret;
+
+	/*
+	 * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
+	 * use to load the initial ramdisk.
+	 */
+	ret = EFI_CALL(efi_install_multiple_protocol_interfaces
+		       (&efi_initrd_handle,
+			/* initramfs */
+			&efi_guid_device_path, &dp,
+			/* LOAD_FILE2 */
+			&efi_guid_load_file2_protocol,
+			(void *)&efi_lf2_protocol,
+			NULL));
+
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index de7b616c6daa..f03685b11559 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -155,6 +155,11 @@  efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 #endif
+#ifdef CONFIG_EFI_LOAD_FILE2_INITRD
+	ret = efi_initrd_register();
+	if (ret != EFI_SUCCESS)
+		goto out;
+#endif
 #ifdef CONFIG_NET
 	ret = efi_net_register();
 	if (ret != EFI_SUCCESS)