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.
prev parent 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