[PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Vincent Stehlé]

Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
after having called EFI_ENTRY().

This is inspired by a barebox fix. [1]

Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
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>
---
 lib/efi_loader/efi_boottime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index de57823bd44..b6966652d97 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
 				      &number_of_children,
 				      &child_handle_buffer);
 	if (r != EFI_SUCCESS)
-		return r;
+		return EFI_EXIT(r);
 	sole_child = (number_of_children == 1);
 
 	if (child_handle) {
-- 
2.53.0

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Simon Glass]

Hi Vincent,

On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>
> Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
> after having called EFI_ENTRY().
>
> This is inspired by a barebox fix. [1]

Hmm, does Barebox support EFI_LOADER now? I wish we could bring these
projects back together :-)

>
> Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
> Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
> 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>
> ---
>  lib/efi_loader/efi_boottime.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index de57823bd44..b6966652d97 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
>                                       &number_of_children,
>                                       &child_handle_buffer);
>         if (r != EFI_SUCCESS)
> -               return r;
> +               return EFI_EXIT(r);
>         sole_child = (number_of_children == 1);
>
>         if (child_handle) {
> --
> 2.53.0
>

Regards,
Simon

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Ilias Apalodimas]

Hi Vincent,


On Thu, 18 Jun 2026 at 18:19, Simon Glass <sjg@chromium.org> wrote:

> Hi Vincent,
>
> On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com>
> wrote:
> >
> > Make sure to call EFI_EXIT() in efi_disconnect_controller(), when
> returning
> > after having called EFI_ENTRY().
> >
> > This is inspired by a barebox fix. [1]
>
> Hmm, does Barebox support EFI_LOADER now? I wish we could bring these
> projects back together :-)
>
> >
> > Link:
> https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69
> [1]
> > Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole
> child")
> > 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>
> > ---
> >  lib/efi_loader/efi_boottime.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c
> b/lib/efi_loader/efi_boottime.c
> > index de57823bd44..b6966652d97 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
> >                                       &number_of_children,
> >                                       &child_handle_buffer);
> >         if (r != EFI_SUCCESS)
> > -               return r;
> > +               return EFI_EXIT(r);
>

Any reason we can just goto out;

Thanks
/Ilias

> >         sole_child = (number_of_children == 1);
> >
> >         if (child_handle) {
> > --
> > 2.53.0
> >
>
> Regards,
> Simon
>

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Simon Glass]

On Thu, 18 Jun 2026 at 16:21, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Vincent,
>
>
> On Thu, 18 Jun 2026 at 18:19, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Vincent,
>>
>> On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>> >
>> > Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
>> > after having called EFI_ENTRY().
>> >
>> > This is inspired by a barebox fix. [1]
>>
>> Hmm, does Barebox support EFI_LOADER now? I wish we could bring these
>> projects back together :-)
>>
>> >
>> > Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
>> > Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
>> > 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>
>> > ---
>> >  lib/efi_loader/efi_boottime.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> > index de57823bd44..b6966652d97 100644
>> > --- a/lib/efi_loader/efi_boottime.c
>> > +++ b/lib/efi_loader/efi_boottime.c
>> > @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
>> >                                       &number_of_children,
>> >                                       &child_handle_buffer);
>> >         if (r != EFI_SUCCESS)
>> > -               return r;
>> > +               return EFI_EXIT(r);
>
>
> Any reason we can just goto out;

+1...every other error path uses 'goto out' so the single EFI_EXIT()
at the bottom handles things - child_handle_buffer is initialised to
NULL so the free() at out is safe.

>
> Thanks
> /Ilias
>>
>> >         sole_child = (number_of_children == 1);
>> >
>> >         if (child_handle) {
>> > --
>> > 2.53.0

Regards,
Simon

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Vincent Stehlé]

On Thu, Jun 18, 2026 at 04:27:00PM +0100, Simon Glass wrote:
> On Thu, 18 Jun 2026 at 16:21, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Vincent,
> >
> >
> > On Thu, 18 Jun 2026 at 18:19, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Vincent,
> >>
> >> On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> >> >
> >> > Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
> >> > after having called EFI_ENTRY().
> >> >
> >> > This is inspired by a barebox fix. [1]
> >>
> >> Hmm, does Barebox support EFI_LOADER now? I wish we could bring these
> >> projects back together :-)
> >>
> >> >
> >> > Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
> >> > Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
> >> > 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>
> >> > ---
> >> >  lib/efi_loader/efi_boottime.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> >> > index de57823bd44..b6966652d97 100644
> >> > --- a/lib/efi_loader/efi_boottime.c
> >> > +++ b/lib/efi_loader/efi_boottime.c
> >> > @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
> >> >                                       &number_of_children,
> >> >                                       &child_handle_buffer);
> >> >         if (r != EFI_SUCCESS)
> >> > -               return r;
> >> > +               return EFI_EXIT(r);
> >
> >
> > Any reason we can just goto out;
> 
> +1...every other error path uses 'goto out' so the single EFI_EXIT()
> at the bottom handles things - child_handle_buffer is initialised to
> NULL so the free() at out is safe.

