Hi Vincent,
On Fri, 23 Jan 2026 at 14:58, Vincent Stehlé <[email protected]> wrote:
>
> The efi_exit() function frees the loaded image memory by calling
> efi_delete_image(). However, when CONFIG_EFI_TCG2_PROTOCOL is enabled, the
> image_obj->image_type structure member is accessed after the memory has
> been freed.
>
> Fix this by keeping a copy of image_type, as is already done for exit_jmp.
>
> Fixes: 8fc4e0b4273a ("efi_loader: add boot variable measurement")
> Signed-off-by: Vincent Stehlé <[email protected]>
> Cc: Heinrich Schuchardt <[email protected]>
> Cc: Ilias Apalodimas <[email protected]>
> Cc: Tom Rini <[email protected]>
> Cc: Masahisa Kojima <[email protected]>
> ---
>
> Hi,
>
> This can be verified with sandbox_defconfig + CONFIG_VALGRIND=y and the
> following command:
>
> valgrind --suppressions=scripts/u-boot.supp \
> ./u-boot -T -c "setenv efi_selftest start image return; \
> bootefi selftest"
>
> Best regards,
> Vincent.
>
> lib/efi_loader/efi_boottime.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index ddc935d2240..0b3f2fd276c 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -3453,6 +3453,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
> image_handle,
> struct efi_loaded_image_obj *image_obj =
> (struct efi_loaded_image_obj *)image_handle;
> jmp_buf *exit_jmp;
> + u16 image_type;
>
> EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status,
> exit_data_size, exit_data);
> @@ -3496,13 +3497,14 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t
> image_handle,
> }
> /* efi_delete_image() frees image_obj. Copy before the call. */
> exit_jmp = image_obj->exit_jmp;
> + image_type = image_obj->image_type;
> *image_obj->exit_status = exit_status;
> - if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION ||
> + if (image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION ||
> exit_status != EFI_SUCCESS)
> efi_delete_image(image_obj, loaded_image_protocol);
>
> if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> - if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION)
> {
> + if (image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
This looks ok, but I don't remember the TCG spec on top of my head.
Can't we just move the measurement before deleting the handle?
Thanks
/Ilias
> ret = efi_tcg2_measure_efi_app_exit();
> if (ret != EFI_SUCCESS)
> log_debug("tcg2 measurement fails (0x%lx)\n",
> --
> 2.51.0
>
