Hi Jonathan, On Mon, 3 Feb 2025 at 23:54, 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> > --- > lib/efi_loader/efi_firmware.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index 5a754c9cd03..d8b6d34ccab 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,9 +665,22 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > if (status != EFI_SUCCESS) > return EFI_EXIT(status); > > + orig_dfu_env = env_get("dfu_alt_info");
I don't think this will work. env_get returns a pointer to the data which is going to change after the env_set IOW this piece env_set("dfu_alt_info", "test1"); orig_dfu_env = env_get("dfu_alt_info"); printf("now %s\n", orig_dfu_env); -> will print test1 env_set("dfu_alt_info", "test2"); dfu_env = env_get("dfu_alt_info"); printf("Orig %s now %s\n", orig_dfu_env, lala); -> will print test2 test2 You have to copy the value of the pointer to an allocated piece of memory to restore it Thanks /Ilias > + ret = env_set("dfu_alt_info", update_info.dfu_string); > + if (ret) { > + pr_err("unable to set env variable \"dfu_alt_info\"!\n"); > + return -EINVAL; > + } > + > if (fit_update(image)) > return EFI_EXIT(EFI_DEVICE_ERROR); > > + ret = env_set("dfu_alt_info", orig_dfu_env); > + if (ret) { > + pr_err("unable to set env variable \"dfu_alt_info\"!\n"); > + return -EINVAL; > + } > + > efi_firmware_set_fmp_state_var(&state, image_index); > > return EFI_EXIT(EFI_SUCCESS); > @@ -717,6 +732,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,10 +763,23 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > } > } > > + orig_dfu_env = env_get("dfu_alt_info"); > + ret = env_set("dfu_alt_info", update_info.dfu_string); > + if (ret) { > + pr_err("unable to set env variable \"dfu_alt_info\"!\n"); > + return -EINVAL; > + } > + > if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size, > NULL, NULL)) > return EFI_EXIT(EFI_DEVICE_ERROR); > > + ret = env_set("dfu_alt_info", orig_dfu_env); > + if (ret) { > + pr_err("unable to set env variable \"dfu_alt_info\"!\n"); > + return -EINVAL; > + } > + > efi_firmware_set_fmp_state_var(&state, image_index); > > return EFI_EXIT(EFI_SUCCESS); > -- > 2.34.1 >