Hi Vincent,
On 2026-10-02T08:37:46, Vincent Stehlé <[email protected]> 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é <[email protected]>
> [...]
>
> 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
