* [PATCH 0/2] efi: fix and test FMP image info authentication attribute
@ 2026-09-22 14:58 Vincent Stehlé
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-09-22 14:58 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Vincent Stehlé
Here is a couple of patches to:
- Do a small fix in the UEFI Firmware Management Protocol:
GetImageInfo() returns a correct authentication flag for the first
image, but not for the subsequent ones.
- Add a minimal FMP selftest, just enough to catch this issue, and which
should be easy enough to extend in the future.
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
---
Vincent Stehlé (2):
efi_loader: fix image info authentication attribute
efi_selftest: add minimal fmp test
lib/efi_loader/efi_firmware.c | 2 +-
lib/efi_selftest/Makefile | 2 +
lib/efi_selftest/efi_selftest_fmp.c | 135 ++++++++++++++++++++++++++++++++++++
3 files changed, 138 insertions(+), 1 deletion(-)
---
base-commit: a06e89ab05eaa2b8344d521319399333cd760ae5
change-id: 20260922-authi-329cc6a9a07c
Best regards,
--
Vincent.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] efi_loader: fix image info authentication attribute
2026-09-22 14:58 [PATCH 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
@ 2026-09-22 14:58 ` Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
2026-10-01 12:37 ` Ilias Apalodimas
2026-09-22 14:58 ` [PATCH 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
2026-10-02 8:37 ` [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2 siblings, 2 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-09-22 14:58 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Vincent Stehlé
The GetImageInfo() function of the UEFI Firmware Management Protocol
returns the authentication required attribute correctly in
AttributesSetting only for the first image; fix this.
Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Sughosh Ganu <sughosh.ganu@arm.com>
---
lib/efi_loader/efi_firmware.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index b41969c70fd..84de95f5c23 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -364,7 +364,7 @@ static efi_status_t efi_fill_image_desc_array(
/* Check if the capsule authentication is enabled */
if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
- image_info[0].attributes_setting |=
+ image_info[i].attributes_setting |=
IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
image_info[i].hardware_instance = 1;
--
2.53.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] efi_selftest: add minimal fmp test
2026-09-22 14:58 [PATCH 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
@ 2026-09-22 14:58 ` Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
2026-10-01 13:55 ` Heinrich Schuchardt
2026-10-02 8:37 ` [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2 siblings, 2 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-09-22 14:58 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Vincent Stehlé
Add a minimal test for the UEFI Firmware Management Protocol.
At this point the test only does the following:
- Get current firmware images info.
- For each image, verify consistency of the authentication flags.
This can be tested on the sandbox with the following command:
./u-boot -T -c "setenv efi_selftest firmware management protocol; \
bootefi selftest"
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
lib/efi_selftest/Makefile | 2 +
lib/efi_selftest/efi_selftest_fmp.c | 135 ++++++++++++++++++++++++++++++++++++
2 files changed, 137 insertions(+)
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 67102648703..9882d9f2748 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -80,6 +80,8 @@ obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
obj-$(CONFIG_EFI_DEBUG_SUPPORT) += efi_selftest_debug_support.o
+obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_selftest_fmp.o
+
targets += \
efi_miniapp_file_image_exception.h \
efi_miniapp_file_image_exit.h \
diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
new file mode 100644
index 00000000000..c17cf59a237
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_fmp.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test Firmware Management Protocol support.
+ *
+ * Copyright (C) 2026 Arm Ltd.
+ */
+#include <efi_selftest.h>
+
+static const efi_guid_t firmware_management_protocol_guid =
+ EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+
+static struct efi_boot_services *boottime;
+static struct efi_firmware_management_protocol *fmp;
+
+/**
+ * setup() - Setup unit test.
+ * @handle: handle of the loaded image
+ * @systable: system table
+ *
+ * Keep Boot Services and locate firmware management protocol.
+ *
+ * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
+ */
+static int setup(const efi_handle_t handle,
+ const struct efi_system_table *systable)
+{
+ efi_status_t ret;
+
+ boottime = systable->boottime;
+
+ ret = boottime->locate_protocol(&firmware_management_protocol_guid,
+ NULL, (void **)&fmp);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("Firmware management protocol not available.\n");
+ return EFI_ST_FAILURE;
+ }
+
+ return EFI_ST_SUCCESS;
+}
+
+/**
+ * execute() - Perform the test.
+ *
+ * Call GetImageInfo() to retrieve current firmware images descriptors.
+ * For each descriptor, verify image authentication consistency.
+ *
+ * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
+ */
+static int execute(void)
+{
+ efi_status_t ret;
+ efi_uintn_t img_info_size, desc_size;
+ struct efi_firmware_image_descriptor *img_info = NULL;
+ u32 desc_ver, pkg_ver;
+ u8 desc_cnt;
+ u16 *pkg_ver_name;
+ int i, result = EFI_ST_FAILURE;
+
+ /* Determine buffer size */
+ img_info_size = 0;
+ ret = fmp->get_image_info(fmp, &img_info_size, NULL, NULL, NULL, NULL,
+ NULL, NULL);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ /* Allocate enough memory for image info */
+ ret = boottime->allocate_pool(EFI_LOADER_DATA, img_info_size,
+ (void **)&img_info);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("AllocatePool returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ ret = fmp->get_image_info(fmp, &img_info_size, img_info, &desc_ver,
+ &desc_cnt, &desc_size, &pkg_ver,
+ &pkg_ver_name);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ if (desc_ver != EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION) {
+ efi_st_error("Bad DescriptorVersion %u\n", desc_ver);
+ goto out;
+ }
+
+ /* Verify each image info */
+ for (i = 0; i < desc_cnt; i++) {
+ const struct efi_firmware_image_descriptor *inf = &img_info[i];
+ u64 attr_supp = inf->attributes_supported,
+ attr_set = inf->attributes_setting;
+
+ efi_st_printf(" image %u: %ps\n", inf->image_index, inf->image_id_name);
+
+ /* Verify authentication */
+ if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
+ (attr_supp & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
+ !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
+ efi_st_error("Bad AttributesSupported %u\n",
+ (unsigned int)attr_supp);
+ goto out;
+ }
+
+ if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
+ (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
+ !(attr_set & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
+ efi_st_error("Bad AttributesSetting %u\n",
+ (unsigned int)attr_set);
+ goto out;
+ }
+ }
+
+ result = EFI_ST_SUCCESS;
+
+out:
+ if (img_info) {
+ ret = boottime->free_pool(img_info);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("FreePool returned %u\n",
+ (unsigned int)ret);
+ result = EFI_ST_FAILURE;
+ }
+ }
+
+ return result;
+}
+
+EFI_UNIT_TEST(fmp) = {
+ .name = "firmware management protocol",
+ .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+ .setup = setup,
+ .execute = execute,
+};
--
2.53.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] efi_loader: fix image info authentication attribute
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
@ 2026-10-01 1:31 ` Simon Glass
2026-10-01 12:37 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Simon Glass @ 2026-10-01 1:31 UTC (permalink / raw)
To: vincent.stehle
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Tom Rini,
Sughosh Ganu, Udit Kumar, Marek Vasut
On 2026-09-22T14:58:39, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> efi_loader: fix image info authentication attribute
>
> The GetImageInfo() function of the UEFI Firmware Management Protocol
> returns the authentication required attribute correctly in
> AttributesSetting only for the first image; fix this.
>
> Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Sughosh Ganu <sughosh.ganu@arm.com>
>
> lib/efi_loader/efi_firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] efi_selftest: add minimal fmp test
2026-09-22 14:58 ` [PATCH 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
@ 2026-10-01 1:31 ` Simon Glass
2026-10-01 15:46 ` Vincent Stehlé
2026-10-01 13:55 ` Heinrich Schuchardt
1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2026-10-01 1:31 UTC (permalink / raw)
To: vincent.stehle
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Tom Rini,
Sughosh Ganu, Udit Kumar, Marek Vasut
Hi Vincent,
On 2026-09-22T14:58:39, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> efi_selftest: add minimal fmp test
>
> Add a minimal test for the UEFI Firmware Management Protocol.
> At this point the test only does the following:
>
> - Get current firmware images info.
> - For each image, verify consistency of the authentication flags.
>
> This can be tested on the sandbox with the following command:
>
> ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> bootefi selftest"
>
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> lib/efi_selftest/Makefile | 2 +
> lib/efi_selftest/efi_selftest_fmp.c | 135 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 137 insertions(+)
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + /* Verify each image info */
> + for (i = 0; i < desc_cnt; i++) {
> + const struct efi_firmware_image_descriptor *inf = &img_info[i];
The spec says callers must step through the array using
DescriptorSize, since firmware may return a larger (newer) descriptor
than the one we know about. U-Boot happens to use sizeof(), but since
this is a protocol test, please can you use desc_size as the stride,
e.g. (void *)img_info + i * desc_size, and perhaps check that
desc_size >= sizeof(*inf)?
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + /* Verify authentication */
> + if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
> + (attr_supp & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> + !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
> + efi_st_error("Bad AttributesSupported %u\n",
> + (unsigned int)attr_supp);
> + goto out;
> + }
efi_firmware.c uses IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE), so
please match that. There is no SPL variant of this option, so
CONFIG_IS_ENABLED() just adds confusion.
This only checks one direction. You could also check that any bit set
in AttributesSetting is also set in AttributesSupported, and that
AUTHENTICATION_REQUIRED is not set in AttributesSetting when
authentication is disabled. That would also catch regressions on
boards without CONFIG_EFI_CAPSULE_AUTHENTICATE. What do you think?
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + ret = boottime->locate_protocol(&firmware_management_protocol_guid,
> + NULL, (void **)&fmp);
This only tests the first FMP instance. With
CONFIG_EFI_CAPSULE_FIRMWARE_FIT and _RAW both enabled on sandbox there
can be two, so using locate_handle_buffer() and checking each one
would make the test more useful as it grows. I'm fine with leaving
that for later though, since the patch says 'minimal'.
Regards,
Simon
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] efi_loader: fix image info authentication attribute
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
@ 2026-10-01 12:37 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2026-10-01 12:37 UTC (permalink / raw)
To: Vincent Stehlé
Cc: u-boot, Heinrich Schuchardt, Tom Rini, Sughosh Ganu, Udit Kumar,
Marek Vasut
On Tue, 22 Sept 2026 at 17:58, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>
> The GetImageInfo() function of the UEFI Firmware Management Protocol
> returns the authentication required attribute correctly in
> AttributesSetting only for the first image; fix this.
>
> Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Sughosh Ganu <sughosh.ganu@arm.com>
> ---
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> lib/efi_loader/efi_firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index b41969c70fd..84de95f5c23 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -364,7 +364,7 @@ static efi_status_t efi_fill_image_desc_array(
>
> /* Check if the capsule authentication is enabled */
> if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
> - image_info[0].attributes_setting |=
> + image_info[i].attributes_setting |=
> IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
>
> image_info[i].hardware_instance = 1;
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] efi_selftest: add minimal fmp test
2026-09-22 14:58 ` [PATCH 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
@ 2026-10-01 13:55 ` Heinrich Schuchardt
2026-10-02 9:26 ` Vincent Stehlé
1 sibling, 1 reply; 17+ messages in thread
From: Heinrich Schuchardt @ 2026-10-01 13:55 UTC (permalink / raw)
To: Vincent Stehlé
Cc: Ilias Apalodimas, Tom Rini, Sughosh Ganu, Udit Kumar, Marek Vasut,
u-boot
On 9/22/26 16:58, Vincent Stehlé wrote:
> Add a minimal test for the UEFI Firmware Management Protocol.
> At this point the test only does the following:
>
> - Get current firmware images info.
> - For each image, verify consistency of the authentication flags.
>
> This can be tested on the sandbox with the following command:
>
> ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> bootefi selftest"
>
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> lib/efi_selftest/Makefile | 2 +
> lib/efi_selftest/efi_selftest_fmp.c | 135 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 137 insertions(+)
>
> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
> index 67102648703..9882d9f2748 100644
> --- a/lib/efi_selftest/Makefile
> +++ b/lib/efi_selftest/Makefile
> @@ -80,6 +80,8 @@ obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
>
> obj-$(CONFIG_EFI_DEBUG_SUPPORT) += efi_selftest_debug_support.o
>
> +obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_selftest_fmp.o
> +
> targets += \
> efi_miniapp_file_image_exception.h \
> efi_miniapp_file_image_exit.h \
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> new file mode 100644
> index 00000000000..c17cf59a237
> --- /dev/null
> +++ b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Test Firmware Management Protocol support.
> + *
> + * Copyright (C) 2026 Arm Ltd.
> + */
> +#include <efi_selftest.h>
> +
> +static const efi_guid_t firmware_management_protocol_guid =
> + EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +
> +static struct efi_boot_services *boottime;
> +static struct efi_firmware_management_protocol *fmp;
> +
> +/**
> + * setup() - Setup unit test.
> + * @handle: handle of the loaded image
> + * @systable: system table
> + *
> + * Keep Boot Services and locate firmware management protocol.
> + *
> + * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
> + */
> +static int setup(const efi_handle_t handle,
> + const struct efi_system_table *systable)
> +{
> + efi_status_t ret;
> +
> + boottime = systable->boottime;
> +
> + ret = boottime->locate_protocol(&firmware_management_protocol_guid,
> + NULL, (void **)&fmp);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("Firmware management protocol not available.\n");
> + return EFI_ST_FAILURE;
> + }
> +
> + return EFI_ST_SUCCESS;
> +}
> +
> +/**
> + * execute() - Perform the test.
> + *
> + * Call GetImageInfo() to retrieve current firmware images descriptors.
> + * For each descriptor, verify image authentication consistency.
> + *
> + * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
> + */
> +static int execute(void)
> +{
> + efi_status_t ret;
> + efi_uintn_t img_info_size, desc_size;
> + struct efi_firmware_image_descriptor *img_info = NULL;
> + u32 desc_ver, pkg_ver;
> + u8 desc_cnt;
> + u16 *pkg_ver_name;
> + int i, result = EFI_ST_FAILURE;
> +
> + /* Determine buffer size */
> + img_info_size = 0;
> + ret = fmp->get_image_info(fmp, &img_info_size, NULL, NULL, NULL, NULL,
> + NULL, NULL);
> + if (ret != EFI_BUFFER_TOO_SMALL) {
> + efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
> + goto out;
> + }
This fails if the number of images is 0.
ERROR: GetImageInfo returned 2 (EFI_INVALID_PARAMETER)
+struct efi_fw_image fw_images[] = {
+};
+
+struct efi_capsule_update_info update_info = {
+ .dfu_string = NULL,
+ .num_images = ARRAY_SIZE(fw_images),
+ .images = fw_images,
+};
This looks like a bug in efi_fill_image_desc_array((). Why does it
return EFI_INVALID_PARAMETER if the number of images is 0?
We should just return *img_info_size = 0 and EFI_SUCCESS instead.
The test should return EFI_ST_SUCCESS in this case.
Best regards
Heinrich
> +
> + /* Allocate enough memory for image info */
> + ret = boottime->allocate_pool(EFI_LOADER_DATA, img_info_size,
> + (void **)&img_info);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("AllocatePool returned %u\n", (unsigned int)ret);
> + goto out;
> + }
> +
> + ret = fmp->get_image_info(fmp, &img_info_size, img_info, &desc_ver,
> + &desc_cnt, &desc_size, &pkg_ver,
> + &pkg_ver_name);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
> + goto out;
> + }
> +
> + if (desc_ver != EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION) {
> + efi_st_error("Bad DescriptorVersion %u\n", desc_ver);
> + goto out;
> + }
> +
> + /* Verify each image info */
> + for (i = 0; i < desc_cnt; i++) {
> + const struct efi_firmware_image_descriptor *inf = &img_info[i];
> + u64 attr_supp = inf->attributes_supported,
> + attr_set = inf->attributes_setting;
> +
> + efi_st_printf(" image %u: %ps\n", inf->image_index, inf->image_id_name);
> +
> + /* Verify authentication */
> + if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
> + (attr_supp & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> + !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
> + efi_st_error("Bad AttributesSupported %u\n",
> + (unsigned int)attr_supp);
> + goto out;
> + }
> +
> + if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
> + (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> + !(attr_set & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
> + efi_st_error("Bad AttributesSetting %u\n",
> + (unsigned int)attr_set);
> + goto out;
> + }
> + }
> +
> + result = EFI_ST_SUCCESS;
> +
> +out:
> + if (img_info) {
> + ret = boottime->free_pool(img_info);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("FreePool returned %u\n",
> + (unsigned int)ret);
> + result = EFI_ST_FAILURE;
> + }
> + }
> +
> + return result;
> +}
> +
> +EFI_UNIT_TEST(fmp) = {
> + .name = "firmware management protocol",
> + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
> + .setup = setup,
> + .execute = execute,
> +};
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] efi_selftest: add minimal fmp test
2026-10-01 1:31 ` Simon Glass
@ 2026-10-01 15:46 ` Vincent Stehlé
0 siblings, 0 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-10-01 15:46 UTC (permalink / raw)
To: Simon Glass
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Tom Rini,
Sughosh Ganu, Udit Kumar, Marek Vasut
On Thu, Oct 01, 2026 at 02:31:48AM +0100, Simon Glass wrote:
> Hi Vincent,
Hi Simon,
Thank you very much for your review.
I will send a v2 addressing all your remarks.
Best regards,
Vincent.
>
> On 2026-09-22T14:58:39, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> > efi_selftest: add minimal fmp test
> >
> > Add a minimal test for the UEFI Firmware Management Protocol.
> > At this point the test only does the following:
> >
> > - Get current firmware images info.
> > - For each image, verify consistency of the authentication flags.
> >
> > This can be tested on the sandbox with the following command:
> >
> > ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> > bootefi selftest"
> >
> > Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >
> > lib/efi_selftest/Makefile | 2 +
> > lib/efi_selftest/efi_selftest_fmp.c | 135 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 137 insertions(+)
>
> > diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> > @@ -0,0 +1,135 @@
> > + /* Verify each image info */
> > + for (i = 0; i < desc_cnt; i++) {
> > + const struct efi_firmware_image_descriptor *inf = &img_info[i];
>
> The spec says callers must step through the array using
> DescriptorSize, since firmware may return a larger (newer) descriptor
> than the one we know about. U-Boot happens to use sizeof(), but since
> this is a protocol test, please can you use desc_size as the stride,
> e.g. (void *)img_info + i * desc_size, and perhaps check that
> desc_size >= sizeof(*inf)?
>
> > diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> > @@ -0,0 +1,135 @@
> > + /* Verify authentication */
> > + if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
> > + (attr_supp & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> > + !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
> > + efi_st_error("Bad AttributesSupported %u\n",
> > + (unsigned int)attr_supp);
> > + goto out;
> > + }
>
> efi_firmware.c uses IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE), so
> please match that. There is no SPL variant of this option, so
> CONFIG_IS_ENABLED() just adds confusion.
>
> This only checks one direction. You could also check that any bit set
> in AttributesSetting is also set in AttributesSupported, and that
> AUTHENTICATION_REQUIRED is not set in AttributesSetting when
> authentication is disabled. That would also catch regressions on
> boards without CONFIG_EFI_CAPSULE_AUTHENTICATE. What do you think?
>
> > diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> > @@ -0,0 +1,135 @@
> > + ret = boottime->locate_protocol(&firmware_management_protocol_guid,
> > + NULL, (void **)&fmp);
>
> This only tests the first FMP instance. With
> CONFIG_EFI_CAPSULE_FIRMWARE_FIT and _RAW both enabled on sandbox there
> can be two, so using locate_handle_buffer() and checking each one
> would make the test more useful as it grows. I'm fine with leaving
> that for later though, since the patch says 'minimal'.
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute
2026-09-22 14:58 [PATCH 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
2026-09-22 14:58 ` [PATCH 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
@ 2026-10-02 8:37 ` Vincent Stehlé
2026-10-02 8:37 ` [PATCH v2 1/2] efi_loader: fix " Vincent Stehlé
2026-10-02 8:37 ` [PATCH v2 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
2 siblings, 2 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-10-02 8:37 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Simon Glass, Vincent Stehlé
Here is a couple of patches to:
- Do a small fix in the UEFI Firmware Management Protocol:
GetImageInfo() returns a correct authentication flag for the first
image, but not for the subsequent ones.
- Add a minimal FMP selftest, just enough to catch this issue, and which
should be easy enough to extend in the future.
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
---
Changes in v2:
- Rework FMP selftest with Simon's suggestions (changes details in the
patch).
- Link to v1: https://patch.msgid.link/20260922-authi-v1-0-e943530349bb@arm.com
---
Vincent Stehlé (2):
efi_loader: fix image info authentication attribute
efi_selftest: add minimal fmp test
lib/efi_loader/efi_firmware.c | 2 +-
lib/efi_selftest/Makefile | 2 +
lib/efi_selftest/efi_selftest_fmp.c | 240 ++++++++++++++++++++++++++++++++++++
3 files changed, 243 insertions(+), 1 deletion(-)
---
base-commit: c92d8c3c431e37a17c3b37918037dbd7ade32d1e
change-id: 20260922-authi-329cc6a9a07c
Best regards,
--
Vincent.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] efi_loader: fix image info authentication attribute
2026-10-02 8:37 ` [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
@ 2026-10-02 8:37 ` Vincent Stehlé
2026-10-03 5:39 ` Simon Glass
2026-10-08 10:33 ` Ilias Apalodimas
2026-10-02 8:37 ` [PATCH v2 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
1 sibling, 2 replies; 17+ messages in thread
From: Vincent Stehlé @ 2026-10-02 8:37 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Simon Glass, Vincent Stehlé
The GetImageInfo() function of the UEFI Firmware Management Protocol
returns the authentication required attribute correctly in
AttributesSetting only for the first image; fix this.
Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Sughosh Ganu <sughosh.ganu@arm.com>
---
lib/efi_loader/efi_firmware.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index b41969c70fd..84de95f5c23 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -364,7 +364,7 @@ static efi_status_t efi_fill_image_desc_array(
/* Check if the capsule authentication is enabled */
if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
- image_info[0].attributes_setting |=
+ image_info[i].attributes_setting |=
IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
image_info[i].hardware_instance = 1;
--
2.53.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] efi_selftest: add minimal fmp test
2026-10-02 8:37 ` [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2026-10-02 8:37 ` [PATCH v2 1/2] efi_loader: fix " Vincent Stehlé
@ 2026-10-02 8:37 ` Vincent Stehlé
2026-10-03 5:40 ` Simon Glass
1 sibling, 1 reply; 17+ messages in thread
From: Vincent Stehlé @ 2026-10-02 8:37 UTC (permalink / raw)
To: u-boot
Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Sughosh Ganu,
Udit Kumar, Marek Vasut, Simon Glass, Vincent Stehlé
Add a minimal test for the UEFI Firmware Management Protocol.
At this point the test only does the following:
For all handles in the system with FMP,
- Get current firmware images info.
- For each image, verify consistency of the flags, with some attention to
authentication.
This can be tested on the sandbox with the following command:
./u-boot -T -c "setenv efi_selftest firmware management protocol; \
bootefi selftest"
Testing with multiple handles supporting FMP can be done by enabling
both CONFIG_EFI_CAPSULE_FIRMWARE_FIT and
CONFIG_EFI_CAPSULE_FIRMWARE_RAW.
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Check that DescriptorSize is not smaller than the sizeof() U-Boot
knows about, and use DescriptorSize as the stride when iterating the
img_info[] array.
- Use IS_ENABLED() instead of CONFIG_IS_ENABLED().
- Check that any bit set in AttributesSetting is also set in
AttributesSupported.
- Check that AUTHENTICATION_REQUIRED is not set in AttributesSetting when
authentication is disabled.
- Use locate_handle_buffer() to discover all handles in the system with
FMP, and test them all.
(All those changes above suggested by Simon.)
- As a precaution, check that the total size of the descriptors is not
larger than ImageInfoSize.
- Verify that U-Boot knows all the image attributes set in
AttributesSetting and AttributesSupported.
- Move the original test to a test_fmp() function, which tests a single
handle, and call it on all the discovered handles.
- Print also the handle when printing images informations.
- Rely only on AttributesSetting to determine if an image is updatable.
- Do not forget to free PackageVersionName.
---
lib/efi_selftest/Makefile | 2 +
lib/efi_selftest/efi_selftest_fmp.c | 240 ++++++++++++++++++++++++++++++++++++
2 files changed, 242 insertions(+)
diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 67102648703..9882d9f2748 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -80,6 +80,8 @@ obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
obj-$(CONFIG_EFI_DEBUG_SUPPORT) += efi_selftest_debug_support.o
+obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_selftest_fmp.o
+
targets += \
efi_miniapp_file_image_exception.h \
efi_miniapp_file_image_exit.h \
diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
new file mode 100644
index 00000000000..e082888990e
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_fmp.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test Firmware Management Protocol support.
+ *
+ * Copyright (C) 2026 Arm Ltd.
+ */
+#include <efi_selftest.h>
+
+static const efi_guid_t firmware_management_protocol_guid =
+ EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+
+static struct efi_boot_services *boottime;
+
+/**
+ * setup() - Setup unit test.
+ * @handle: handle of the loaded image
+ * @systable: system table
+ *
+ * Keep Boot Services.
+ *
+ * Return: EFI_ST_SUCCESS for success.
+ */
+static int setup(const efi_handle_t handle,
+ const struct efi_system_table *systable)
+{
+ boottime = systable->boottime;
+ return EFI_ST_SUCCESS;
+}
+
+/**
+ * test_fmp() - Perform FMP test on one handle.
+ * @handle: firmware management protocol handle to test
+ *
+ * Call GetImageInfo() to retrieve current firmware images descriptors.
+ * For each descriptor, verify image authentication consistency.
+ *
+ * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
+ */
+static int test_fmp(efi_handle_t handle)
+{
+ efi_status_t ret;
+ efi_uintn_t img_info_size, desc_size, total_size;
+ struct efi_firmware_image_descriptor *img_info = NULL;
+ u32 desc_ver, pkg_ver;
+ u8 desc_cnt;
+ u16 *pkg_ver_name = NULL;
+ int i, result = EFI_ST_FAILURE;
+ struct efi_firmware_management_protocol *fmp;
+
+ /* Get the FMP of this handle */
+ ret = boottime->handle_protocol(handle,
+ &firmware_management_protocol_guid,
+ (void **)&fmp);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("HandleProtocol returned %u\n", (unsigned int)ret);
+ return EFI_ST_FAILURE;
+ }
+
+ /* Determine buffer size */
+ img_info_size = 0;
+ ret = fmp->get_image_info(fmp, &img_info_size, NULL, NULL, NULL, NULL,
+ NULL, NULL);
+ if (ret != EFI_BUFFER_TOO_SMALL) {
+ efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ /* Allocate enough memory for image info */
+ ret = boottime->allocate_pool(EFI_LOADER_DATA, img_info_size,
+ (void **)&img_info);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("AllocatePool returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ ret = fmp->get_image_info(fmp, &img_info_size, img_info, &desc_ver,
+ &desc_cnt, &desc_size, &pkg_ver,
+ &pkg_ver_name);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
+ goto out;
+ }
+
+ if (desc_ver != EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION) {
+ efi_st_error("Bad DescriptorVersion %u\n", desc_ver);
+ goto out;
+ }
+
+ if (desc_size < sizeof(struct efi_firmware_image_descriptor) ||
+ __builtin_mul_overflow(desc_size, desc_cnt, &total_size) ||
+ total_size > img_info_size) {
+ efi_st_error("Bad DescriptorSize %u\n",
+ (unsigned int)desc_size);
+ goto out;
+ }
+
+ /* Verify each image info */
+ for (i = 0; i < desc_cnt; i++) {
+ const struct efi_firmware_image_descriptor *inf =
+ (const struct efi_firmware_image_descriptor *)
+ ((uintptr_t)img_info + i * desc_size);
+ u64 attr_supp = inf->attributes_supported,
+ attr_set = inf->attributes_setting;
+ const u64 attr_mask = IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
+ IMAGE_ATTRIBUTE_RESET_REQUIRED |
+ IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
+ IMAGE_ATTRIBUTE_IN_USE |
+ IMAGE_ATTRIBUTE_UEFI_IMAGE |
+ IMAGE_ATTRIBUTE_DEPENDENCY;
+
+ efi_st_printf(" handle %p, image %u: %ps\n", handle,
+ inf->image_index, inf->image_id_name);
+
+ /* No unknown image attributes supported or set. */
+ if (attr_supp & ~attr_mask) {
+ efi_st_error("Unknown AttributesSupported %u\n",
+ (unsigned int)attr_supp);
+ goto out;
+ }
+
+ if (attr_set & ~attr_mask) {
+ efi_st_error("Unknown AttributesSetting %u\n",
+ (unsigned int)attr_set);
+ goto out;
+ }
+
+ /* No unsupported image attributes set. */
+ if (attr_set & ~attr_supp) {
+ efi_st_error("Unsupported AttributesSetting %u\n",
+ (unsigned int)attr_set);
+ goto out;
+ }
+
+ /*
+ * When support for authentication is compiled in, our updatable
+ * images must require it, both as supported and set.
+ */
+ if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
+ (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
+ !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
+ efi_st_error("Bad AttributesSupported %u\n",
+ (unsigned int)attr_supp);
+ goto out;
+ }
+
+ if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
+ (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
+ !(attr_set & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
+ efi_st_error("Bad AttributesSetting %u\n",
+ (unsigned int)attr_set);
+ goto out;
+ }
+
+ /*
+ * When support for authentication is not compiled in, our
+ * updatable images must not advertise it as set.
+ */
+ if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
+ (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
+ (attr_set & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
+ efi_st_error("Bad AttributesSetting %u\n",
+ (unsigned int)attr_set);
+ goto out;
+ }
+ }
+
+ result = EFI_ST_SUCCESS;
+
+out:
+ if (img_info) {
+ ret = boottime->free_pool(img_info);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("FreePool returned %u\n",
+ (unsigned int)ret);
+ result = EFI_ST_FAILURE;
+ }
+ }
+
+ if (pkg_ver_name) {
+ ret = boottime->free_pool(pkg_ver_name);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("FreePool returned %u\n",
+ (unsigned int)ret);
+ result = EFI_ST_FAILURE;
+ }
+ }
+
+ return result;
+}
+
+/**
+ * execute() - Perform the test on all handles with FMP.
+ *
+ * Call test_fmp() on each handle in the system supporting FMP.
+ *
+ * Return: EFI_ST_SUCCESS for success, EFI_ST_FAILURE in case of error.
+ */
+static int execute(void)
+{
+ int i, result = EFI_ST_SUCCESS;
+ efi_handle_t *buffer = NULL;
+ efi_uintn_t count;
+ efi_status_t ret;
+
+ /* Locate all handles with FMP */
+ ret = boottime->locate_handle_buffer(BY_PROTOCOL,
+ &firmware_management_protocol_guid,
+ NULL, &count, &buffer);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("LocateHandleBuffer returned %u\n",
+ (unsigned int)ret);
+ return EFI_ST_FAILURE;
+ }
+
+ /* Test all handles */
+ for (i = 0; i < count; i++) {
+ result = test_fmp(buffer[i]);
+ if (result != EFI_ST_SUCCESS)
+ break;
+ }
+
+ if (buffer) {
+ /* Free handles buffer */
+ ret = boottime->free_pool(buffer);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("FreePool returned %u\n",
+ (unsigned int)ret);
+ result = EFI_ST_FAILURE;
+ }
+ }
+
+ return result;
+}
+
+EFI_UNIT_TEST(fmp) = {
+ .name = "firmware management protocol",
+ .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+ .setup = setup,
+ .execute = execute,
+};
--
2.53.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] efi_selftest: add minimal fmp test
2026-10-01 13:55 ` Heinrich Schuchardt
@ 2026-10-02 9:26 ` Vincent Stehlé
2026-10-02 14:39 ` Heinrich Schuchardt
0 siblings, 1 reply; 17+ messages in thread
From: Vincent Stehlé @ 2026-10-02 9:26 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, Tom Rini, Sughosh Ganu, Udit Kumar, Marek Vasut,
u-boot
On Thu, Oct 01, 2026 at 03:55:10PM +0200, Heinrich Schuchardt wrote:
> On 9/22/26 16:58, Vincent Stehlé wrote:
> > Add a minimal test for the UEFI Firmware Management Protocol.
(..)
> > + /* Determine buffer size */
> > + img_info_size = 0;
> > + ret = fmp->get_image_info(fmp, &img_info_size, NULL, NULL, NULL, NULL,
> > + NULL, NULL);
> > + if (ret != EFI_BUFFER_TOO_SMALL) {
> > + efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
> > + goto out;
> > + }
>
> This fails if the number of images is 0.
>
> ERROR: GetImageInfo returned 2 (EFI_INVALID_PARAMETER)
>
> +struct efi_fw_image fw_images[] = {
> +};
> +
> +struct efi_capsule_update_info update_info = {
> + .dfu_string = NULL,
> + .num_images = ARRAY_SIZE(fw_images),
> + .images = fw_images,
> +};
>
> This looks like a bug in efi_fill_image_desc_array((). Why does it return
> EFI_INVALID_PARAMETER if the number of images is 0?
>
> We should just return *img_info_size = 0 and EFI_SUCCESS instead.
>
> The test should return EFI_ST_SUCCESS in this case.
Hi Heinrich,
Thank you very much for reviewing.
I confirm that I could reproduce this corner case with your changes.
In the end I think the test (v2) itself is fine, as it does pass when I
replace the calls to get_image_info() by the following code:
+ img_info = NULL;
+ desc_ver = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
+ desc_cnt = 0;
+ desc_size = sizeof(struct efi_firmware_image_descriptor);
+ pkg_ver = 0;
+ pkg_ver_name = NULL;
I will try to send a fix for efi_fill_image_desc_array() as you
mentioned, but if you agree that will be separated from this series.
Best regards,
Vincent.
>
> Best regards
>
> Heinrich
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] efi_selftest: add minimal fmp test
2026-10-02 9:26 ` Vincent Stehlé
@ 2026-10-02 14:39 ` Heinrich Schuchardt
0 siblings, 0 replies; 17+ messages in thread
From: Heinrich Schuchardt @ 2026-10-02 14:39 UTC (permalink / raw)
To: Vincent Stehlé
Cc: Ilias Apalodimas, Tom Rini, Sughosh Ganu, Udit Kumar, Marek Vasut,
u-boot
Am 2. Oktober 2026 11:26:38 MESZ schrieb "Vincent Stehlé" <vincent.stehle@arm.com>:
>On Thu, Oct 01, 2026 at 03:55:10PM +0200, Heinrich Schuchardt wrote:
>> On 9/22/26 16:58, Vincent Stehlé wrote:
>> > Add a minimal test for the UEFI Firmware Management Protocol.
>(..)
>> > + /* Determine buffer size */
>> > + img_info_size = 0;
>> > + ret = fmp->get_image_info(fmp, &img_info_size, NULL, NULL, NULL, NULL,
>> > + NULL, NULL);
>> > + if (ret != EFI_BUFFER_TOO_SMALL) {
>> > + efi_st_error("GetImageInfo returned %u\n", (unsigned int)ret);
>> > + goto out;
>> > + }
>>
>> This fails if the number of images is 0.
>>
>> ERROR: GetImageInfo returned 2 (EFI_INVALID_PARAMETER)
>>
>> +struct efi_fw_image fw_images[] = {
>> +};
>> +
>> +struct efi_capsule_update_info update_info = {
>> + .dfu_string = NULL,
>> + .num_images = ARRAY_SIZE(fw_images),
>> + .images = fw_images,
>> +};
>>
>> This looks like a bug in efi_fill_image_desc_array((). Why does it return
>> EFI_INVALID_PARAMETER if the number of images is 0?
>>
>> We should just return *img_info_size = 0 and EFI_SUCCESS instead.
>>
>> The test should return EFI_ST_SUCCESS in this case.
>
>Hi Heinrich,
>
>Thank you very much for reviewing.
>
>I confirm that I could reproduce this corner case with your changes.
>
>In the end I think the test (v2) itself is fine, as it does pass when I
>replace the calls to get_image_info() by the following code:
>
>+ img_info = NULL;
>+ desc_ver = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
>+ desc_cnt = 0;
>+ desc_size = sizeof(struct efi_firmware_image_descriptor);
>+ pkg_ver = 0;
>+ pkg_ver_name = NULL;
>
>I will try to send a fix for efi_fill_image_desc_array() as you
>mentioned, but if you agree that will be separated from this series.
The UEFI specification does not define if AllocatePool(size = 0) may return NULL which would lead to a failure of the subsequent Free().
I think we should return if size = 0 before trying to allocate memory.
Best regards
Heinrich
>
>Best regards,
>Vincent.
>
>>
>> Best regards
>>
>> Heinrich
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] efi_loader: fix image info authentication attribute
2026-10-02 8:37 ` [PATCH v2 1/2] efi_loader: fix " Vincent Stehlé
@ 2026-10-03 5:39 ` Simon Glass
2026-10-08 10:33 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Simon Glass @ 2026-10-03 5:39 UTC (permalink / raw)
To: vincent.stehle
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Tom Rini,
Sughosh Ganu, Udit Kumar, Marek Vasut, Simon Glass
On 2026-10-02T08:37:46, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> efi_loader: fix image info authentication attribute
>
> The GetImageInfo() function of the UEFI Firmware Management Protocol
> returns the authentication required attribute correctly in
> AttributesSetting only for the first image; fix this.
>
> Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Sughosh Ganu <sughosh.ganu@arm.com>
>
> lib/efi_loader/efi_firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] efi_selftest: add minimal fmp test
2026-10-02 8:37 ` [PATCH v2 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
@ 2026-10-03 5:40 ` Simon Glass
2026-10-03 15:23 ` Tom Rini
0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2026-10-03 5:40 UTC (permalink / raw)
To: vincent.stehle
Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas, Tom Rini,
Sughosh Ganu, Udit Kumar, Marek Vasut, Simon Glass
Hi Vincent,
On 2026-10-02T08:37:46, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> efi_selftest: add minimal fmp test
>
> Add a minimal test for the UEFI Firmware Management Protocol.
> At this point the test only does the following:
>
> For all handles in the system with FMP,
> - Get current firmware images info.
> - For each image, verify consistency of the flags, with some attention to
> authentication.
>
> This can be tested on the sandbox with the following command:
>
> ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> bootefi selftest"
>
> Testing with multiple handles supporting FMP can be done by enabling
> both CONFIG_EFI_CAPSULE_FIRMWARE_FIT and
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW.
>
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> [...]
>
> lib/efi_selftest/Makefile | 2 +
> lib/efi_selftest/efi_selftest_fmp.c | 240 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 242 insertions(+)
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,240 @@
> + if (desc_size < sizeof(struct efi_firmware_image_descriptor) ||
> + __builtin_mul_overflow(desc_size, desc_cnt, &total_size) ||
> + total_size > img_info_size) {
Please can you use check_mul_overflow() from <linux/overflow.h> rather
than the builtin? Nothing else in lib/efi_selftest or lib/efi_loader
uses the builtin directly.
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,240 @@
> + if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) &&
> + (attr_set & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> + (attr_set & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
Just to check, is there a reason to limit this to updatable images? I
suspect AUTHENTICATION_REQUIRED should never be set when
authentication is disabled, so the IMAGE_UPDATABLE condition could be
dropped. Not a blocker though.
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,240 @@
> + int i, result = EFI_ST_SUCCESS;
> + efi_handle_t *buffer = NULL;
> + efi_uintn_t count;
Since count is an efi_uintn_t, please can you make i the same type, to
avoid the signed/unsigned comparison in the loop?
Regards,
Simon
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] efi_selftest: add minimal fmp test
2026-10-03 5:40 ` Simon Glass
@ 2026-10-03 15:23 ` Tom Rini
0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2026-10-03 15:23 UTC (permalink / raw)
To: Simon Glass
Cc: vincent.stehle, u-boot, Heinrich Schuchardt, Ilias Apalodimas,
Sughosh Ganu, Udit Kumar, Marek Vasut
[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]
On Sat, Oct 03, 2026 at 06:40:04AM +0100, Simon Glass wrote:
> Hi Vincent,
>
> On 2026-10-02T08:37:46, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> > efi_selftest: add minimal fmp test
> >
> > Add a minimal test for the UEFI Firmware Management Protocol.
> > At this point the test only does the following:
> >
> > For all handles in the system with FMP,
> > - Get current firmware images info.
> > - For each image, verify consistency of the flags, with some attention to
> > authentication.
> >
> > This can be tested on the sandbox with the following command:
> >
> > ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> > bootefi selftest"
> >
> > Testing with multiple handles supporting FMP can be done by enabling
> > both CONFIG_EFI_CAPSULE_FIRMWARE_FIT and
> > CONFIG_EFI_CAPSULE_FIRMWARE_RAW.
> >
> > Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> > [...]
> >
> > lib/efi_selftest/Makefile | 2 +
> > lib/efi_selftest/efi_selftest_fmp.c | 240 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 242 insertions(+)
>
> > diff --git a/lib/efi_selftest/efi_selftest_fmp.c b/lib/efi_selftest/efi_selftest_fmp.c
> > @@ -0,0 +1,240 @@
> > + if (desc_size < sizeof(struct efi_firmware_image_descriptor) ||
> > + __builtin_mul_overflow(desc_size, desc_cnt, &total_size) ||
> > + total_size > img_info_size) {
>
> Please can you use check_mul_overflow() from <linux/overflow.h> rather
> than the builtin? Nothing else in lib/efi_selftest or lib/efi_loader
> uses the builtin directly.
Importing that header first and making use of it more widely second
would be a reasonable thing for a backlog / TODO list for someone. As
Rasmus is the original author in Linux, it might even be on his list
somewhere (along with re-syncing the related compiler header parts).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/2] efi_loader: fix image info authentication attribute
2026-10-02 8:37 ` [PATCH v2 1/2] efi_loader: fix " Vincent Stehlé
2026-10-03 5:39 ` Simon Glass
@ 2026-10-08 10:33 ` Ilias Apalodimas
1 sibling, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2026-10-08 10:33 UTC (permalink / raw)
To: Vincent Stehlé
Cc: u-boot, Heinrich Schuchardt, Tom Rini, Sughosh Ganu, Udit Kumar,
Marek Vasut, Simon Glass
On Fri, 2 Oct 2026 at 11:38, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>
> The GetImageInfo() function of the UEFI Firmware Management Protocol
> returns the authentication required attribute correctly in
> AttributesSetting only for the first image; fix this.
>
> Fixes: 88a2ef2720f5 ("efi_loader: Enable uefi capsule authentication")
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Sughosh Ganu <sughosh.ganu@arm.com>
> ---
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> lib/efi_loader/efi_firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index b41969c70fd..84de95f5c23 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -364,7 +364,7 @@ static efi_status_t efi_fill_image_desc_array(
>
> /* Check if the capsule authentication is enabled */
> if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
> - image_info[0].attributes_setting |=
> + image_info[i].attributes_setting |=
> IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED;
>
> image_info[i].hardware_instance = 1;
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-10-08 10:33 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 14:58 [PATCH 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2026-09-22 14:58 ` [PATCH 1/2] efi_loader: fix " Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
2026-10-01 12:37 ` Ilias Apalodimas
2026-09-22 14:58 ` [PATCH 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
2026-10-01 1:31 ` Simon Glass
2026-10-01 15:46 ` Vincent Stehlé
2026-10-01 13:55 ` Heinrich Schuchardt
2026-10-02 9:26 ` Vincent Stehlé
2026-10-02 14:39 ` Heinrich Schuchardt
2026-10-02 8:37 ` [PATCH v2 0/2] efi: fix and test FMP image info authentication attribute Vincent Stehlé
2026-10-02 8:37 ` [PATCH v2 1/2] efi_loader: fix " Vincent Stehlé
2026-10-03 5:39 ` Simon Glass
2026-10-08 10:33 ` Ilias Apalodimas
2026-10-02 8:37 ` [PATCH v2 2/2] efi_selftest: add minimal fmp test Vincent Stehlé
2026-10-03 5:40 ` Simon Glass
2026-10-03 15:23 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.