[PATCH v6 2/8] efi_loader: store firmware version into FmpState variable

Masahisa Kojima masahisa.kojima at linaro.org
Tue May 30 02:31:54 CEST 2023


Hi Heinrich,

On Sun, 28 May 2023 at 17:39, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 5/19/23 12:32, 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
> >
> >   lib/efi_loader/efi_firmware.c | 161 ++++++++++++++++++++++++++++++----
> >   1 file changed, 146 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index cc650e1443..fc085e3c08 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -10,6 +10,7 @@
> >   #include <charset.h>
> >   #include <dfu.h>
> >   #include <efi_loader.h>
> > +#include <efi_variable.h>
> >   #include <fwu.h>
> >   #include <image.h>
> >   #include <signatures.h>
> > @@ -36,11 +37,52 @@ struct fmp_payload_header {
> >       u32 lowest_supported_version;
> >   };
> >
> > +/**
> > + * struct fmp_state - fmp firmware update state
> > + *
> > + * This structure describes the state of the firmware update
> > + * through FMP protocol.
> > + *
> > + * @fw_version:                      Firmware versions used
> > + * @lowest_supported_version:        Lowest supported version
> > + * @last_attempt_version:    Last attempt version
> > + * @last_attempt_status:     Last attempt status
> > + */
> > +struct fmp_state {
> > +     u32 fw_version;
> > +     u32 lowest_supported_version; /* not used */
> > +     u32 last_attempt_version; /* not used */
> > +     u32 last_attempt_status; /* not used */
> > +};
> > +
> >   __weak void set_dfu_alt_info(char *interface, char *devstr)
> >   {
> >       env_set("dfu_alt_info", update_info.dfu_string);
> >   }
> >
> > +/**
> > + * efi_firmware_get_image_type_id - get image_type_id
> > + * @image_index:     image index
> > + *
> > + * Return the image_type_id identified by the image index.
> > + *
> > + * Return:           pointer to the image_type_id, NULL if image_index is invalid
> > + */
> > +static
> > +efi_guid_t *efi_firmware_get_image_type_id(u8 image_index)
> > +{
> > +     int i;
> > +     struct efi_fw_image *fw_array;
> > +
> > +     fw_array = update_info.images;
> > +     for (i = 0; i < update_info.num_images; i++) {
> > +             if (fw_array[i].image_index == image_index)
> > +                     return &fw_array[i].image_type_id;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> >   /* Place holder; not supported */
> >   static
> >   efi_status_t EFIAPI efi_firmware_get_image_unsupported(
> > @@ -194,8 +236,6 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> >   {
> >       const void *image = *p_image;
> >       efi_uintn_t image_size = *p_image_size;
> > -     u32 fmp_hdr_signature;
> > -     struct fmp_payload_header *header;
> >       void *capsule_payload;
> >       efi_status_t status;
> >       efi_uintn_t capsule_payload_size;
> > @@ -222,24 +262,107 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image,
> >               debug("Updating capsule without authenticating.\n");
> >       }
> >
> > -     fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
> > -     header = (void *)image;
> > +     *p_image = image;
> > +     *p_image_size = image_size;
> > +     return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_set_fmp_state_var - set FmpStateXXXX variable
> > + * @state:           Pointer to fmp state
> > + * @image_index:     image index
> > + *
> > + * Update the FmpStateXXXX variable with the firmware update state.
> > + *
> > + * Return:           status code
> > + */
> > +static
> > +efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_index)
> > +{
> > +     u16 varname[13]; /* u"FmpStateXXXX" */
> > +     efi_status_t ret;
> > +     efi_guid_t *image_type_id;
> > +     struct fmp_state var_state = { 0 };
> > +
> > +     image_type_id = efi_firmware_get_image_type_id(image_index);
> > +     if (!image_type_id)
> > +             return EFI_INVALID_PARAMETER;
> > +
> > +     efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> > +                             image_index);
> > +
> > +     /*
> > +      * Only the fw_version is set here.
> > +      * lowest_supported_version in FmpState variable is ignored since
> > +      * it can be tampered if the file based EFI variable storage is used.
> > +      */
> > +     var_state.fw_version = state->fw_version;
> > +
> > +     ret = efi_set_variable_int(varname, image_type_id,
> > +                                EFI_VARIABLE_READ_ONLY |
> > +                                EFI_VARIABLE_NON_VOLATILE |
> > +                                EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +                                EFI_VARIABLE_RUNTIME_ACCESS,
> > +                                sizeof(var_state), &var_state, false);
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> > + * 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
>
> Due to the missing colon this leads to a build failure for 'make htmldocs':
>
> ./lib/efi_loader/efi_firmware.c:401: warning: Function parameter or
> member 'state' not described in 'efi_firmware_get_fw_version'
>
>
> > + *
> > + * 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;
> > -     return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > + * efi_firmware_verify_image - verify image
> > + * @p_image:         Pointer to new image
> > + * @p_image_size:    Pointer to size of new image
> > + * @image_index              Image index
> > + * @state            Pointer to fmp state
>
> Due to the missing colons this leads to a build failure for 'make htmldocs':
>
> ./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
> member 'image_index' not described in 'efi_firmware_verify_image'
> ./lib/efi_loader/efi_firmware.c:437: warning: Function parameter or
> member 'state' not described in 'efi_firmware_verify_image'

Sorry, I will fix it in the next version.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > + *
> > + * 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);
> >   }
> >
>


More information about the U-Boot mailing list