[PATCH] efi_loader: fix ecpt size computation

[Heinrich Schuchardt]

On 2/11/26 12:47, Vincent Stehlé wrote:
> On Wed, Feb 11, 2026 at 10:56:30AM +0100, Heinrich Schuchardt wrote:
>> On 2/11/26 10:33, Vincent Stehlé wrote:
>>> The size of the memory allocated for the EFI Conformance Profiles Table is
>>> computed with `num_entries' always equal to zero, which is incorrect when
>>> CONFIG_EFI_EBBR_2_1_CONFORMANCE is enabled.
>>>
>>> This can be verified by allocating the ECPT memory with malloc() instead of
>>> efi_allocate_pool(), building u-boot with sandbox_defconfig and
>>> CONFIG_VALGRIND=y, and by finally running the following command:
>>>
>>>     valgrind --suppressions=scripts/u-boot.supp \
>>>       ./u-boot -T -c 'efidebug tables'
>>>
>>> Fix this by using an array of the supported profiles GUIDs instead, which
>>> should also be easier to extend in the future.
>>
>> Thank you Vincent for this patch.
> 
> Hi Heinrich,
> 
> Thanks for reviewing.
> 
>> Maybe mention in the commit message that U-Boot should publish all supported
>> EBBR revisions.
> 
> Ok, will do.
> 
>>
>>>
>>> Fixes: 6b92c1735205 ("efi: Create ECPT table")
>>> Signed-off-by: Vincent Stehlé <vincent.stehle at arm.com>
>>> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> Cc: Tom Rini <trini at konsulko.com>
>>> Cc: Jose Marinho <jose.marinho at arm.com>
>>> ---
>>>    lib/efi_loader/efi_conformance.c | 18 +++++++++---------
>>>    1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_conformance.c b/lib/efi_loader/efi_conformance.c
>>> index 2bae93a94bd..9768b5ae824 100644
>>> --- a/lib/efi_loader/efi_conformance.c
>>> +++ b/lib/efi_loader/efi_conformance.c
>>> @@ -13,8 +13,12 @@
>>>    #include <malloc.h>
>>>    static const efi_guid_t efi_ecpt_guid = EFI_CONFORMANCE_PROFILES_TABLE_GUID;
>>> -static const efi_guid_t efi_ebbr_2_1_guid =
>>> -	EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID;
>>> +
>>> +static const efi_guid_t profiles[] = {
>>
>> As profiles[] is not used outside efi_ecpt_register(), there is no need to
>> make it a static variable.
> 
> I am not sure why a "global" symbol would be better than a static one, but if
> you like this solution we could make the profiles[] array extern.

Sorry to not be clear. I wanted to suggest a local symbol.

Best regards

Heinrich

> 
> Then I guess we should rename it with a much longer name to avoid namespace
> "pollution":
> 
> 	const efi_guid_t efi_conformance_profiles[] = {
> 		...
> 
> All the sizeof() and ARRAY_SIZE() would have to follow, too.
> 
> Alternatively, we could move the array declaration inside the
> efi_ecpt_register() function completely:
> 
> 	efi_status_t efi_ecpt_register(void)
> 	{
> 		struct efi_conformance_profiles_table *ecpt;
> 		efi_status_t ret;
> 		size_t ecpt_size;
> 
> 		static const efi_guid_t profiles[] = {
> 		#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
> 			EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
> 		#endif
> 		};
> 
> 		...
> 
> This is still static here, but for storage duration not linkage. One advantage
> is that the name would not have to change. I am not a big fan of the #if in
> function scope, but if you prefer this solution this is fine as well.
> 
> Best regards,
> Vincent.
> 
>>
>> Otherwise looks good to me.
>>
>> Best regards
>>
>> Heinrich
>>
>>
>>> +#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
>>> +	EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
>>> +#endif
>>> +};
>>>    /**
>>>     * efi_ecpt_register() - Install the ECPT system table.
>>> @@ -23,12 +27,11 @@ static const efi_guid_t efi_ebbr_2_1_guid =
>>>     */
>>>    efi_status_t efi_ecpt_register(void)
>>>    {
>>> -	u16 num_entries = 0;
>>>    	struct efi_conformance_profiles_table *ecpt;
>>>    	efi_status_t ret;
>>>    	size_t ecpt_size;
>>> -	ecpt_size = num_entries * sizeof(efi_guid_t)
>>> +	ecpt_size = sizeof(profiles)
>>>    		+ sizeof(struct efi_conformance_profiles_table);
>>>    	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, ecpt_size,
>>>    				(void **)&ecpt);
>>> @@ -39,12 +42,9 @@ efi_status_t efi_ecpt_register(void)
>>>    		return ret;
>>>    	}
>>> -	if (CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE))
>>> -		guidcpy(&ecpt->conformance_profiles[num_entries++],
>>> -			&efi_ebbr_2_1_guid);
>>> -
>>> +	memcpy(ecpt->conformance_profiles, profiles, sizeof(profiles));
>>>    	ecpt->version = EFI_CONFORMANCE_PROFILES_TABLE_VERSION;
>>> -	ecpt->number_of_profiles = num_entries;
>>> +	ecpt->number_of_profiles = ARRAY_SIZE(profiles);
>>>    	/* Install the ECPT in the system configuration table. */
>>>    	ret = efi_install_configuration_table(&efi_ecpt_guid, (void *)ecpt);
>>