On Thu, 27 Feb 2025 at 00:36, Jonathan Humphreys <j-humphr...@ti.com> wrote:
>
> 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 <j-humphr...@ti.com>
> Reviewed-by: Mattijs Korpershoek <mkorpersh...@baylibre.com>
> ---
> lib/efi_loader/efi_firmware.c | 51 ++++++++++++++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 3 deletions(-)
>
> 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);
> --
> 2.34.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
