Hello Heinrich,
Thanks a lot for the detailed feedback on how to address root issue. Indeed
not an obvious fix.
Regards,
Etienne
On Thu, 8 Sept 2022 at 08:03, Heinrich Schuchardt <xypron.g...@gmx.de>
wrote:
> On 9/7/22 10:20, Etienne Carriere wrote:
> > Changes efi_delete_handle() to not free EFI handles that are not related
> > to EFI objects.
> >
> > This change tries to resolved an issue seen since U-Boot v2022.07
> > in which EFI ExitBootService attempts to release some EFI handles twice.
> >
> > The issue was seen booting a EFI shell that invokes 'connect -r' and
> > then boots a Linux kernel. Execution of connect command makes EFI
> > subsystem to bind a block device for each root block devices EFI handles.
> > However these EFI device handles are already bound to a driver and we
> > can have 2 registered devices relating to the same EFI handler. On
> > ExitBootService, the loop removing the devices makes these EFI handles
> > to be released twice which corrupts memory.
> >
> > This patch prevents the memory release operation caused by the issue but
> > I don't think this patch is the right way to addresse the problem. Any
> > help will be much appreciated.
> >
> > Signed-off-by: Etienne Carriere <etienne.carri...@linaro.org>
> > ---
> > lib/efi_loader/efi_boottime.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c
> b/lib/efi_loader/efi_boottime.c
> > index 4da64b5d29..e38990ace2 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -619,9 +619,15 @@ efi_status_t efi_remove_all_protocols(const
> efi_handle_t handle)
> > */
> > void efi_delete_handle(efi_handle_t handle)
> > {
> > + efi_status_t ret;
> > +
> > if (!handle)
> > return;
>
> The patch does not solve the underlying problem but checking the
> validity of the handle makes sense anyway as we have a lot of callers.
>
> We can remove this check as we test the return value of
> efi_remove_all_protocols().
>
> > - efi_remove_all_protocols(handle);
> > +
> > + ret = efi_remove_all_protocols(handle);
> > + if (ret == EFI_INVALID_PARAMETER)
>
> We should write a message here.
>
> log_err("Can't remove invalid handle %p\n", handle);
>
> Best regards
>
> Heinrich
>
> > + return;
> > +
> > list_del(&handle->link);
> > free(handle);
> > }
>
>
