[PATCH v3 3/9] capsule: Put a check for image index before the update

[Masami Hiramatsu]

Hi Sughosh,

This looks good to me.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>

Thank you!

2022年3月30日(水) 23:51 Sughosh Ganu <sughosh.ganu at linaro.org>:
>
> The current capsule update code compares the image GUID value in the
> capsule header with the image GUID value obtained from the
> GetImageInfo function of the Firmware Management Protocol(FMP). This
> comparison is done to ascertain if the FMP's SetImage function can be
> called for the update. Make this checking more robust by comparing the
> image_index value passed through the capsule with that returned by the
> FMP's GetImageInfo function. This protects against the scenario of the
> firmware being updated in a wrong partition/location on the storage
> device if an incorrect value has been passed through the capsule,
> since the image_index is used to determine the location of the update
> on the storage device.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>
> Changes since V2: New patch
>
>  lib/efi_loader/efi_capsule.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index f00440163d..f03f4c9044 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -128,6 +128,7 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
>  /**
>   * efi_fmp_find - search for Firmware Management Protocol drivers
>   * @image_type:                Image type guid
> + * @image_index:       Image Index
>   * @instance:          Instance number
>   * @handles:           Handles of FMP drivers
>   * @no_handles:                Number of handles
> @@ -141,8 +142,8 @@ void set_capsule_result(int index, struct efi_capsule_header *capsule,
>   * * NULL              - on failure
>   */
>  static struct efi_firmware_management_protocol *
> -efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
> -            efi_uintn_t no_handles)
> +efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance,
> +            efi_handle_t *handles, efi_uintn_t no_handles)
>  {
>         efi_handle_t *handle;
>         struct efi_firmware_management_protocol *fmp;
> @@ -203,6 +204,7 @@ efi_fmp_find(efi_guid_t *image_type, u64 instance, efi_handle_t *handles,
>                         log_debug("+++ desc[%d] index: %d, name: %ls\n",
>                                   j, desc->image_index, desc->image_id_name);
>                         if (!guidcmp(&desc->image_type_id, image_type) &&
> +                           (desc->image_index == image_index) &&
>                             (!instance ||
>                              !desc->hardware_instance ||
>                               desc->hardware_instance == instance))
> @@ -449,8 +451,8 @@ static efi_status_t efi_capsule_update_firmware(
>                 }
>
>                 /* find a device for update firmware */
> -               /* TODO: should we pass index as well, or nothing but type? */
>                 fmp = efi_fmp_find(&image->update_image_type_id,
> +                                  image->update_image_index,
>                                    image->update_hardware_instance,
>                                    handles, no_handles);
>                 if (!fmp) {
> --
> 2.25.1
>


-- 
Masami Hiramatsu