[PATCH v2 04/11] efi_loader: fix open_file_system() memory leak on fallback path

[Simon Glass]

Hi Heinrich,

On 2026-06-21T08:19:04, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
> efi_loader: fix open_file_system() memory leak on fallback path
>
> In dtbdump.c and smbiosdump.c, open_file_system() falls back to
> locating the UEFI system partition via locate_handle_buffer() when
> the loaded image's own partition does not expose the simple file
> system protocol. The handle buffer returned by locate_handle_buffer()
> must be freed via free_pool().
>
> The guard condition read 'if (handle)' (checking the image handle,
> which is never NULL) instead of 'if (handle_buffer)', so the buffer
> was freed unconditionally even when locate_handle_buffer() was never
> called and handle_buffer remained NULL, and it also obscured the
> intent. Change the condition to 'if (handle_buffer)'.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>
> lib/efi_loader/dtbdump.c    | 2 +-
>  lib/efi_loader/smbiosdump.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

> diff --git a/lib/efi_loader/dtbdump.c b/lib/efi_loader/dtbdump.c
> @@ -350,7 +350,7 @@ open_file_system(struct efi_simple_file_system_protocol **file_system)
>                                       EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>       if (ret != EFI_SUCCESS)
>               error(u"Failed to open simple file system protocol\r\n");
> -     if (handle)
> +     if (handle_buffer)
>               bs->free_pool(handle_buffer);

The fix is right, but the commit message overstates the bug.
locate_handle_buffer() is always called before we reach this
free_pool(), and the success-of-first-open paths return early, so
nothing leaks. The real bug is that when locate_handle_buffer() fails,
handle_buffer stays NULL and we call free_pool(NULL), which is
undefined per the UEFI spec. Please reword the subject and body along
the lines of 'avoid free_pool(NULL) on the fallback path'. The code
change itself is fine.

Regards,
Simon