On 3/27/19 5:40 AM, AKASHI Takahiro wrote:
> Add CONFIG_CMD_STANDALONE_EFIBOOTMGR.
> With this patch, EFI boot manager can be kicked in by a standalone
> command, efibootmgr.

I miss your comment form 0/11 here:

 * When we will support "secure boot" in the future, EFI Boot Manager
   is expected to be invoked as a standalone command without any
   arguments to mitigate security surfaces.

Simply requiring that environment variable $fdtaddr, which is under user
control, is used for loading the device tree does not offer any security
at all.

For secure boot I would expect that the device tree has to be part of a
signed binary and that that signature is checked.

If the user passes in a device tree that should be okay if it has the
required signature.

>
> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> ---
>  cmd/Kconfig   |  8 ++++++++
>  cmd/bootefi.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 4bcc5c45579d..aa27b21956b3 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -224,6 +224,14 @@ config CMD_BOOTEFI
>       help
>         Boot an EFI image from memory.
>
> +config CMD_STANDALONE_EFIBOOTMGR
> +     bool "Enable EFI Boot Manager as a standalone command"
> +     depends on CMD_BOOTEFI
> +     default n
> +     help
> +          With this option enabled, EFI Boot Manager can be invoked
> +       as a standalone command.
> +
>  config CMD_BOOTEFI_HELLO_COMPILE
>       bool "Compile a standard EFI hello world binary for testing"
>       depends on CMD_BOOTEFI && !CPU_V7M && !SANDBOX
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 17dccd718008..2a2ff4034831 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -635,3 +635,38 @@ void efi_set_bootdev(const char *dev, const char *devnr, 
> const char *path)
>               bootefi_image_path = NULL;
>       }
>  }
> +
> +#ifdef CONFIG_CMD_STANDALONE_EFIBOOTMGR
> +static int do_efibootmgr_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
> +                          char * const argv[])
> +{
> +     char *fdt_opt;
> +     int ret;
> +
> +     if (argc != 1)
> +             return CMD_RET_USAGE;

It is unclear why you do not allow the user to pass the location of the
device tree as a parameter.

> +
> +     fdt_opt = env_get("fdtaddr");

You are looking at some environment variable $fdtaddr while most boards
use $fdt_addr_r as preferred address to load the device tree.

Best regards

Heinrich

> +     if (!fdt_opt)> +                fdt_opt = env_get("fdtcontroladdr");
> +
> +     ret = do_efibootmgr(fdt_opt);
> +
> +     free(fdt_opt);
> +
> +     return ret;
> +}
> +
> +#ifdef CONFIG_SYS_LONGHELP
> +static char efibootmgr_help_text[] =
> +     "(no arguments)\n"
> +     " - Launch EFI boot manager and execute EFI application\n"
> +     "   based on BootNext and BootOrder variables\n";
> +#endif
> +
> +U_BOOT_CMD(
> +     efibootmgr, 1, 0, do_efibootmgr_cmd,
> +     "Launch EFI boot manager",
> +     efibootmgr_help_text
> +);
> +#endif /* CONFIG_CMD_STANDALONE_EFIBOOTMGR */
>

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to