[PATCH 0/2] efi: fix and test FMP image info authentication attribute

[PATCH 0/2] efi: fix and test FMP image info authentication attribute

[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.

[PATCH 1/2] efi_loader: fix image info authentication attribute

[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

Re: [PATCH 1/2] efi_loader: fix image info authentication attribute

[Simon Glass]

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>

Re: [PATCH 1/2] efi_loader: fix image info authentication attribute

[Ilias Apalodimas]

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
>

[PATCH 2/2] efi_selftest: add minimal fmp test

[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

Re: [PATCH 2/2] efi_selftest: add minimal fmp test

[Simon Glass]

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

Re: [PATCH 2/2] efi_selftest: add minimal fmp test

[Vincent Stehlé]

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

Re: [PATCH 2/2] efi_selftest: add minimal fmp test

[Heinrich Schuchardt]

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,
> +};
> 

Re: [PATCH 2/2] efi_selftest: add minimal fmp test

[Vincent Stehlé]

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

Re: [PATCH 2/2] efi_selftest: add minimal fmp test

[Heinrich Schuchardt]

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

[PATCH v2 0/2] efi: fix and test FMP image info authentication attribute

[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.

[PATCH v2 1/2] efi_loader: fix image info authentication attribute

[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

Re: [PATCH v2 1/2] efi_loader: fix image info authentication attribute

[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>

Re: [PATCH v2 1/2] efi_loader: fix image info authentication attribute

[Ilias Apalodimas]

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
>

[PATCH v2 2/2] efi_selftest: add minimal fmp test

[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

Re: [PATCH v2 2/2] efi_selftest: add minimal fmp test

[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

Re: [PATCH v2 2/2] efi_selftest: add minimal fmp test

[Tom Rini]

[-- 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 --]