Heinrich,
On Thu, Sep 09, 2021 at 12:30:49AM +0300, Ilias Apalodimas wrote:
> There is two unneeded EFI_CALL references in tcg2_measure_pe_image().
> The first one in efi_search_protocol() and the second on in the device path
> calculation. The second isn't even a function we should be calling, but a
> pointer assignment, which happens to work with the existing macro.
It is a quite common mistake.
For most people, it is not very trivial when we should use EFI_CALL
and when should not.
Do you think that we should leave a note somewhere?
It would be much better if we can check any occurrence of mismatch,
say using EFI_CALL for a non-EFIAPI function, at compile time.
-Takahiro Akashi
> While at it switch the malloc call to a calloc, remove the unnecessary cast
> and get rid of an unneeded if statement before copying the device path
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> ---
> lib/efi_loader/efi_tcg2.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 1319a8b37868..d026af2b2350 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -839,20 +839,19 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
> efi_size,
> if (ret != EFI_SUCCESS)
> return ret;
>
> - ret = EFI_CALL(efi_search_protocol(&handle->header,
> - &efi_guid_loaded_image_device_path,
> - &handler));
> + ret = efi_search_protocol(&handle->header,
> + &efi_guid_loaded_image_device_path, &handler);
> if (ret != EFI_SUCCESS)
> return ret;
>
> - device_path = EFI_CALL(handler->protocol_interface);
> + device_path = handler->protocol_interface;
> device_path_length = efi_dp_size(device_path);
> if (device_path_length > 0) {
> /* add end node size */
> device_path_length += sizeof(struct efi_device_path);
> }
> event_size = sizeof(struct uefi_image_load_event) + device_path_length;
> - image_load_event = (struct uefi_image_load_event *)malloc(event_size);
> + image_load_event = calloc(1, event_size);
> if (!image_load_event)
> return EFI_OUT_OF_RESOURCES;
>
> @@ -875,10 +874,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
> efi_size,
> goto out;
> }
>
> - if (device_path_length > 0) {
> - memcpy(image_load_event->device_path, device_path,
> - device_path_length);
> - }
> + /* device_path_length might be zero */
> + memcpy(image_load_event->device_path, device_path, device_path_length);
>
> ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> event_size, (u8 *)image_load_event);
> --
> 2.33.0
>
