U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Vincent Stehlé" <vincent.stehle@arm.com>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: u-boot@lists.denx.de, Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Simon Glass <sjg@chromium.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Tom Rini <trini@konsulko.com>
Subject: Re: [PATCH v2] efi_loader: fix missing EFI_EXIT in efi_disconnect_controller
Date: Mon, 22 Jun 2026 10:43:31 +0200	[thread overview]
Message-ID: <ajj1s_tAXNtVO_GM@debian> (raw)
In-Reply-To: <CAC_iWjL+A5qcrJPEiR-f3__P_3z5DQApJWVANS+DNUga-vB2Fw@mail.gmail.com>

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.

      reply	other threads:[~2026-06-22  8:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18 15:09 [PATCH] efi_loader: add missing EFI_EXIT in efi_disconnect_controller Vincent Stehlé
2026-06-18 15:18 ` Simon Glass
2026-06-18 15:20   ` Ilias Apalodimas
2026-06-18 15:27     ` Simon Glass
2026-06-18 15:48       ` Vincent Stehlé
2026-06-18 15:28   ` Ahmad Fatoum
2026-06-18 15:33   ` Vincent Stehlé
2026-06-18 16:09 ` [PATCH v2] efi_loader: fix " Vincent Stehlé
2026-06-19  8:22   ` Ilias Apalodimas
2026-06-22  8:43     ` Vincent Stehlé [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajj1s_tAXNtVO_GM@debian \
    --to=vincent.stehle@arm.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=ilias.apalodimas@linaro.org \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox