public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH] efi_loader: fix ecpt size computation
@ 2026-02-11  9:33 Vincent Stehlé
  2026-02-11  9:56 ` Heinrich Schuchardt
  2026-02-12 14:40 ` [PATCH v2] " Vincent Stehlé
  0 siblings, 2 replies; 7+ messages in thread
From: Vincent Stehlé @ 2026-02-11  9:33 UTC (permalink / raw)
  To: u-boot
  Cc: Vincent Stehlé, Heinrich Schuchardt, Ilias Apalodimas,
	Tom Rini, Jose Marinho

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.

Fixes: 6b92c1735205 ("efi: Create ECPT table")
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: Jose Marinho <jose.marinho@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[] = {
+#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);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi_loader: fix ecpt size computation
  2026-02-11  9:33 [PATCH] efi_loader: fix ecpt size computation Vincent Stehlé
@ 2026-02-11  9:56 ` Heinrich Schuchardt
  2026-02-11 11:47   ` Vincent Stehlé
  2026-02-12 14:40 ` [PATCH v2] " Vincent Stehlé
  1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2026-02-11  9:56 UTC (permalink / raw)
  To: Vincent Stehlé
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Jose Marinho,
	u-boot

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.

Maybe mention in the commit message that U-Boot should publish all 
supported EBBR revisions.

> 
> Fixes: 6b92c1735205 ("efi: Create ECPT table")
> 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: Jose Marinho <jose.marinho@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.

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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi_loader: fix ecpt size computation
  2026-02-11  9:56 ` Heinrich Schuchardt
@ 2026-02-11 11:47   ` Vincent Stehlé
  2026-02-11 11:50     ` Heinrich Schuchardt
  0 siblings, 1 reply; 7+ messages in thread
From: Vincent Stehlé @ 2026-02-11 11:47 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Jose Marinho,
	u-boot

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@arm.com>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Jose Marinho <jose.marinho@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.

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);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi_loader: fix ecpt size computation
  2026-02-11 11:47   ` Vincent Stehlé
@ 2026-02-11 11:50     ` Heinrich Schuchardt
  2026-02-11 12:25       ` Vincent Stehlé
  0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2026-02-11 11:50 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Ilias Apalodimas, Tom Rini, Jose Marinho, u-boot

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@arm.com>
>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: Jose Marinho <jose.marinho@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);
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] efi_loader: fix ecpt size computation
  2026-02-11 11:50     ` Heinrich Schuchardt
@ 2026-02-11 12:25       ` Vincent Stehlé
  0 siblings, 0 replies; 7+ messages in thread
From: Vincent Stehlé @ 2026-02-11 12:25 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Heinrich Schuchardt, Ilias Apalodimas, Tom Rini, Jose Marinho,
	u-boot

On Wed, Feb 11, 2026 at 12:50:38PM +0100, Heinrich Schuchardt wrote:
> 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:
(snip)
> > > > 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.

Hi Heinrich,

The profiles[] array is "local" to the efi_conformance.c file already as it has
internal linkage, thanks to the static specifier. Is this the symbol name you
would like to prefix maybe?

	static const efi_guid_t efi_conformance_profiles[] = {
		...

Best regards,
Vincent.

> 
> Best regards
> 
> Heinrich

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] efi_loader: fix ecpt size computation
  2026-02-11  9:33 [PATCH] efi_loader: fix ecpt size computation Vincent Stehlé
  2026-02-11  9:56 ` Heinrich Schuchardt
@ 2026-02-12 14:40 ` Vincent Stehlé
  2026-02-15  7:30   ` Heinrich Schuchardt
  1 sibling, 1 reply; 7+ messages in thread
From: Vincent Stehlé @ 2026-02-12 14:40 UTC (permalink / raw)
  To: u-boot
  Cc: Vincent Stehlé, Heinrich Schuchardt, Ilias Apalodimas,
	Tom Rini, Jose Marinho

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 as U-Boot should publish the
GUIDs for all supported EBBR revisions.

Fixes: 6b92c1735205 ("efi: Create ECPT table")
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Jose Marinho <jose.marinho@arm.com>
---


Changes for v2:
- Mention in the commit message that U-Boot should publish all supported
  EBBR GUIDs (suggested by Heinrich)
- Move the array declaration into the function (suggested by Heinrich)


 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..2d31800ccb8 100644
--- a/lib/efi_loader/efi_conformance.c
+++ b/lib/efi_loader/efi_conformance.c
@@ -13,8 +13,6 @@
 #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;
 
 /**
  * efi_ecpt_register() - Install the ECPT system table.
@@ -23,12 +21,17 @@ 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)
+	static const efi_guid_t profiles[] = {
+	#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
+		EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
+	#endif
+	};
+
+	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);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] efi_loader: fix ecpt size computation
  2026-02-12 14:40 ` [PATCH v2] " Vincent Stehlé
@ 2026-02-15  7:30   ` Heinrich Schuchardt
  0 siblings, 0 replies; 7+ messages in thread
From: Heinrich Schuchardt @ 2026-02-15  7:30 UTC (permalink / raw)
  To: Vincent Stehlé; +Cc: Ilias Apalodimas, Tom Rini, Jose Marinho, u-boot

On 2/12/26 15:40, 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 as U-Boot should publish the
> GUIDs for all supported EBBR revisions.
> 
> Fixes: 6b92c1735205 ("efi: Create ECPT table")
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>0
> Cc: Jose Marinho <jose.marinho@arm.com>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
> 
> 
> Changes for v2:
> - Mention in the commit message that U-Boot should publish all supported
>    EBBR GUIDs (suggested by Heinrich)
> - Move the array declaration into the function (suggested by Heinrich)
> 
> 
>   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..2d31800ccb8 100644
> --- a/lib/efi_loader/efi_conformance.c
> +++ b/lib/efi_loader/efi_conformance.c
> @@ -13,8 +13,6 @@
>   #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;
>   
>   /**
>    * efi_ecpt_register() - Install the ECPT system table.
> @@ -23,12 +21,17 @@ 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)
> +	static const efi_guid_t profiles[] = {
> +	#if CONFIG_IS_ENABLED(EFI_EBBR_2_1_CONFORMANCE)
> +		EFI_CONFORMANCE_PROFILE_EBBR_2_1_GUID,
> +	#endif
> +	};
> +
> +	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);


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-02-15  7:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-11  9:33 [PATCH] efi_loader: fix ecpt size computation Vincent Stehlé
2026-02-11  9:56 ` Heinrich Schuchardt
2026-02-11 11:47   ` Vincent Stehlé
2026-02-11 11:50     ` Heinrich Schuchardt
2026-02-11 12:25       ` Vincent Stehlé
2026-02-12 14:40 ` [PATCH v2] " Vincent Stehlé
2026-02-15  7:30   ` Heinrich Schuchardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox