| Message ID | 20230323110906.23783-2-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | FMP versioning support | expand |
Kojima-san, On Thu, Mar 23, 2023 at 08:09:01PM +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, it contains the > firmware version and lowest supported version. > > This commit utilizes the FMP Payload Header, reads 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.kojima@linaro.org> > --- > 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 | 250 ++++++++++++++++++++++++++++++---- > 1 file changed, 222 insertions(+), 28 deletions(-) > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index 93e2b01c07..fb5f7906d3 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> [...] > +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))) { Why is this a memcmp? if (header->signature == FMP_PAYLOAD_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); I guess this needs to run regardless of the result to set fw_version etc? /efi_firmware_parse_payload_header() is a bit misleading though. It doesn't parse the fmp payload header. Instead it sets some values that are contained in the header and adjusts the image size accordingly. Can we come up with a better name? > + > + return ret; > +} > + > +/** > + * efi_firmware_print_result - print the firmware update result > + * @status: status code > + * @state Pointer to fmp state > + * > + * Print the firmware update result > + */ > +void efi_firmware_print_result(efi_status_t status, struct fmp_state *state) > +{ > + if (status == EFI_SUCCESS) { > + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) > + log_info("Firmware successfully written\n"); > + else > + log_info("Firmware updated to version %u\n", > + state->fw_version); > + } > } This is an overkill tbh. Even if A/B updates are supported the firmware is updated to a newer version. Whether or not the new version will be *accepted* on a subsequent reboot is irrelevant. Moreover if we define a function it should be static, but I would prefer just getting rid of it. > > /** > @@ -330,7 +494,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 +504,25 @@ 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)) { > + status = EFI_DEVICE_ERROR; > + state.last_attempt_status = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; > + goto err; > + } > > - if (fit_update(image)) > - return EFI_EXIT(EFI_DEVICE_ERROR); > + 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); > + efi_firmware_print_result(status, &state); > + > + return EFI_EXIT(status); > } > > const struct efi_firmware_management_protocol efi_fmp_fit = { > @@ -391,7 +568,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 +578,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)) { > /* > @@ -411,15 +591,29 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( > ret = fwu_get_image_index(&image_index); > if (ret) { > 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; > + > + /* TODO: implement versioning for FWU multi bank update */ > + if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) > + efi_firmware_set_fmp_state_var(&state, image_index, updated); > + > + efi_firmware_print_result(status, &state); > > - return EFI_EXIT(EFI_SUCCESS); > + return EFI_EXIT(status); > } > > const struct efi_firmware_management_protocol efi_fmp_raw = { > -- > 2.17.1 > Thanks /Ilias
Kojima-san
Apologies for the late review comments but I have some concerns about
the design and storing the info as EFI variables. If the backing
storage is a file we can't trust any of that information since anyone
can tamper with the file, although the variables are defined as RO.
The new struct defines
struct fmp_state {
u32 fw_version;
u32 lowest_supported_version;
u32 last_attempt_version;
u32 last_attempt_status;
}
I think we should change the approach a bit.
fw_version -> this should go into the signature.dts we use to store
the EFI capsule certificates
lowest_supported_version -> same here, put it into the .dts file
last_attempt_version -> This is essentially the fw_version *after* the
upgrade is successful
last_attempt_status -> CapsuleXXXX holds that result for us and we
already have code to parse it.
This shields further against someone trying to manipulate
lowest_supported_version, in case someone uses this for rollback
protection checks.
Can we modify this patch accordingly?
Regards
/Ilias
On Tue, 28 Mar 2023 at 14:53, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Kojima-san,
>
> On Thu, Mar 23, 2023 at 08:09:01PM +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, it contains the
> > firmware version and lowest supported version.
> >
> > This commit utilizes the FMP Payload Header, reads 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.kojima@linaro.org>
> > ---
Hi Ilias, On Tue, 28 Mar 2023 at 22:02, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Kojima-san > Apologies for the late review comments but I have some concerns about > the design and storing the info as EFI variables. If the backing > storage is a file we can't trust any of that information since anyone > can tamper with the file, although the variables are defined as RO. > > The new struct defines > struct fmp_state { > u32 fw_version; > u32 lowest_supported_version; > u32 last_attempt_version; > u32 last_attempt_status; > } > > I think we should change the approach a bit. > fw_version -> this should go into the signature.dts we use to store > the EFI capsule certificates > lowest_supported_version -> same here, put it into the .dts file OK, I will add these two values into .dts file. > last_attempt_version -> This is essentially the fw_version *after* the > upgrade is successful > last_attempt_status -> CapsuleXXXX holds that result for us and we > already have code to parse it. To fill ESRT, I think these two values are not very important. I will omit these values in my next series. > > This shields further against someone trying to manipulate > lowest_supported_version, in case someone uses this for rollback > protection checks. > > Can we modify this patch accordingly? Thank you for your comment. I will revise this series. Regards, Masahisa Kojima > > Regards > /Ilias > > > > > On Tue, 28 Mar 2023 at 14:53, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Kojima-san, > > > > On Thu, Mar 23, 2023 at 08:09:01PM +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, it contains the > > > firmware version and lowest supported version. > > > > > > This commit utilizes the FMP Payload Header, reads 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.kojima@linaro.org> > > > ---
Kojima-san On Wed, 29 Mar 2023 at 10:34, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > Hi Ilias, > > On Tue, 28 Mar 2023 at 22:02, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Kojima-san > > Apologies for the late review comments but I have some concerns about > > the design and storing the info as EFI variables. If the backing > > storage is a file we can't trust any of that information since anyone > > can tamper with the file, although the variables are defined as RO. > > > > The new struct defines > > struct fmp_state { > > u32 fw_version; > > u32 lowest_supported_version; > > u32 last_attempt_version; > > u32 last_attempt_status; > > } > > > > I think we should change the approach a bit. > > fw_version -> this should go into the signature.dts we use to store > > the EFI capsule certificates > > lowest_supported_version -> same here, put it into the .dts file > > OK, I will add these two values into .dts file. I think you can keep most of the code as-is. IOW keep the struct and just fill in the values from the .dts. I think that would make the abstraction easier when we try to add similar functionality to A/B updates. Regards /Ilias > > > last_attempt_version -> This is essentially the fw_version *after* the > > upgrade is successful > > last_attempt_status -> CapsuleXXXX holds that result for us and we > > already have code to parse it. > > To fill ESRT, I think these two values are not very important. > I will omit these values in my next series. > > > > > This shields further against someone trying to manipulate > > lowest_supported_version, in case someone uses this for rollback > > protection checks. > > > > Can we modify this patch accordingly? > > Thank you for your comment. > I will revise this series. > > Regards, > Masahisa Kojima > > > > > Regards > > /Ilias > > > > > > > > > > On Tue, 28 Mar 2023 at 14:53, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Kojima-san, > > > > > > On Thu, Mar 23, 2023 at 08:09:01PM +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, it contains the > > > > firmware version and lowest supported version. > > > > > > > > This commit utilizes the FMP Payload Header, reads 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.kojima@linaro.org> > > > > ---
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c index 93e2b01c07..fb5f7906d3 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,6 +37,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); @@ -102,6 +121,29 @@ efi_status_t EFIAPI efi_firmware_set_package_info_unsupported( return EFI_EXIT(EFI_UNSUPPORTED); } +/** + * 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 < num_image_type_guids; i++) { + if (fw_array[i].image_index == image_index) + return &fw_array[i].image_type_id; + } + + return NULL; +} + /** * efi_fill_image_desc_array - populate image descriptor array * @image_info_size: Size of @image_info @@ -182,6 +224,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,14 +233,13 @@ 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_status_t status = EFI_SUCCESS; efi_uintn_t capsule_payload_size; if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE)) { @@ -208,13 +250,14 @@ efi_status_t efi_firmware_capsule_authenticate(const void **p_image, &capsule_payload_size); if (status == EFI_SECURITY_VIOLATION) { - printf("Capsule authentication check failed. Aborting update\n"); - return status; + log_err("Capsule authentication check failed. Aborting update\n"); + state->last_attempt_status = + LAST_ATTEMPT_STATUS_ERROR_AUTH_ERROR; } else if (status != EFI_SUCCESS) { - return status; + state->last_attempt_status = + LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; } - debug("Capsule authentication successful\n"); image = capsule_payload; image_size = capsule_payload_size; } else { @@ -222,24 +265,145 @@ 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 status; +} + +/** + * 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; + 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); + size = sizeof(var_state); + ret = efi_get_variable_int(varname, image_type_id, 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; + + 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_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; +} + +/** + * efi_firmware_print_result - print the firmware update result + * @status: status code + * @state Pointer to fmp state + * + * Print the firmware update result + */ +void efi_firmware_print_result(efi_status_t status, struct fmp_state *state) +{ + if (status == EFI_SUCCESS) { + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) + log_info("Firmware successfully written\n"); + else + log_info("Firmware updated to version %u\n", + state->fw_version); + } } /** @@ -330,7 +494,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 +504,25 @@ 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)) { + status = EFI_DEVICE_ERROR; + state.last_attempt_status = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; + goto err; + } - if (fit_update(image)) - return EFI_EXIT(EFI_DEVICE_ERROR); + 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); + efi_firmware_print_result(status, &state); + + return EFI_EXIT(status); } const struct efi_firmware_management_protocol efi_fmp_fit = { @@ -391,7 +568,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 +578,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)) { /* @@ -411,15 +591,29 @@ efi_status_t EFIAPI efi_firmware_raw_set_image( ret = fwu_get_image_index(&image_index); if (ret) { 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; + + /* TODO: implement versioning for FWU multi bank update */ + if (!IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) + efi_firmware_set_fmp_state_var(&state, image_index, updated); + + efi_firmware_print_result(status, &state); - return EFI_EXIT(EFI_SUCCESS); + return EFI_EXIT(status); } const struct efi_firmware_management_protocol efi_fmp_raw = {
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, it contains the firmware version and lowest supported version. This commit utilizes the FMP Payload Header, reads 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.kojima@linaro.org> --- 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 | 250 ++++++++++++++++++++++++++++++---- 1 file changed, 222 insertions(+), 28 deletions(-)