Hi,

I think you are right; I will send a v2.

Best regards,
Vincent.

> 
> >
> > Thanks
> > /Ilias
> >>
> >> >         sole_child = (number_of_children == 1);
> >> >
> >> >         if (child_handle) {
> >> > --
> >> > 2.53.0
> 
> Regards,
> Simon

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Ahmad Fatoum]

Hello,

On 6/18/26 5:18 PM, Simon Glass wrote:
> Hi Vincent,
> 
> On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:
>>
>> Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
>> after having called EFI_ENTRY().
>>
>> This is inspired by a barebox fix. [1]
> 
> Hmm, does Barebox support EFI_LOADER now? I wish we could bring these
> projects back together :-)

It's not as extensive as U-Boot's yet, but it's usable for the common
boot me a distro GRUB use case. The code originated from U-Boot with
some parts heavily rewritten to fit barebox, e.g. the EFI filesystem
protocol is implemented on top of dirfds, so it reuses VFS path handling
and other such things.

The imported files have Origin-URL comments pointing back at U-Boot
including the revision they originated from, but we are still in need of
better tooling to make use of these tags.

@Vincent, I suspect a number of fixes in [1] are also applicable to
U-Boot, but I haven't checked.

[1]:
https://lore.kernel.org/barebox/20260216084758.3548990-1-a.fatoum@pengutronix.de/

Cheers,
Ahmad



> 
>>
>> Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
>> Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
>> 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>
>> ---
>>  lib/efi_loader/efi_boottime.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index de57823bd44..b6966652d97 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
>>                                       &number_of_children,
>>                                       &child_handle_buffer);
>>         if (r != EFI_SUCCESS)
>> -               return r;
>> +               return EFI_EXIT(r);
>>         sole_child = (number_of_children == 1);
>>
>>         if (child_handle) {
>> --
>> 2.53.0
>>
> 
> Regards,
> Simon
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller

[Vincent Stehlé]

On Thu, Jun 18, 2026 at 04:18:59PM +0100, Simon Glass wrote:
> Hi Vincent,
> 
> On Thu, 18 Jun 2026 at 16:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:
> >
> > Make sure to call EFI_EXIT() in efi_disconnect_controller(), when returning
> > after having called EFI_ENTRY().
> >
> > This is inspired by a barebox fix. [1]
> 
> Hmm, does Barebox support EFI_LOADER now?

Hi Simon,

It does now: https://www.mail-archive.com/barebox@lists.infradead.org/msg56391.html

Best regards,
Vincent.

> I wish we could bring these
> projects back together :-)
> 
> >
> > Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
> > Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
> > 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>
> > ---
> >  lib/efi_loader/efi_boottime.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index de57823bd44..b6966652d97 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
> >                                       &number_of_children,
> >                                       &child_handle_buffer);
> >         if (r != EFI_SUCCESS)
> > -               return r;
> > +               return EFI_EXIT(r);
> >         sole_child = (number_of_children == 1);
> >
> >         if (child_handle) {
> > --
> > 2.53.0
> >
> 
> Regards,
> Simon

-- 
Vincent.

[PATCH v2] efi_loader: fix missing EFI_EXIT in efi_disconnect_controller

[Vincent Stehlé]

We are missing a call to EFI_EXIT() when returning from
efi_disconnect_controller(), which we need after having called EFI_ENTRY().

Fix this by jumping to the common error path, which does call EFI_EXIT().

Even though the common error path may try to free child_handle_buffer, this
cannot harm in our case as it always NULL.

This is inspired by a barebox fix. [1]

Link: https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69 [1]
Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
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>
---


Changes for v2:
- Drop the return and jump to the common error path with a goto instead


 lib/efi_loader/efi_boottime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index de57823bd44..bcb01c92cf4 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
 				      &number_of_children,
 				      &child_handle_buffer);
 	if (r != EFI_SUCCESS)
