On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote:
> The FMP Payload Header which EDK II capsule generation scripts
> insert has a firmware version.
> This commit reads the lowest supported version stored in the
> device tree, then check if the firmware version in FMP payload header
> of the ongoing capsule is equal or greater than the
> lowest supported version. If the firmware version is lower than
> lowest supported version, capsule update will not be performed.
>
> Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
> ---
> Changes in v6:
> - get aligned to the latest implementation
>
> Changes in v5:
> - newly implement the device tree based versioning
>
> Changes in v4:
> - use log_err() instead of printf()
>
> Changes in v2:
> - add error message when the firmware version is lower than
> lowest supported version
>
> lib/efi_loader/efi_firmware.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 00cf9a088a..7cd0016765 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void
> **p_image,
> * @image_index Image index
> * @state Pointer to fmp state
> *
> - * Verify the capsule file
> + * Verify the capsule authentication and check if the fw_version
> + * is equal or greater than the lowest supported version.
> *
> * Return: status code
> */
> @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void
> **p_image,
> u8 image_index,
> struct fmp_state *state)
> {
> + u32 lsv;
> efi_status_t ret;
> + efi_guid_t *image_type_id;
>
> ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> efi_firmware_get_fw_version(p_image, p_image_size, state);
>
> + /* check lowest_supported_version if capsule authentication passes */
> + if (ret == EFI_SUCCESS) {
What's the point of this here? Can;'t we move this check right after
efi_firmware_capsule_authenticate() and return a security violation if that
failed?
> + image_type_id = efi_firmware_get_image_type_id(image_index);
> + if (!image_type_id)
> + return EFI_INVALID_PARAMETER;
> +
> + efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv);
> + if (state->fw_version < lsv) {
> + log_err("Firmware version %u too low. Expecting >= %u.
> Aborting update\n",
> + state->fw_version, lsv);
> + return EFI_INVALID_PARAMETER;
> + }
> + }
> +
> return ret;
> }
>
> --
> 2.17.1
>
Thanks
/Ilias
