Re: [PATCH] [RFC] lib: efi_loader: don't delete invalid handles

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Sep 07, 2022 at 03:10:44PM -0600, Simon Glass wrote:
> Hi Etienne,
> 
> On Wed, 7 Sept 2022 at 02:20, Etienne Carriere
> <etienne.carriere@linaro.org> 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.carriere@linaro.org>
> > ---
> >  lib/efi_loader/efi_boottime.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> +AKASHI Takahiro who has been working on resolving the mismatch
> between driver model and the EFI implementation. We should be able to
> attach EFI data structures to driver model devices, which may help
> with this issue.

The only driver which supports DRIVER_BINDING_PROTOCOL, hence
connect_controller() interface, in the current UEFI implementation
is "EFI block driver" (lib/efi_driver/efi_block_device.c).

*Ordinary* block devices, other than UCLASS_EFI_LOADER, are set up
without *binding* step. Nevertheless, "connect -r" eventually
ends up calling "bind" operation of efi_block_device against those
devices.
I guess that it is the root cause.

efi_uc_supported() should have a stricter check.

-Takahiro Akashi


> What is the next step, there?
> 
> Regards,
> Simon