Hi Heinrich, On 2026-06-21T08:19:04, Heinrich Schuchardt <[email protected]> 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 <[email protected]> > Reviewed-by: Ilias Apalodimas <[email protected]> > > 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
