[PATCH v2 0/4] EFI: Miscellaneous capsule update fixes
The following set of patches fix separate issues relating to the capsule update code.
The first patch moves the setting of the descriptor_count parameter to the GetImageInfo function after assertion of a non zero image descriptor buffer.
The second patch enables capsule update to proceed even when the OsIndications variable is not set with EFI_IGNORE_OSINDICATIONS config enabled.
The third patch updates the capsule update documentation to highlight the 64 bit value of OsIndications that needs to be set.
The fourth patch defines a common function for the GetImageInfo functionality for both FIT and raw image FMP instances.
Sughosh Ganu (4): EFI: Populate descriptor_count value only when image_info_size is not zero EFI: Do not consider OsIndications variable if CONFIG_EFI_IGNORE_OSINDICATIONS is enabled EFI: Update the documentation to reflect the correct value of OsIndications EFI: FMP: Use a common GetImageInfo function for FIT and raw images
doc/develop/uefi/uefi.rst | 2 +- lib/efi_loader/efi_capsule.c | 7 +-- lib/efi_loader/efi_firmware.c | 85 +++++++---------------------------- 3 files changed, 21 insertions(+), 73 deletions(-)
The GetImageInfo function of the Firmware Mangement Protocol(FMP) gets called initially to query the size of the image descriptor array that would have to be allocated. During this call, the rest of the function arguments, specifically pointers might be passed as NULL. Do not populate the descriptor_count value before it is known that the call to GetImageInfo has been made with the allocated buffer for the image descriptors.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---
Changes since V1: None
lib/efi_loader/efi_firmware.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index fe4e084106..9cdefab41f 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -130,9 +130,6 @@ static efi_status_t efi_fill_image_desc_array( struct efi_fw_image *fw_array; int i;
- fw_array = update_info.images; - *descriptor_count = num_image_type_guids; - total_size = sizeof(*image_info) * num_image_type_guids;
if (*image_info_size < total_size) { @@ -142,6 +139,8 @@ static efi_status_t efi_fill_image_desc_array( } *image_info_size = total_size;
+ fw_array = update_info.images; + *descriptor_count = num_image_type_guids; *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION; *descriptor_size = sizeof(*image_info); *package_version = 0xffffffff; /* not supported */
Hi Sughosh,
On Wed, 1 Jun 2022 at 19:01, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The GetImageInfo function of the Firmware Mangement Protocol(FMP) gets called initially to query the size of the image descriptor array that would have to be allocated. During this call, the rest of the function arguments, specifically pointers might be passed as NULL. Do not populate the descriptor_count value before it is known that the call to GetImageInfo has been made with the allocated buffer for the image descriptors.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
This patch solves the hang issue I observed on master with CapsuleApp.efi when doing
FS5:EFI/BOOT/app/CapsuleApp.efi -P
Which is part of the SystemReady IR ACS compliance suite. Tested on a RockPi4b board.
Tested-by: Peter Griffin peter.griffin@linaro.org
Peter
Changes since V1: None
lib/efi_loader/efi_firmware.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index fe4e084106..9cdefab41f 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -130,9 +130,6 @@ static efi_status_t efi_fill_image_desc_array( struct efi_fw_image *fw_array; int i;
fw_array = update_info.images;*descriptor_count = num_image_type_guids;total_size = sizeof(*image_info) * num_image_type_guids; if (*image_info_size < total_size) {@@ -142,6 +139,8 @@ static efi_status_t efi_fill_image_desc_array( } *image_info_size = total_size;
fw_array = update_info.images;*descriptor_count = num_image_type_guids; *descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION; *descriptor_size = sizeof(*image_info); *package_version = 0xffffffff; /* not supported */-- 2.25.1
The EFI_IGNORE_OSINDICATIONS config symbol was introduced as a mechanism to have capsule updates work even on platforms where the SetVariable runtime service was not supported. The current logic requires the OsIndications variable to have been set to a 64 bit value even when the EFI_IGNORE_OSINDICATIONS config is enabled. Return an error code on not being able to read the variable only when EFI_IGNORE_OSINDICATIONS is not enabled.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org ---
Changes since V1: * Read the OsIndications variable, if present and clear the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED flag even if the CONFIG_EFI_IGNORE_OSINDICATIONS config is enabled.
lib/efi_loader/efi_capsule.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index c76a5f3570..a6b98f066a 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1058,14 +1058,15 @@ static void efi_capsule_scan_done(void) */ static efi_status_t check_run_capsules(void) { - u64 os_indications; + u64 os_indications = 0x0; efi_uintn_t size; efi_status_t r;
size = sizeof(os_indications); r = efi_get_variable_int(u"OsIndications", &efi_global_variable_guid, NULL, &size, &os_indications, NULL); - if (r != EFI_SUCCESS || size != sizeof(os_indications)) + if (!IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS) && + (r != EFI_SUCCESS || size != sizeof(os_indications))) return EFI_NOT_FOUND;
if (os_indications & @@ -1084,7 +1085,7 @@ static efi_status_t check_run_capsules(void) return EFI_SUCCESS; } else if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) { return EFI_SUCCESS; - } else { + } else { return EFI_NOT_FOUND; } }
The OsIndications is a 64 bit variable, and the current code expects the value of the variable to be 64 bit. Update the documentation to reflect this fact.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---
Changes since V1: None
doc/develop/uefi/uefi.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index 753a4e5e29..941e427093 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -326,7 +326,7 @@ bit in OsIndications variable with
.. code-block:: console
- => setenv -e -nv -bs -rt -v OsIndications =0x04 + => setenv -e -nv -bs -rt -v OsIndications =0x0000000000000004
Since U-boot doesn't currently support SetVariable at runtime, its value won't be taken over across the reboot. If this is the case, you can skip
The GetImageInfo function definitions for the FIT images and raw images are the same. Use a common function for the both the Firmware Management Protocol(FMP) instances for raw and FIT images.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org ---
Changes since V1: None
lib/efi_loader/efi_firmware.c | 80 ++++++----------------------------- 1 file changed, 14 insertions(+), 66 deletions(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index 9cdefab41f..0ffee3fb1f 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -177,18 +177,8 @@ static efi_status_t efi_fill_image_desc_array( return EFI_SUCCESS; }
-#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_FIT -/* - * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update - * method with existing FIT image format, and handles - * - multiple regions of firmware via DFU - * but doesn't support - * - versioning of firmware image - * - package information - */ - /** - * efi_firmware_fit_get_image_info - return information about the current + * efi_firmware_get_image_info - return information about the current * firmware image * @this: Protocol instance * @image_info_size: Size of @image_info @@ -206,7 +196,7 @@ static efi_status_t efi_fill_image_desc_array( * Return status code */ static -efi_status_t EFIAPI efi_firmware_fit_get_image_info( +efi_status_t EFIAPI efi_firmware_get_image_info( struct efi_firmware_management_protocol *this, efi_uintn_t *image_info_size, struct efi_firmware_image_descriptor *image_info, @@ -239,6 +229,16 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info( return EFI_EXIT(ret); }
+#ifdef CONFIG_EFI_CAPSULE_FIRMWARE_FIT +/* + * This FIRMWARE_MANAGEMENT_PROTOCOL driver provides a firmware update + * method with existing FIT image format, and handles + * - multiple regions of firmware via DFU + * but doesn't support + * - versioning of firmware image + * - package information + */ + /** * efi_firmware_fit_set_image - update the firmware image * @this: Protocol instance @@ -278,7 +278,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( }
const struct efi_firmware_management_protocol efi_fmp_fit = { - .get_image_info = efi_firmware_fit_get_image_info, + .get_image_info = efi_firmware_get_image_info, .get_image = efi_firmware_get_image_unsupported, .set_image = efi_firmware_fit_set_image, .check_image = efi_firmware_check_image_unsupported, @@ -293,58 +293,6 @@ const struct efi_firmware_management_protocol efi_fmp_fit = { * method with raw data. */
-/** - * efi_firmware_raw_get_image_info - return information about the current - * firmware image - * @this: Protocol instance - * @image_info_size: Size of @image_info - * @image_info: Image information - * @descriptor_version: Pointer to version number - * @descriptor_count: Pointer to number of descriptors - * @descriptor_size: Pointer to descriptor size - * @package_version: Package version - * @package_version_name: Package version's name - * - * Return information bout the current firmware image in @image_info. - * @image_info will consist of a number of descriptors. - * Each descriptor will be created based on "dfu_alt_info" variable. - * - * Return status code - */ -static -efi_status_t EFIAPI efi_firmware_raw_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 && - (!image_info || !descriptor_version || !descriptor_count || - !descriptor_size || !package_version || !package_version_name)) - return EFI_EXIT(EFI_INVALID_PARAMETER); - - ret = efi_fill_image_desc_array(image_info_size, image_info, - descriptor_version, descriptor_count, - descriptor_size, package_version, - package_version_name); - - return EFI_EXIT(ret); -} - /** * efi_firmware_raw_set_image - update the firmware image * @this: Protocol instance @@ -430,7 +378,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( }
const struct efi_firmware_management_protocol efi_fmp_raw = { - .get_image_info = efi_firmware_raw_get_image_info, + .get_image_info = efi_firmware_get_image_info, .get_image = efi_firmware_get_image_unsupported, .set_image = efi_firmware_raw_set_image, .check_image = efi_firmware_check_image_unsupported,
participants (2)
-
Peter Griffin -
Sughosh Ganu