Heinrich, On Wed, Mar 18, 2020 at 10:06:38AM +0100, Heinrich Schuchardt wrote: > On 3/18/20 9:17 AM, AKASHI Takahiro wrote: > > On Wed, Mar 18, 2020 at 09:04:44AM +0100, Heinrich Schuchardt wrote: > > > On 3/17/20 3:12 AM, AKASHI Takahiro wrote: > > > > In this commit, a very simple firmware management protocol driver > > > > is implemented. It will take a single FIT image firmware in a capsule > > > > and apply the data using an existing update_tftp() interface. > > > > > > > > To specify a device and location to be updated, > > > > CONFIG_EFI_CAPSULE_FIT_INTERFACE, and > > > > CONFIG_EFI_CAPSULE_FIT_DEVICE > > > > are used. > > > > > > > > Signed-off-by: AKASHI Takahiro <[email protected]> > > > > --- > > > > include/efi_loader.h | 3 + > > > > lib/efi_loader/Kconfig | 24 ++++- > > > > lib/efi_loader/Makefile | 1 + > > > > lib/efi_loader/efi_firmware.c | 191 > > > > ++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 218 insertions(+), 1 deletion(-) > > > > create mode 100644 lib/efi_loader/efi_firmware.c > > > > > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > > > index c701672e18db..79bdf9586d24 100644 > > > > --- a/include/efi_loader.h > > > > +++ b/include/efi_loader.h > > > > @@ -134,6 +134,7 @@ extern const struct efi_hii_config_access_protocol > > > > efi_hii_config_access; > > > > extern const struct efi_hii_database_protocol efi_hii_database; > > > > extern const struct efi_hii_string_protocol efi_hii_string; > > > > extern const struct efi_rng_protocol efi_rng_protocol; > > > > +extern const struct efi_firmware_management_protocol efi_fmp_fit; > > > > > > > > uint16_t *efi_dp_str(struct efi_device_path *dp); > > > > > > > > @@ -180,6 +181,8 @@ extern const efi_guid_t > > > > efi_guid_hii_database_protocol; > > > > extern const efi_guid_t efi_guid_hii_string_protocol; > > > > /* GUID of capsule update result */ > > > > extern const efi_guid_t efi_guid_capsule_report; > > > > +/* GUID of firmware management protocol */ > > > > +extern const efi_guid_t efi_guid_firmware_management_protocol; > > > > > > > > /* GUID of RNG protocol */ > > > > extern const efi_guid_t efi_guid_rng_protocol; > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > index 43d6f75d557a..41b1e9b5543c 100644 > > > > --- a/lib/efi_loader/Kconfig > > > > +++ b/lib/efi_loader/Kconfig > > > > @@ -97,7 +97,6 @@ config EFI_CAPSULE_UPDATE > > > > Select this option if you want to use capsule update feature, > > > > including firmware updates and variable updates. > > > > > > > > - > > > > if EFI_CAPSULE_UPDATE > > > > > > > > config EFI_CAPSULE_UPDATE_FIRMWARE > > > > @@ -107,6 +106,29 @@ config EFI_CAPSULE_UPDATE_FIRMWARE > > > > Select this option if you want to enable capsule-based > > > > firmware update > > > > > > > > +config EFI_CAPSULE_FIT_SIMPLE > > > > + bool "Firmware management protocol for simple FIT image" > > > > + depends on EFI_CAPSULE_UPDATE_FIRMWARE > > > > + depends on FIT > > > > + select UPDATE_TFTP > > > > > > UPDATE_TFTP is a very unsecure setting. A rogue DHCP and tFTP server can > > > be used to compromise a device where this is enabled. > > > > > > Why should we need to enable an insecure network protocol to have > > > capsule updates? > > > > 1. This is a sample FMP driver to demonstrate a power of capsule > > 2. update_tftp() is called *only* against the interface and device > > that are specified by configuration. It's up to the developer. > > I am concerned about the dhcp or tftp command becoming insecure with > CONFIG_UPDATE_TFTP=y.
I don't know how CONFIG_UPDATE_TFTP can cause insecure situation here. See my comment in [1]. > I would prefer a CONFIG_UPDATE controlling if common/update.c is > compiled and a separate CONFIG_UPDATE_TFTP which enables what is tFTP > specific. It would be possible, but it also means that the meaning of CONFIG_UPDATE_TFTP will be changed. Is it acceptable? [1] https://lists.denx.de/pipermail/u-boot/2020-March/401741.html -Takahiro Akashi > Best regards > > Heinrich > > > 3. Later on, capsule authentication support will be implemented. > > > > So I believe that my approach here makes good sense. > > > > -Takahiro Akashi > > > > > Best regards > > > > > > Heinrich > > > > > > > + select DFU > > > > + default n > > > > + help > > > > + Select this option if you want to enable firmware management > > > > protocol > > > > + for simple FIT image > > > > + > > > > +config EFI_CAPSULE_FIT_INTERFACE > > > > + string "Storage interface for storing FIT image" > > > > + depends on EFI_CAPSULE_FIT_SIMPLE > > > > + help > > > > + Define storage interface for storing FIT image > > > > + > > > > +config EFI_CAPSULE_FIT_DEVICE > > > > + string "Storage device for storing FIT image" > > > > + depends on EFI_CAPSULE_FIT_SIMPLE > > > > + help > > > > + Define storage device for storing FIT image > > > > + > > > > endif > > > > > > > > config EFI_CAPSULE_ON_DISK > > > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > > > > index f19096924bef..50da10e0e3d9 100644 > > > > --- a/lib/efi_loader/Makefile > > > > +++ b/lib/efi_loader/Makefile > > > > @@ -23,6 +23,7 @@ obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o > > > > obj-y += efi_bootmgr.o > > > > obj-y += efi_boottime.o > > > > obj-$(CONFIG_EFI_CAPSULE_UPDATE) += efi_capsule.o > > > > +obj-$(CONFIG_EFI_CAPSULE_FIT_SIMPLE) += efi_firmware.o > > > > obj-y += efi_console.o > > > > obj-y += efi_device_path.o > > > > obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o > > > > diff --git a/lib/efi_loader/efi_firmware.c > > > > b/lib/efi_loader/efi_firmware.c > > > > new file mode 100644 > > > > index 000000000000..021c93196242 > > > > --- /dev/null > > > > +++ b/lib/efi_loader/efi_firmware.c > > > > @@ -0,0 +1,191 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * EFI Firmware management protocol for FIT image > > > > + * > > > > + * Copyright (c) 2018 Linaro Limited > > > > + * Author: AKASHI Takahiro > > > > + */ > > > > + > > > > +#include <common.h> > > > > +#include <efi_loader.h> > > > > +#include <net.h> > > > > + > > > > +/* > > > > + * This FIMRWARE_MANAGEMENT_PROTOCOL driver provides a simple > > > > + * firmware update method, and handles > > > > + * - a single region of firmware via DFU > > > > + * but doesn't support > > > > + * - versioning of firmware image > > > > + * - package information > > > > + */ > > > > +const efi_guid_t efi_firmware_image_type_uboot_fit = > > > > + EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID; > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_get_image_info( > > > > + struct efi_firmware_management_protocol *this, > > > > + efi_uintn_t *image_info_size, > > > > + struct efi_firmware_image_descriptor *image_info, > > > > + u32 *descriptor_version, > > > > + u8 *descriptor_count, > > > > + efi_uintn_t *descriptor_size, > > > > + u32 *package_version, > > > > + u16 **package_version_name) > > > > +{ > > > > + efi_status_t ret = EFI_SUCCESS; > > > > + > > > > + EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this, > > > > + image_info_size, image_info, > > > > + descriptor_version, descriptor_count, descriptor_size, > > > > + package_version, package_version_name); > > > > + > > > > + if (!image_info_size) > > > > + return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > + > > > > + if (*image_info_size < sizeof(*image_info)) { > > > > + *image_info_size = sizeof(*image_info); > > > > + return EFI_EXIT(EFI_BUFFER_TOO_SMALL); > > > > + } > > > > + > > > > + if (!image_info) > > > > + return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > + > > > > + *image_info_size = sizeof(*image_info); > > > > + if (descriptor_version) > > > > + *descriptor_version = > > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION; > > > > + if (descriptor_count) > > > > + *descriptor_count = 1; > > > > + if (descriptor_size) > > > > + *descriptor_size = sizeof(*image_info); > > > > + if (package_version) > > > > + *package_version = 0xffffffff; /* not supported */ > > > > + if (package_version_name) > > > > + *package_version_name = NULL; /* not supported */ > > > > + > > > > + image_info[0].image_index = 1; > > > > + image_info[0].image_type_id = efi_firmware_image_type_uboot_fit; > > > > + image_info[0].image_id = 0; > > > > + image_info[0].image_id_name = L"fw_simple"; > > > > + image_info[0].version = 0; /* not supported */ > > > > + image_info[0].version_name = NULL; /* not supported */ > > > > + image_info[0].size = 0; > > > > +#if defined(CONFIG_FIT) && defined(CONFIG_UPDATE_TFTP) > > > > + image_info[0].attributes_supported = > > > > + EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE; > > > > + image_info[0].attributes_setting = > > > > EFI_IMAGE_ATTRIBUTE_IMAGE_UPDATABLE; > > > > +#else > > > > + image_info[0].attributes_supported = 0; > > > > + image_info[0].attributes_setting = 0; > > > > +#endif > > > > + image_info[0].lowest_supported_image_version = 0; > > > > + image_info[0].last_attempt_version = 0; > > > > + image_info[0].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > > > > + image_info[0].hardware_instance = 1; > > > > + image_info[0].dependencies = NULL; > > > > + > > > > + return EFI_EXIT(ret); > > > > +} > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_get_image( > > > > + struct efi_firmware_management_protocol *this, > > > > + u8 image_index, > > > > + void *image, > > > > + efi_uintn_t *image_size) > > > > +{ > > > > + EFI_ENTRY("%p %d %p %p\n", this, image_index, image, > > > > image_size); > > > > + > > > > + return EFI_EXIT(EFI_UNSUPPORTED); > > > > +} > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_set_image( > > > > + struct efi_firmware_management_protocol *this, > > > > + u8 image_index, > > > > + const void *image, > > > > + efi_uintn_t image_size, > > > > + const void *vendor_code, > > > > + efi_status_t (*progress)(efi_uintn_t completion), > > > > + u16 **abort_reason) > > > > +{ > > > > + EFI_ENTRY("%p %d %p %ld %p %p %p\n", this, image_index, image, > > > > + image_size, vendor_code, progress, abort_reason); > > > > + > > > > +#if defined(CONFIG_FIT) && defined(CONFIG_UPDATE_TFTP) > > > > + if (!image || image_index != 1) > > > > + return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > + > > > > + if (update_tftp((ulong)image, CONFIG_EFI_CAPSULE_FIT_INTERFACE, > > > > + CONFIG_EFI_CAPSULE_FIT_DEVICE)) > > > > + return EFI_EXIT(EFI_DEVICE_ERROR); > > > > + > > > > + return EFI_EXIT(EFI_SUCCESS); > > > > +#else > > > > + return EFI_EXIT(EFI_UNSUPPORTED); > > > > +#endif > > > > +} > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_check_image( > > > > + struct efi_firmware_management_protocol *this, > > > > + u8 image_index, > > > > + const void *image, > > > > + efi_uintn_t *image_size, > > > > + u32 *image_updatable) > > > > +{ > > > > + EFI_ENTRY("%p %d %p %p %p\n", this, image_index, image, > > > > image_size, > > > > + image_updatable); > > > > + > > > > + return EFI_EXIT(EFI_UNSUPPORTED); > > > > +} > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_get_package_info( > > > > + struct efi_firmware_management_protocol *this, > > > > + u32 *package_version, > > > > + u16 **package_version_name, > > > > + u32 *package_version_name_maxlen, > > > > + u64 *attributes_supported, > > > > + u64 *attributes_setting) > > > > +{ > > > > + EFI_ENTRY("%p %p %p %p %p %p\n", this, package_version, > > > > + package_version_name, package_version_name_maxlen, > > > > + attributes_supported, attributes_setting); > > > > + > > > > + return EFI_EXIT(EFI_UNSUPPORTED); > > > > +} > > > > + > > > > +static > > > > +efi_status_t EFIAPI efi_fmp_fit_set_package_info( > > > > + struct efi_firmware_management_protocol *this, > > > > + const void *image, > > > > + efi_uintn_t *image_size, > > > > + const void *vendor_code, > > > > + u32 package_version, > > > > + const u16 *package_version_name) > > > > +{ > > > > + EFI_ENTRY("%p %p %p %p %x %p\n", this, image, image_size, > > > > vendor_code, > > > > + package_version, package_version_name); > > > > + > > > > + return EFI_EXIT(EFI_UNSUPPORTED); > > > > +} > > > > + > > > > +const struct efi_firmware_management_protocol efi_fmp_fit_simple = { > > > > + .get_image_info = efi_fmp_fit_get_image_info, > > > > + .get_image = efi_fmp_fit_get_image, > > > > + .set_image = efi_fmp_fit_set_image, > > > > + .check_image = efi_fmp_fit_check_image, > > > > + .get_package_info = efi_fmp_fit_get_package_info, > > > > + .set_package_info = efi_fmp_fit_set_package_info, > > > > +}; > > > > + > > > > +efi_status_t arch_efi_load_capsule_drivers(void) > > > > +{ > > > > + efi_status_t ret; > > > > + > > > > + ret = EFI_CALL(efi_install_multiple_protocol_interfaces( > > > > + &efi_root, > > > > &efi_guid_firmware_management_protocol, > > > > + &efi_fmp_fit_simple, NULL)); > > > > + > > > > + return ret; > > > > +} > > > > > > > >
