Re: [PATCH] efi_selftest: fix guid comparison

["Vincent Stehlé" <vincent.stehle@arm.com>]

On Sat, Jun 13, 2026 at 10:22:39AM +0200, Heinrich Schuchardt wrote:
> On 6/11/26 11:29, Vincent Stehlé wrote:
> > The `loaded image' efi selftest is comparing protocol GUIDs with the wrong
> > polarity.
> > This can be verified on the sandbox, where two protocols GUIDs are
> > retrieved by the test from the image handle in the following order:
> > 
> > 1. Loaded Image Device Path Protocol GUID
> > 2. Loaded Image Protocol GUID
> > 
> > The test matches on the first GUID, while it is in fact looking for the
> > second one; fix the comparison polarity.
> > 
> > While at it, use guidcmp() instead of memcmp() to spare the size parameter,
> > and use the Loaded Image Protocol GUID definition from efi_api.h instead or
> > redefining it locally.
> 
> Hello Vincent,
> 
> Thank you for catching the issue with the incorrect checkout for the Loaded
> Image Protocol.

Hi Heinrich,

Thanks for your review.

> We never use U-Boot library functions in efi_selftest to allow converting it
> into a standalone EFI application.

Ok, this makes sense.
I will send a v2 keeping memcmp().

Note that I see 4 calls to guidcmp() in the selftests already. I think this does
not harm in practice because guidcmp() this is marked inline.
I also see more than a dozen places where memcmp() is used to compare GUIDs in
the selftests.

Would you like me to send another series to add a local definition of guidcmp()
for the selftests and convert all memcmp() when appropriate?
Or should I rather send a series converting all uses of guidcmp() in the
selftests to memcmp(), maybe?

> 
> > 
> > Fixes: efe79a7c0de0 ("efi_selftest: test for loaded image protocol")
> > 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: Alexander Graf <agraf@suse.de>
> > ---
> >   lib/efi_selftest/efi_selftest_loaded_image.c | 7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/efi_selftest/efi_selftest_loaded_image.c b/lib/efi_selftest/efi_selftest_loaded_image.c
> > index 5889ab12617..48890a66db7 100644
> > --- a/lib/efi_selftest/efi_selftest_loaded_image.c
> > +++ b/lib/efi_selftest/efi_selftest_loaded_image.c
> > @@ -9,9 +9,7 @@
> >   #include <efi_selftest.h>
> > -static efi_guid_t loaded_image_protocol_guid =
> > -	EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2,
> > -		 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b);
> > +static efi_guid_t loaded_image_protocol_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
> 
> This change is not described in the commit message.

It was a bit hidden, but it was mentioned:

  (..) and use the Loaded Image Protocol GUID definition from efi_api.h instead
  or redefining it locally.

> With the change we no
> longer test if efi_api.h has the correct value of the GUID. Why would you
> prefer this?

Sure, it could make sense to duplicate the GUIDs definitions for test purposes.
I will keep the definition as it is in the v2.

Note that we have a dozen places in the selftests using the common GUID
definitions from efi_api.h; shall I send a patch series to replace those?

Best regards,
Vincent.

> 
> >   static struct efi_boot_services *boottime;
> >   efi_handle_t image_handle;
> > @@ -60,8 +58,7 @@ static int execute(void)
> >   	efi_st_printf("%u protocols installed on image handle\n",
> >   		      (unsigned int)protocol_buffer_count);
> >   	for (i = 0; i < protocol_buffer_count; ++i) {
> > -		if (memcmp(protocol_buffer[i], &loaded_image_protocol_guid,
> > -			   sizeof(efi_guid_t)))
> > +		if (!guidcmp(protocol_buffer[i], &loaded_image_protocol_guid))
> >   			found = true;
> 
> Changing the polarity is correct.
> 
> Best regards
> 
> Heinrich
> 
> >   	}
> >   	if (!found) {

-- 
Vincent.