From 546152624f46555b71d6478b9ecb0db67ae43ba6 Mon Sep 17 00:00:00 2001 From: Jonathan Humphreys Date: Wed, 26 Feb 2025 16:35:46 -0600 Subject: efi_firmware: set EFI capsule dfu_alt_info env explicitly The current implementation of EFI capsule update uses set_dfu_alt_info() to set the dfu_alt_info environment variable with the settings it requires. However, set_dfu_alt_info() is doing this for all DFU operations, even those unrelated to capsule update. Thus other uses of DFU, such as DFU boot which sets its own value for the dfu_alt_info environment variable, will have that setting overwritten with the capsule update setting. Similarly, any user defined value for the dfu_alt_info environment variable would get overwritten when any DFU operation was performed, including simply performing a "dfu 0 list" command. The solution is stop using the set_dfu_alt_info() mechanism to set the dfu_alt_info environment variable and instead explicitly set it to the capsule update's setting just before performing the capsule update's DFU operation, and then restore the environment variable back to its original value. This patch implements the explicit setting and restoring of the dfu_alt_info environment variable as part of the EFI capsule update operation. The fix is fully implemented in a subsequent patch that removes the capsule update dfu_alt_info support in set_dfu_alt_info(). Signed-off-by: Jonathan Humphreys Reviewed-by: Mattijs Korpershoek Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_firmware.c | 51 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) (limited to 'lib/efi_loader/efi_firmware.c') diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index 5a754c9cd03..0ffaf5c8f72 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -649,8 +649,10 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( efi_status_t (*progress)(efi_uintn_t completion), u16 **abort_reason) { + int ret; efi_status_t status; struct fmp_state state = { 0 }; + char *orig_dfu_env; EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image, image_size, vendor_code, progress, abort_reason); @@ -663,7 +665,28 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( if (status != EFI_SUCCESS) return EFI_EXIT(status); - if (fit_update(image)) + orig_dfu_env = env_get("dfu_alt_info"); + if (orig_dfu_env) { + orig_dfu_env = strdup(orig_dfu_env); + if (!orig_dfu_env) { + log_err("strdup() failed!\n"); + return EFI_EXIT(EFI_OUT_OF_RESOURCES); + } + } + if (env_set("dfu_alt_info", update_info.dfu_string)) { + log_err("Unable to set env variable \"dfu_alt_info\"!\n"); + free(orig_dfu_env); + return EFI_EXIT(EFI_DEVICE_ERROR); + } + + ret = fit_update(image); + + if (env_set("dfu_alt_info", orig_dfu_env)) + log_warning("Unable to restore env variable \"dfu_alt_info\". Further DFU operations may fail!\n"); + + free(orig_dfu_env); + + if (ret) return EFI_EXIT(EFI_DEVICE_ERROR); efi_firmware_set_fmp_state_var(&state, image_index); @@ -717,6 +740,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( u8 dfu_alt_num; efi_status_t status; struct fmp_state state = { 0 }; + char *orig_dfu_env; EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image, image_size, vendor_code, progress, abort_reason); @@ -747,8 +771,29 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( } } - if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size, - NULL, NULL)) + orig_dfu_env = env_get("dfu_alt_info"); + if (orig_dfu_env) { + orig_dfu_env = strdup(orig_dfu_env); + if (!orig_dfu_env) { + log_err("strdup() failed!\n"); + return EFI_EXIT(EFI_OUT_OF_RESOURCES); + } + } + if (env_set("dfu_alt_info", update_info.dfu_string)) { + log_err("Unable to set env variable \"dfu_alt_info\"!\n"); + free(orig_dfu_env); + return EFI_EXIT(EFI_DEVICE_ERROR); + } + + ret = dfu_write_by_alt(dfu_alt_num, (void *)image, image_size, + NULL, NULL); + + if (env_set("dfu_alt_info", orig_dfu_env)) + log_warning("Unable to restore env variable \"dfu_alt_info\". Further DFU operations may fail!\n"); + + free(orig_dfu_env); + + if (ret) return EFI_EXIT(EFI_DEVICE_ERROR); efi_firmware_set_fmp_state_var(&state, image_index); -- cgit v1.2.3 From 6f7fb8d29f26e8d67bde9717f4679e2df45724bc Mon Sep 17 00:00:00 2001 From: Jonathan Humphreys Date: Wed, 26 Feb 2025 16:35:47 -0600 Subject: board: remove capsule update support in set_dfu_alt_info() Now that capsule update sets the dfu_alt_info environment variable explicitly, there is no need to support it in the set_dfu_alt_info() function. Decouple SET_DFU_ALT_INFO from EFI_CAPSULE_FIRMWARE_FIT and EFI_CAPSULE_FIRMWARE_RAW. For many boards, this was the only use of set_dfu_alt_info() so remove the function entirely. Fixes: a9e6f01a941f ("efi: Define set_dfu_alt_info() for boards with UEFI capsule update enabled") Signed-off-by: Jonathan Humphreys Signed-off-by: Michal Simek Reviewed-by: Mattijs Korpershoek Reviewed-by: Neil Armstrong # for board/libre-computer/* Reviewed-by: Ilias Apalodimas Reviewed-by: Wadim Egorov # for --- lib/efi_loader/efi_firmware.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'lib/efi_loader/efi_firmware.c') diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index 0ffaf5c8f72..d44dc09813e 100644 --- a/lib/efi_loader/efi_firmware.c +++ b/lib/efi_loader/efi_firmware.c @@ -56,11 +56,6 @@ struct fmp_state { u32 last_attempt_status; /* not used */ }; -__weak void set_dfu_alt_info(char *interface, char *devstr) -{ - env_set("dfu_alt_info", update_info.dfu_string); -} - /** * efi_firmware_get_image_type_id - get image_type_id * @image_index: image index -- cgit v1.2.3