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

[Heinrich Schuchardt]

On 9/7/22 23:10, Simon Glass wrote:
> Hi Etienne,
>
> On Wed, 7 Sept 2022 at 02:20, Etienne Carriere
> <etienne.carriere at 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 at 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.
>
> What is the next step, there?
>
> Regards,
> Simon

One of the bugs is in efi_disk_delete_raw().

The only allowable way to delete a handle is to delete all protocols
that are installed on it. But there are some caveats:

* Protocols may not be removable because they have been opened by a
driver or a child controller.
* We should only remove those protocols that we installed.

A correct DM-EFI interface implementation would do the following:

* When creating a block device create a handle.
* Install the EFI_BLOCK_IO_PROTOCOL on it.
* Use ConnectController() to install all other protocols on it. Our
implementation of the binding protocol then must open the
EFI_BLOCK_IO_PROTOCOL with EFI_OPEN_PROTOCOL_BY_DRIVER.
* When trying to remove the block device call
UninstallProtocolInterface() for the EFI_BLOCK_IO_PROTOCOL. This invokes
DisconnectController() for all drivers that have opened the protocol
with EFI_OPEN_PROTOCOL_BY_DRIVER. Only if uninstalling the protocol
interface succeeds remove the block device.

To make this all work we have to change efi_bl_bind(). We have to
differentiate here between a handle being passed in from outside
(IF_EFI_LOADER) and a handle for a U-Boot device. We should be able to
do so using the field dev in efi_object. If it is NULL, the handle is
not for a U-Boot device.

Further we need to implement the missing unbind function in the
efi_block driver.

The ConnectController() call has to be in efi_disk_probe().
The UninstallProtocolInterface() call has to be in efi_disk_remove().

Enough work for the next release cycle.

Best regards

Heinrich