On Wed, Mar 01, 2023 at 06:15:19PM +0900, Masahisa Kojima wrote: > Firmware version management is not implemented in the current > FMP protocol. > EDK2 reference implementation capsule generation script inserts > the FMP Payload Header right before the payload, it contains the > firmware version and lowest supported version. > > This commit utilizes the FMP Payload Header, read the header and > stores the firmware version, lowest supported version, > last attempt version and last attempt status into "FmpStateXXXX" > EFI non-volatile variable. XXXX indicates the image index, > since FMP protocol handles multiple image indexes. > > 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.koj...@linaro.org> > --- > Changes in v2: > - modify indent > > lib/efi_loader/efi_firmware.c | 198 ++++++++++++++++++++++++++++++---- > 1 file changed, 175 insertions(+), 23 deletions(-) > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index 93e2b01c07..d1afafb052 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> > @@ -18,6 +19,12 @@ > > #define FMP_PAYLOAD_HDR_SIGNATURE SIGNATURE_32('M', 'S', 'S', '1') > > +#define EFI_FMP_STATE_GUID \ > + EFI_GUID(0x84bed885, 0x193a, 0x403f, 0xa2, 0x78, \ > + 0xe8, 0x9e, 0x23, 0x8a, 0xd6, 0xe1) > + > +static const efi_guid_t efi_guid_fmp_state = EFI_FMP_STATE_GUID; > + > /** > * struct fmp_payload_header - EDK2 header for the FMP payload > * > @@ -36,6 +43,24 @@ 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; > + u32 last_attempt_version; > + u32 last_attempt_status; > +}; > + > __weak void set_dfu_alt_info(char *interface, char *devstr) > { > env_set("dfu_alt_info", update_info.dfu_string); > @@ -182,6 +207,7 @@ static efi_status_t efi_fill_image_desc_array( > * efi_firmware_capsule_authenticate - authenticate the capsule if enabled > * @p_image: Pointer to new image > * @p_image_size: Pointer to size of new image > + * @state Pointer to fmp state > * > * Authenticate the capsule if authentication is enabled. > * The image pointer and the image size are updated in case of success. > @@ -190,12 +216,11 @@ static efi_status_t efi_fill_image_desc_array( > */ > static > efi_status_t efi_firmware_capsule_authenticate(const void **p_image, > - efi_uintn_t *p_image_size) > + efi_uintn_t *p_image_size, > + struct fmp_state *state) > { > 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; > @@ -209,8 +234,12 @@ efi_status_t efi_firmware_capsule_authenticate(const > void **p_image, > > if (status == EFI_SECURITY_VIOLATION) { > printf("Capsule authentication check failed. Aborting > update\n"); > + state->last_attempt_status = > + LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR; > return status; > } else if (status != EFI_SUCCESS) { > + state->last_attempt_status = > + LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > return status; > } > > @@ -222,24 +251,124 @@ 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 > + * @updated: flag to indicate firmware update is successful > + * > + * 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, > + bool updated) > +{ > + u16 varname[13]; /* u"FmpStateXXXX" */ > + efi_status_t ret; > + efi_uintn_t size; > + struct fmp_state var_state = { 0 }; > + > + efi_create_indexed_name(varname, sizeof(varname), "FmpState", > + image_index); > + size = sizeof(var_state); > + ret = efi_get_variable_int(varname, &efi_guid_fmp_state, NULL, &size, > + &var_state, NULL); > + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > + return ret; > + > + /* > + * When the capsule update is successful, FmpStateXXXX variable is set > + * according to the fmp payload header information. If there is no fmp > payload > + * header in the capsule file, all values are set to 0. > + * When the capsule update fails, only last attempt information of > FmpStateXXXX > + * variable is updated, fw_version and lowest_supported_version keep > original > + * value or 0(in case no FmpStateXXXX variable found). > + */ > + if (updated) { > + var_state.fw_version = state->fw_version; > + var_state.lowest_supported_version = > state->lowest_supported_version; > + var_state.last_attempt_version = state->last_attempt_version; > + var_state.last_attempt_status = state->last_attempt_status; > + } else { > + var_state.last_attempt_version = state->last_attempt_version; > + var_state.last_attempt_status = state->last_attempt_status; > + } > + > + ret = efi_set_variable_int(varname, &efi_guid_fmp_state, > + EFI_VARIABLE_NON_VOLATILE | > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS, > + sizeof(var_state), &var_state, false);
One of my concerns about using UEFI variables as a storage for internal data like firmwware status is that they are unnecessarily accessible to users. In this sense, I think that those variable should be *read-only* and maintained only by the system. -Takahiro Akashi > + return ret; > +} > > +/** > + * efi_firmware_parse_payload_header - parse 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_parse_payload_header(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 (!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. > - */ > + /* FMP header is inserted above the capsule payload */ > + state->fw_version = header->fw_version; > + state->lowest_supported_version = > header->lowest_supported_version; > + state->last_attempt_version = header->fw_version; > 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 > + * > + * 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, state); > + efi_firmware_parse_payload_header(p_image, p_image_size, state); > + > + return ret; > } > > /** > @@ -330,7 +459,9 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > efi_status_t (*progress)(efi_uintn_t completion), > u16 **abort_reason) > { > + bool updated; > 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,14 +469,22 @@ 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); > + goto err; > > - if (fit_update(image)) > - return EFI_EXIT(EFI_DEVICE_ERROR); > + if (fit_update(image)) { > + status = EFI_DEVICE_ERROR; > + goto err; > + } > > - return EFI_EXIT(EFI_SUCCESS); > + state.last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > +err: > + updated = (status == EFI_SUCCESS) ? true : false; > + efi_firmware_set_fmp_state_var(&state, image_index, updated); > + > + return EFI_EXIT(status); > } > > const struct efi_firmware_management_protocol efi_fmp_fit = { > @@ -391,7 +530,9 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > u16 **abort_reason) > { > int ret; > + bool updated; > 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,9 +540,10 @@ 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); > + goto err; > > if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) { > /* > @@ -410,16 +552,26 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > */ > ret = fwu_get_image_index(&image_index); > if (ret) { > + state.last_attempt_status = > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > log_debug("Unable to get FWU image_index\n"); > - return EFI_EXIT(EFI_DEVICE_ERROR); > + status = EFI_DEVICE_ERROR; > + goto err; > } > } > > if (dfu_write_by_alt(image_index - 1, (void *)image, image_size, > - NULL, NULL)) > - return EFI_EXIT(EFI_DEVICE_ERROR); > + NULL, NULL)) { > + status = EFI_DEVICE_ERROR; > + state.last_attempt_status = > LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > + goto err; > + } > + > + state.last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > +err: > + updated = (status == EFI_SUCCESS) ? true : false; > + efi_firmware_set_fmp_state_var(&state, image_index, updated); > > - return EFI_EXIT(EFI_SUCCESS); > + return EFI_EXIT(status); > } > > const struct efi_firmware_management_protocol efi_fmp_raw = { > -- > 2.17.1 >