[PATCH v2] efi: Handle NULL return value when getting loaded image protocol

[PATCH v2] efi: Handle NULL return value when getting loaded image protocol

[Ard Biesheuvel]

The EFI spec mandates that the handle produced by the LoadImage boot
service has a LoadedImage protocol instance installed on it, but for
robustness, we should still deal with a NULL return value from the
helper routine that obtains this protocol pointer.

If this happens, don't try to start the image but unload it and return
an error.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 grub-core/loader/efi/linux.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
index 90ad1a7b82a76066..8211f7892ad391f1 100644
--- a/grub-core/loader/efi/linux.c
+++ b/grub-core/loader/efi/linux.c
@@ -214,6 +214,11 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, grub_size_t size, char *args)
 
   /* Convert command line to UCS-2 */
   loaded_image = grub_efi_get_loaded_image (image_handle);
+  if (loaded_image == NULL)
+    {
+      grub_error (GRUB_ERR_BAD_FIRMWARE, "missing loaded_image proto");
+      goto unload;
+    }
   loaded_image->load_options_size = len =
     (grub_strlen (args) + 1) * sizeof (grub_efi_char16_t);
   loaded_image->load_options =
@@ -229,9 +234,11 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, grub_size_t size, char *args)
   status = b->start_image (image_handle, 0, NULL);
 
   /* When successful, not reached */
-  b->unload_image (image_handle);
+  grub_error (GRUB_ERR_BAD_OS, "start_image() returned %" PRIuGRUB_EFI_UINTN_T, status);
   grub_efi_free_pages ((grub_addr_t) loaded_image->load_options,
 		       GRUB_EFI_BYTES_TO_PAGES (loaded_image->load_options_size));
+unload:
+  b->unload_image (image_handle);
 
   return grub_errno;
 }
-- 
2.39.2

Re: [PATCH v2] efi: Handle NULL return value when getting loaded image protocol

[Daniel Kiper]

On Wed, May 24, 2023 at 07:29:09PM +0200, Ard Biesheuvel wrote:
> The EFI spec mandates that the handle produced by the LoadImage boot
> service has a LoadedImage protocol instance installed on it, but for
> robustness, we should still deal with a NULL return value from the
> helper routine that obtains this protocol pointer.
>
> If this happens, don't try to start the image but unload it and return
> an error.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel