| Message ID | 20230519103214.1239656-3-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | FMP versioning support | expand |
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@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()? > - 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(). 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
On Tue, 23 May 2023 at 06:24, Ilias Apalodimas <ilias.apalodimas@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@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
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@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' 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); > } >
Hi Heinrich, On Sun, 28 May 2023 at 17:39, Heinrich Schuchardt <xypron.glpk@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@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); > > } > > >
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 + * + * 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 + * + * 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); }
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@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(-)