On Thu, Mar 31, 2022 at 06:57:45PM +0530, Sughosh Ganu wrote:
> 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.g...@linaro.org>
> Reviewed-by: Masami Hiramatsu <masami.hirama...@linaro.org>
> ---
>
> Changes since V3: None
>
> 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
>
Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
