All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] efi/libstub: Allow EFI_NOT_FOUND on LOAD_FILE2_PROTOCOL calls for initrd
@ 2020-12-14 17:01 Ilias Apalodimas
  2020-12-14 17:39 ` Heinrich Schuchardt
  0 siblings, 1 reply; 3+ messages in thread
From: Ilias Apalodimas @ 2020-12-14 17:01 UTC (permalink / raw)
  To: ard.biesheuvel
  Cc: Ilias Apalodimas, Ard Biesheuvel, Arvind Sankar, Ingo Molnar,
	Heinrich Schuchardt, linux-efi, linux-kernel

At the moment the EFI stub tries to load an initrd from the
cmdline provided option only if the LoadFile2 protocol does not exist
on the initrd device path.

This might prove problematic for EFI installers that need their own
version of initrd to start the installation process and the firmware
installs the protocol but doesn't have a file to back it up (yet).
Although some firmware implementations return EFI_NOT_FOUND, we
currently return EFI_LOAD_ERROR in efi_load_initrd_dev_path() which
stops the cmdline provided initrd to load.

So let's change the behavior slightly here and explicitly respect the
firmware in case it returns EFI_NOT_FOUND. This way we can load the
cmdline provided initrd.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index aa8da0a49829..391aae2f0cde 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -560,6 +560,7 @@ static const struct {
  * * %EFI_SUCCESS if the initrd was loaded successfully, in which
  *   case @load_addr and @load_size are assigned accordingly
  * * %EFI_NOT_FOUND if no LoadFile2 protocol exists on the initrd device path
+ *   or if the firmware provides LoadFile2 but can't find a file to load
  * * %EFI_INVALID_PARAMETER if load_addr == NULL or load_size == NULL
  * * %EFI_OUT_OF_RESOURCES if memory allocation failed
  * * %EFI_LOAD_ERROR in all other cases
@@ -599,7 +600,14 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
 				(void *)initrd_addr);
 	if (status != EFI_SUCCESS) {
 		efi_free(initrd_size, initrd_addr);
-		return EFI_LOAD_ERROR;
+		/*
+		 * Some firmware implementations might install the EFI
+		 * protocol without checking the file is present and return
+		 * EFI_NOT_FOUND when trying to load the file.
+		 * If that's the case, allow the cmdline defined initrd to
+		 * load.
+		 */
+		return status == EFI_NOT_FOUND ? status : EFI_LOAD_ERROR;
 	}
 
 	*load_addr = initrd_addr;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] efi/libstub: Allow EFI_NOT_FOUND on LOAD_FILE2_PROTOCOL calls for initrd
  2020-12-14 17:01 [PATCH] efi/libstub: Allow EFI_NOT_FOUND on LOAD_FILE2_PROTOCOL calls for initrd Ilias Apalodimas
@ 2020-12-14 17:39 ` Heinrich Schuchardt
  2020-12-14 17:48   ` Ilias Apalodimas
  0 siblings, 1 reply; 3+ messages in thread
From: Heinrich Schuchardt @ 2020-12-14 17:39 UTC (permalink / raw)
  To: Ilias Apalodimas, ard.biesheuvel
  Cc: Ard Biesheuvel, Arvind Sankar, Ingo Molnar, linux-efi, linux-kernel

On 14.12.20 18:01, Ilias Apalodimas wrote:
> At the moment the EFI stub tries to load an initrd from the
> cmdline provided option only if the LoadFile2 protocol does not exist
> on the initrd device path.
>
> This might prove problematic for EFI installers that need their own
> version of initrd to start the installation process and the firmware

Did you hit a real world case?

> installs the protocol but doesn't have a file to back it up (yet).
> Although some firmware implementations return EFI_NOT_FOUND, we
> currently return EFI_LOAD_ERROR in efi_load_initrd_dev_path() which
> stops the cmdline provided initrd to load.
>
> So let's change the behavior slightly here and explicitly respect the
> firmware in case it returns EFI_NOT_FOUND. This way we can load the
> cmdline provided initrd.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  drivers/firmware/efi/libstub/efi-stub-helper.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index aa8da0a49829..391aae2f0cde 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -560,6 +560,7 @@ static const struct {
>   * * %EFI_SUCCESS if the initrd was loaded successfully, in which
>   *   case @load_addr and @load_size are assigned accordingly
>   * * %EFI_NOT_FOUND if no LoadFile2 protocol exists on the initrd device path
> + *   or if the firmware provides LoadFile2 but can't find a file to load
>   * * %EFI_INVALID_PARAMETER if load_addr == NULL or load_size == NULL
>   * * %EFI_OUT_OF_RESOURCES if memory allocation failed
>   * * %EFI_LOAD_ERROR in all other cases
> @@ -599,7 +600,14 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
>  				(void *)initrd_addr);
>  	if (status != EFI_SUCCESS) {
>  		efi_free(initrd_size, initrd_addr);
> -		return EFI_LOAD_ERROR;
> +		/*
> +		 * Some firmware implementations might install the EFI

In U-Boot the filename is set a compile time. As the path may relate to
a removable medium, it would not make sense to check the existence of
the file when installing the protocol.

> +		 * protocol without checking the file is present and return
> +		 * EFI_NOT_FOUND when trying to load the file.
> +		 * If that's the case, allow the cmdline defined initrd to
> +		 * load.

U-Boot's implementation could also return EFI_NO_MEDIA if
CONFIG_EFI_INITRD_FILESPEC relates to a non-existent partition.

Why should we fall back to the command line in this case?

The whole idea of this protocol is to disallow the specification of an
initrd via the command line.

Best regards

Heinrich

> +		 */
> +		return status == EFI_NOT_FOUND ? status : EFI_LOAD_ERROR;
>  	}
>
>  	*load_addr = initrd_addr;
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] efi/libstub: Allow EFI_NOT_FOUND on LOAD_FILE2_PROTOCOL calls for initrd
  2020-12-14 17:39 ` Heinrich Schuchardt
