hi Heinrich

On Thu, 8 Sept 2022 at 09:54, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

On 9/8/22 08:46, Ilias Apalodimas wrote:

...

Hi all,

On Thu, 8 Sept 2022 at 09:18, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

...

From: Etienne Carriere etienne.carriere@linaro.org

From: Etienne Carriere etienne.carriere@linaro.org

Change efi_delete_handle() to not free EFI handles twice.

This change tries to resolved an issue seen since U-Boot v2022.07 in which 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 but does not resolve the underlying problem.

Signed-off-by: Etienne Carriere etienne.carriere@linaro.org

Add log message. Reviewed-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

v2: add log message remove superfluous NULL test tweak commit message

---

lib/efi_loader/efi_boottime.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4da64b5d29..6f7333638a 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -619,9 +619,14 @@ efi_status_t efi_remove_all_protocols(const efi_handle_t handle) */ void efi_delete_handle(efi_handle_t handle) {

•   if (!handle)
  

•   efi_status_t ret;
  

•   ret = efi_remove_all_protocols(handle);
  

•   if (ret == EFI_INVALID_PARAMETER) {
  

•           log_err("Can't remove invalid handle %p\n", handle);
             return;
  

•   efi_remove_all_protocols(handle);
  

•   }
  

•    list_del(&handle->link);
     free(handle);
  

Can't we just set the handle NULL here?

If you set handle=NULL in efi_delete_handle(), this has no effect as the caller passes in handle and not *handle.

Yea you are right

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Best regards

Heinrich