[PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
Masahisa Kojima
masahisa.kojima at linaro.org
Tue May 23 03:55:00 CEST 2023
On Tue, 23 May 2023 at 06:24, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Kojima-san,
>
> On Fri, May 19, 2023 at 07:32:08PM +0900, Masahisa Kojima wrote:
> > Firmware version management is not implemented in the current
> > FMP protocol.
> > EDK II reference implementation capsule generation script inserts
> > the FMP Payload Header right before the payload, FMP Payload Header
> > contains the firmware version and lowest supported version.
> >
> > This commit utilizes the FMP Payload Header, reads the header and
> > stores the firmware version into "FmpStateXXXX" EFI non-volatile variable.
> > XXXX indicates the image index, since FMP protocol handles multiple
> > image indexes.
> > Note that lowest supported version included in the FMP Payload Header
> > is not used. If the platform uses file-based EFI variable storage,
> > it can be tampered. The file-based EFI variable storage is not the
> > right place to store the lowest supported version for anti-rollback
> > protection.
> >
> > This change is compatible with the existing FMP implementation.
> > This change does not mandate the FMP Payload Header.
> > If no FMP Payload Header is found in the capsule file, fw_version,
> > lowest supported version, last attempt version and last attempt
> > status is 0 and this is the same behavior as existing FMP
> > implementation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > Changed in v6:
> > - only store the fw_version in the FmpState EFI variable
> >
> > Changes in v4:
> > - move lines that are the same in both branches out of the if statement
> > - s/EDK2/EDK II/
> > - create print result function
> > - set last_attempt_version when capsule authentication failed
> > - use log_err() instead of printf()
> >
> > Changes in v3:
> > - exclude CONFIG_FWU_MULTI_BANK_UPDATE case
> > - set image_type_id as a vendor field of FmpStateXXXX variable
> > - set READ_ONLY flag for FmpStateXXXX variable
> > - add error code for FIT image case
> >
> > Changes in v2:
> > - modify indent
> >
>
> [...]
>
> > +/**
> > + * efi_firmware_get_fw_version - get fw_version from FMP payload header
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @state Pointer to fmp state
> > + *
> > + * Parse the FMP payload header and fill the fmp_state structure.
> > + * If no FMP payload header is found, fmp_state structure is not updated.
> > + *
> > + */
> > +static void efi_firmware_get_fw_version(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + struct fmp_state *state)
> > +{
> > + const void *image = *p_image;
> > + efi_uintn_t image_size = *p_image_size;
> > + const struct fmp_payload_header *header;
> > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> > +
> > + header = image;
> > + if (header->signature == fmp_hdr_signature) {
> > + /* FMP header is inserted above the capsule payload */
> > + state->fw_version = header->fw_version;
> >
> > - if (!memcmp(&header->signature, &fmp_hdr_signature,
> > - sizeof(fmp_hdr_signature))) {
> > - /*
> > - * When building the capsule with the scripts in
> > - * edk2, a FMP header is inserted above the capsule
> > - * payload. Compensate for this header to get the
> > - * actual payload that is to be updated.
> > - */
> > image += header->header_size;
> > image_size -= header->header_size;
> > }
>
> >
> > *p_image = image;
> > *p_image_size = image_size;
>
> Can we get rid of the extra image/image_size here and move this inside the
> if()?
Yes, thanks.
>
> > - return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_verify_image - verify image
>
> The name is a bit generic here, we need something which describes what
> happens better. The verification already happens in
> efi_firmware_capsule_authenticate().
In the "[PATCH v6 5/8] efi_loader: check lowest supported version" in
this series,
checking lowest_supported_version is added in efi_firmware_verify_image().
So, can we keep this function name?
Regards,
Masahisa Kojima
> Maybe efi_prepare_capsule() or something like that ?
>
> > + * @p_image: Pointer to new image
> > + * @p_image_size: Pointer to size of new image
> > + * @image_index Image index
> > + * @state Pointer to fmp state
> > + *
> > + * Verify the capsule file
> > + *
> > + * Return: status code
> > + */
> > +static
> > +efi_status_t efi_firmware_verify_image(const void **p_image,
> > + efi_uintn_t *p_image_size,
> > + u8 image_index,
> > + struct fmp_state *state)
> > +{
> > + efi_status_t ret;
> > +
> > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size);
> > + efi_firmware_get_fw_version(p_image, p_image_size, state);
> > +
> > + return ret;
> > }
> >
> > /**
> > @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > u16 **abort_reason)
> > {
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
> > if (!image || image_index != 1)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > if (fit_update(image))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> > @@ -392,6 +519,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > {
> > int ret;
> > efi_status_t status;
> > + struct fmp_state state = { 0 };
> >
> > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
> > image_size, vendor_code, progress, abort_reason);
> > @@ -399,7 +527,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > if (!image)
> > return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> > - status = efi_firmware_capsule_authenticate(&image, &image_size);
> > + status = efi_firmware_verify_image(&image, &image_size, image_index,
> > + &state);
> > if (status != EFI_SUCCESS)
> > return EFI_EXIT(status);
> >
> > @@ -419,6 +548,8 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> > NULL, NULL))
> > return EFI_EXIT(EFI_DEVICE_ERROR);
> >
> > + efi_firmware_set_fmp_state_var(&state, image_index);
> > +
> > return EFI_EXIT(EFI_SUCCESS);
> > }
> >
> > --
> > 2.17.1
> >
>
> Thanks
> /Ilias
More information about the U-Boot
mailing list