@ 2020-12-14 17:48   ` Ilias Apalodimas
  0 siblings, 0 replies; 3+ messages in thread
From: Ilias Apalodimas @ 2020-12-14 17:48 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: ard.biesheuvel, Ard Biesheuvel, Arvind Sankar, Ingo Molnar,
	linux-efi, linux-kernel

On Mon, Dec 14, 2020 at 06:39:21PM +0100, Heinrich Schuchardt wrote:
> On 14.12.20 18:01, Ilias Apalodimas wrote:
> > At the moment the EFI stub tries to load an initrd from the
> > cmdline provided option only if the LoadFile2 protocol does not exist
> > on the initrd device path.
> >
> > This might prove problematic for EFI installers that need their own
> > version of initrd to start the installation process and the firmware
> 
> Did you hit a real world case?
> 

Yes while trying to install debian with U-boot, in which I enabled LoadFile2
protocol

[...]
> > @@ -599,7 +600,14 @@ efi_status_t efi_load_initrd_dev_path(unsigned long *load_addr,
> >  				(void *)initrd_addr);
> >  	if (status != EFI_SUCCESS) {
> >  		efi_free(initrd_size, initrd_addr);
> > -		return EFI_LOAD_ERROR;
> > +		/*
> > +		 * Some firmware implementations might install the EFI
> 
> In U-Boot the filename is set a compile time. As the path may relate to
> a removable medium, it would not make sense to check the existence of
> the file when installing the protocol.
> 

Agree. That's why I am trying to change the behavior of the stub slightly
and respect the firmware's return value on this.
More over the whole idea is to load the file exactly when requested, rather
than store it in memory and wait until someone requests it.

> > +		 * protocol without checking the file is present and return
> > +		 * EFI_NOT_FOUND when trying to load the file.
> > +		 * If that's the case, allow the cmdline defined initrd to
> > +		 * load.
> 
> U-Boot's implementation could also return EFI_NO_MEDIA if
> CONFIG_EFI_INITRD_FILESPEC relates to a non-existent partition.
> 
> Why should we fall back to the command line in this case?
> 
> The whole idea of this protocol is to disallow the specification of an
> initrd via the command line.

We are not falling back in that case. We only allow a fallback if 
EFI_NOT_FOUND is explicitly returned. 

That being said my check is wrong. I need to check it on the first invocation
of load file, not the last one. I'll send a V2

Regards
/Ilias

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-14 17:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 17:01 [PATCH] efi/libstub: Allow EFI_NOT_FOUND on LOAD_FILE2_PROTOCOL calls for initrd Ilias Apalodimas
2020-12-14 17:39 ` Heinrich Schuchardt
2020-12-14 17:48   ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.