-		return r;
+		goto out;
 	sole_child = (number_of_children == 1);
 
 	if (child_handle) {
-- 
2.53.0

Re: [PATCH v2] efi_loader: fix missing EFI_EXIT in efi_disconnect_controller

[Ilias Apalodimas]

Thanks Vincent,

On Thu, 18 Jun 2026 at 19:09, Vincent Stehlé <vincent.stehle@arm.com> wrote:

> We are missing a call to EFI_EXIT() when returning from
> efi_disconnect_controller(), which we need after having called EFI_ENTRY().
>
> Fix this by jumping to the common error path, which does call EFI_EXIT().
>
> Even though the common error path may try to free child_handle_buffer, this
> cannot harm in our case as it always NULL.
>
> This is inspired by a barebox fix. [1]
>

There's more in that link and some of them are still valid. Do you plan to
send more?



> Link:
> https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69
> [1]
> Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole
> child")
> 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>
> ---
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>


>
>
> Changes for v2:
> - Drop the return and jump to the common error path with a goto instead
>
>
>  lib/efi_loader/efi_boottime.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index de57823bd44..bcb01c92cf4 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
>                                       &number_of_children,
>                                       &child_handle_buffer);
>         if (r != EFI_SUCCESS)
> -               return r;
> +               goto out;
>         sole_child = (number_of_children == 1);
>
>         if (child_handle) {
> --
> 2.53.0
>
>

Re: [PATCH v2] efi_loader: fix missing EFI_EXIT in efi_disconnect_controller

[Vincent Stehlé]

On Fri, Jun 19, 2026 at 11:22:22AM +0300, Ilias Apalodimas wrote:
> Thanks Vincent, 
> On Thu, 18 Jun 2026 at 19:09, Vincent Stehlé <[1]vincent.stehle@arm.com> wrote:
> 
> > We are missing a call to EFI_EXIT() when returning from
> > efi_disconnect_controller(), which we need after having called EFI_ENTRY().
> 
> > Fix this by jumping to the common error path, which does call EFI_EXIT().
> 
> > Even though the common error path may try to free child_handle_buffer, this
> > cannot harm in our case as it always NULL.
> 
> > This is inspired by a barebox fix. [1]
> 
> There's more in that link and some of them are still valid. Do you plan to send
> more?

Hi Ilias,

I am trying to, but... I have looked at the barebox fixes and I think they
necessitate a triaging and verification effort.

- Some fixes are not applicable to U-Boot (e.g. [1]).
- Some fixes look applicable to U-Boot but in fact are not (I tried to reproduce
  [2] on U-Boot with a unit test for example, but I could not make the test
  fail).
- Some fixes look applicable but I would like to verify completeness (I am
  looking at [3] for example).
- And last but not least, some fixes are difficult for me to review.

This makes me think that we should try to create a "reproducer" in
lib/efi_selftest/ systematically for each fix to the EFI Loader.

Best regards,
Vincent.

[1] https://git.pengutronix.de/cgit/barebox/commit/efi/loader?id=f34f05ff87cf3a0a9f7345227f9a080ddcf3ec48
[2] https://git.pengutronix.de/cgit/barebox/commit/efi/loader?id=cbe4bd04edec9c1d3c843fcc22e1a7ae5c9c4551
[3] https://git.pengutronix.de/cgit/barebox/commit/efi/loader?id=4e9944e5cd1170cb5c54f97f4e12ddeccba53e55

> 
> > Link:
> > [2]https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69
> > [1]
> > Fixes: 314bed6c854e ("efi_loader: fix DisconnectController() for sole child")
> > Signed-off-by: Vincent Stehlé <[3]vincent.stehle@arm.com>
> > Cc: Heinrich Schuchardt <[4]xypron.glpk@gmx.de>
> > Cc: Ilias Apalodimas <[5]ilias.apalodimas@linaro.org>
> > Cc: Tom Rini <[6]trini@konsulko.com>
> > ---
> 
> Reviewed-by: Ilias Apalodimas <[7]ilias.apalodimas@linaro.org>
>  
> 
> > Changes for v2:
> > - Drop the return and jump to the common error path with a goto instead
> 
> >  lib/efi_loader/efi_boottime.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index de57823bd44..bcb01c92cf4 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -3895,7 +3895,7 @@ efi_status_t EFIAPI efi_disconnect_controller(
> >                                       &number_of_children,
> >                                       &child_handle_buffer);
> >         if (r != EFI_SUCCESS)
> > -               return r;
> > +               goto out;
> >         sole_child = (number_of_children == 1);
> 
> >         if (child_handle) {
> > --
> > 2.53.0
> 
> References
> 
>    Visible links
>    1. mailto:vincent.stehle@arm.com
>    2. https://git.pengutronix.de/cgit/barebox/commit/?id=080db65e39a877b000baaf843c997a69821dfe69
>    3. mailto:vincent.stehle@arm.com
>    4. mailto:xypron.glpk@gmx.de
>    5. mailto:ilias.apalodimas@linaro.org
>    6. mailto:trini@konsulko.com
>    7. mailto:ilias.apalodimas@linaro.org

-- 
Vincent.