Re: [PATCH v3 3/4] efi_loader: check lowest supported version in capsule update
On Wed, 22 Mar 2023 at 09:36, Masahisa Kojima
wrote:
>
> On Tue, 21 Mar 2023 at 15:42, Heinrich Schuchardt wrote:
> >
> > On 3/20/23 06:54, Masahisa Kojima wrote:
> > > The FMP Payload Header which EDK2 capsule generation scripts
> > > insert contains lowest supported version.
> > > This commit reads the lowest supported version stored in the
> > > "FmpState" EFI non-volatile variable, then check if the
> > > firmware version of ongoing capsule is equal or greater than
> > > the lowest supported version.
> > >
> > > Signed-off-by: Masahisa Kojima
> > > ---
> > > No changes since v2
> > >
> > > Changes in v2:
> > > - add error message when the firmware version is lower than
> > >lowest supported version
> > >
> > > lib/efi_loader/efi_firmware.c | 50 ++-
> > > 1 file changed, 49 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > > index 289456ecbb..8d6e32006d 100644
> > > --- a/lib/efi_loader/efi_firmware.c
> > > +++ b/lib/efi_loader/efi_firmware.c
> > > @@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void
> > > **p_image,
> > > *p_image_size = image_size;
> > > }
> > >
> > > +/**
> > > + * efi_firmware_get_lowest_supported_version - get the lowest supported
> > > version
> > > + * @image_index: image_index
> > > + *
> > > + * Get the lowest supported version from FmpState variable.
> > > + *
> > > + * Return: lowest supported version, return 0 if reading
> > > FmpState
> > > + * variable failed
> > > + */
> > > +static
> > > +u32 efi_firmware_get_lowest_supported_version(u8 image_index)
> > > +{
> > > + u16 varname[13]; /* u"FmpState" */
> > > + 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 0;
> > > +
> > > + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> > > + image_index);
> > > + size = sizeof(var_state);
> > > + ret = efi_get_variable_int(varname, image_type_id, NULL, ,
> > > +_state, NULL);
> > > + if (ret != EFI_SUCCESS)
> > > + return 0;
> > > +
> > > + return var_state.lowest_supported_version;
> > > +}
> > > +
> > > /**
> > >* efi_firmware_verify_image - verify image
> > >* @p_image:Pointer to new image
> > > @@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void
> > > **p_image,
> > >* @image_index Image index
> > >* @state Pointer to fmp state
> > >*
> > > - * Verify the capsule file
> > > + * Verify the capsule authentication and check if the fw_version
> > > + * is equal or greater than the lowest supported version.
> > >*
> > >* Return: status code
> > >*/
> > > @@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void
> > > **p_image,
> > > struct fmp_state *state)
> > > {
> > > efi_status_t ret;
> > > + u32 lowest_supported_version;
> > >
> > > ret = efi_firmware_capsule_authenticate(p_image, p_image_size,
> > > state);
> > > efi_firmware_parse_payload_header(p_image, p_image_size, state);
> > >
> > > + /* check lowest_supported_version if capsule authentication passes
> > > */
> > > + if (ret == EFI_SUCCESS) {
> > > + lowest_supported_version =
> > > +
> > > efi_firmware_get_lowest_supported_version(image_index);
> > > + if (lowest_supported_version > state->fw_version) {
> > > + printf("fw_version(%u) is too low(expected >%u).
> > > Aborting update\n",
> > > +state->fw_version, lowest_supported_version);
> >
> > Please, use log_warning() or log_err() instead of printf().
> >
> > The addressee is a user not a developer:
> >
> > "Firmware version %u too low. Expecting >= %u"
>
> OK.
>
> >
> > We should have one central place where upon exit we write a message
> > indicating that either the capsule update was successful or failed.
> >
> > "Firmware updated to version %u" : "Firmware update failed"
>
> Yes, I will add the log output for the user.
For failure cases, UpdateCapsule runtime service outputs the result as follows.
"Firmware update failed:
Applying capsule Test08 failed."
So I will not add the "Firmware update failed" in FMP->SetImage() function.
Thanks,
Masahisa Kojima
>
> Thanks,
> Masahisa Kojima
>
> >
> > Best regards
> >
> > Heinrich
> >
> > > + state->last_attempt_status =
> > > + LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
> > > + ret = EFI_INVALID_PARAMETER;
> > > + }
> > > + }
> > > +
> >
Re: [PATCH v3 3/4] efi_loader: check lowest supported version in capsule update
On Tue, 21 Mar 2023 at 15:42, Heinrich Schuchardt wrote:
>
> On 3/20/23 06:54, Masahisa Kojima wrote:
> > The FMP Payload Header which EDK2 capsule generation scripts
> > insert contains lowest supported version.
> > This commit reads the lowest supported version stored in the
> > "FmpState" EFI non-volatile variable, then check if the
> > firmware version of ongoing capsule is equal or greater than
> > the lowest supported version.
> >
> > Signed-off-by: Masahisa Kojima
> > ---
> > No changes since v2
> >
> > Changes in v2:
> > - add error message when the firmware version is lower than
> >lowest supported version
> >
> > lib/efi_loader/efi_firmware.c | 50 ++-
> > 1 file changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> > index 289456ecbb..8d6e32006d 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void
> > **p_image,
> > *p_image_size = image_size;
> > }
> >
> > +/**
> > + * efi_firmware_get_lowest_supported_version - get the lowest supported
> > version
> > + * @image_index: image_index
> > + *
> > + * Get the lowest supported version from FmpState variable.
> > + *
> > + * Return: lowest supported version, return 0 if reading
> > FmpState
> > + * variable failed
> > + */
> > +static
> > +u32 efi_firmware_get_lowest_supported_version(u8 image_index)
> > +{
> > + u16 varname[13]; /* u"FmpState" */
> > + 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 0;
> > +
> > + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> > + image_index);
> > + size = sizeof(var_state);
> > + ret = efi_get_variable_int(varname, image_type_id, NULL, ,
> > +_state, NULL);
> > + if (ret != EFI_SUCCESS)
> > + return 0;
> > +
> > + return var_state.lowest_supported_version;
> > +}
> > +
> > /**
> >* efi_firmware_verify_image - verify image
> >* @p_image:Pointer to new image
> > @@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void
> > **p_image,
> >* @image_index Image index
> >* @state Pointer to fmp state
> >*
> > - * Verify the capsule file
> > + * Verify the capsule authentication and check if the fw_version
> > + * is equal or greater than the lowest supported version.
> >*
> >* Return: status code
> >*/
> > @@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void
> > **p_image,
> > struct fmp_state *state)
> > {
> > efi_status_t ret;
> > + u32 lowest_supported_version;
> >
> > ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state);
> > efi_firmware_parse_payload_header(p_image, p_image_size, state);
> >
> > + /* check lowest_supported_version if capsule authentication passes */
> > + if (ret == EFI_SUCCESS) {
> > + lowest_supported_version =
> > +
> > efi_firmware_get_lowest_supported_version(image_index);
> > + if (lowest_supported_version > state->fw_version) {
> > + printf("fw_version(%u) is too low(expected >%u).
> > Aborting update\n",
> > +state->fw_version, lowest_supported_version);
>
> Please, use log_warning() or log_err() instead of printf().
>
> The addressee is a user not a developer:
>
> "Firmware version %u too low. Expecting >= %u"
OK.
>
> We should have one central place where upon exit we write a message
> indicating that either the capsule update was successful or failed.
>
> "Firmware updated to version %u" : "Firmware update failed"
Yes, I will add the log output for the user.
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > + state->last_attempt_status =
> > + LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
> > + ret = EFI_INVALID_PARAMETER;
> > + }
> > + }
> > +
> > return ret;
> > }
> >
>
Re: [PATCH v3 3/4] efi_loader: check lowest supported version in capsule update
On 3/20/23 06:54, Masahisa Kojima wrote:
The FMP Payload Header which EDK2 capsule generation scripts
insert contains lowest supported version.
This commit reads the lowest supported version stored in the
"FmpState" EFI non-volatile variable, then check if the
firmware version of ongoing capsule is equal or greater than
the lowest supported version.
Signed-off-by: Masahisa Kojima
---
No changes since v2
Changes in v2:
- add error message when the firmware version is lower than
lowest supported version
lib/efi_loader/efi_firmware.c | 50 ++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 289456ecbb..8d6e32006d 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void
**p_image,
*p_image_size = image_size;
}
+/**
+ * efi_firmware_get_lowest_supported_version - get the lowest supported version
+ * @image_index: image_index
+ *
+ * Get the lowest supported version from FmpState variable.
+ *
+ * Return: lowest supported version, return 0 if reading
FmpState
+ * variable failed
+ */
+static
+u32 efi_firmware_get_lowest_supported_version(u8 image_index)
+{
+ u16 varname[13]; /* u"FmpState" */
+ 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 0;
+
+ efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+ image_index);
+ size = sizeof(var_state);
+ ret = efi_get_variable_int(varname, image_type_id, NULL, ,
+ _state, NULL);
+ if (ret != EFI_SUCCESS)
+ return 0;
+
+ return var_state.lowest_supported_version;
+}
+
/**
* efi_firmware_verify_image - verify image
* @p_image: Pointer to new image
@@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void **p_image,
* @image_index Image index
* @state Pointer to fmp state
*
- * Verify the capsule file
+ * Verify the capsule authentication and check if the fw_version
+ * is equal or greater than the lowest supported version.
*
* Return:status code
*/
@@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void
**p_image,
struct fmp_state *state)
{
efi_status_t ret;
+ u32 lowest_supported_version;
ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state);
efi_firmware_parse_payload_header(p_image, p_image_size, state);
+ /* check lowest_supported_version if capsule authentication passes */
+ if (ret == EFI_SUCCESS) {
+ lowest_supported_version =
+ efi_firmware_get_lowest_supported_version(image_index);
+ if (lowest_supported_version > state->fw_version) {
+ printf("fw_version(%u) is too low(expected >%u). Aborting
update\n",
+ state->fw_version, lowest_supported_version);
Please, use log_warning() or log_err() instead of printf().
The addressee is a user not a developer:
"Firmware version %u too low. Expecting >= %u"
We should have one central place where upon exit we write a message
indicating that either the capsule update was successful or failed.
"Firmware updated to version %u" : "Firmware update failed"
Best regards
Heinrich
+ state->last_attempt_status =
+ LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
+ ret = EFI_INVALID_PARAMETER;
+ }
+ }
+
return ret;
}
[PATCH v3 3/4] efi_loader: check lowest supported version in capsule update
The FMP Payload Header which EDK2 capsule generation scripts
insert contains lowest supported version.
This commit reads the lowest supported version stored in the
"FmpState" EFI non-volatile variable, then check if the
firmware version of ongoing capsule is equal or greater than
the lowest supported version.
Signed-off-by: Masahisa Kojima
---
No changes since v2
Changes in v2:
- add error message when the firmware version is lower than
lowest supported version
lib/efi_loader/efi_firmware.c | 50 ++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 289456ecbb..8d6e32006d 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -391,6 +391,39 @@ void efi_firmware_parse_payload_header(const void
**p_image,
*p_image_size = image_size;
}
+/**
+ * efi_firmware_get_lowest_supported_version - get the lowest supported version
+ * @image_index: image_index
+ *
+ * Get the lowest supported version from FmpState variable.
+ *
+ * Return: lowest supported version, return 0 if reading
FmpState
+ * variable failed
+ */
+static
+u32 efi_firmware_get_lowest_supported_version(u8 image_index)
+{
+ u16 varname[13]; /* u"FmpState" */
+ 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 0;
+
+ efi_create_indexed_name(varname, sizeof(varname), "FmpState",
+ image_index);
+ size = sizeof(var_state);
+ ret = efi_get_variable_int(varname, image_type_id, NULL, ,
+ _state, NULL);
+ if (ret != EFI_SUCCESS)
+ return 0;
+
+ return var_state.lowest_supported_version;
+}
+
/**
* efi_firmware_verify_image - verify image
* @p_image: Pointer to new image
@@ -398,7 +431,8 @@ void efi_firmware_parse_payload_header(const void **p_image,
* @image_indexImage index
* @state Pointer to fmp state
*
- * Verify the capsule file
+ * Verify the capsule authentication and check if the fw_version
+ * is equal or greater than the lowest supported version.
*
* Return: status code
*/
@@ -409,10 +443,24 @@ efi_status_t efi_firmware_verify_image(const void
**p_image,
struct fmp_state *state)
{
efi_status_t ret;
+ u32 lowest_supported_version;
ret = efi_firmware_capsule_authenticate(p_image, p_image_size, state);
efi_firmware_parse_payload_header(p_image, p_image_size, state);
+ /* check lowest_supported_version if capsule authentication passes */
+ if (ret == EFI_SUCCESS) {
+ lowest_supported_version =
+ efi_firmware_get_lowest_supported_version(image_index);
+ if (lowest_supported_version > state->fw_version) {
+ printf("fw_version(%u) is too low(expected >%u).
Aborting update\n",
+ state->fw_version, lowest_supported_version);
+ state->last_attempt_status =
+ LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION;
+ ret = EFI_INVALID_PARAMETER;
+ }
+ }
+
return ret;
}
--
2.17.1
