Reuse the parent_image handle and parent_loaded_image

- Reuse parent_image instead of allocating new ones. Firmware might cast
  EFI_LOADED_IMAGE_PROTOCOL * to a larger struct causing issues
- Remove loaded image protocol installation and uninstallation which are no
  longer required

Fixes a bug introduced by cab9c7b5a42effa8a45611fc6b8556138c869b5f.
Fixes #38567.

Co-authored-by: Tobias Heider <tobias.heider@canonical.com>

diff --git a/src/boot/linux.c b/src/boot/linux.c
index db8821176ddc301a5666ec3891f6141e100e0428..38576716cdcc82c2419e6e8472bfb71f81e32a94 100644 (file)
--- a/src/boot/linux.c
+++ b/src/boot/linux.c
@@ -22,6 +22,11 @@
 #define STUB_PAYLOAD_GUID \
         { 0x55c5d1f8, 0x04cd, 0x46b5, { 0x8a, 0x20, 0xe5, 0x6c, 0xbb, 0x30, 0x52, 0xd0 } }
 
+typedef struct {
+        MEMMAP_DEVICE_PATH memmap_path;
+        EFI_DEVICE_PATH end_path;
+} _packed_ KERNEL_FILE_PATH;
+
 typedef struct {
         const void *addr;
         size_t len;
@@ -121,7 +126,7 @@ static EFI_STATUS load_via_boot_services(
 }
 
 EFI_STATUS linux_exec(
-                EFI_HANDLE parent,
+                EFI_HANDLE parent_image,
                 const char16_t *cmdline,
                 const struct iovec *kernel,
                 const struct iovec *initrd) {
@@ -131,7 +136,7 @@ EFI_STATUS linux_exec(
         uint64_t image_base;
         EFI_STATUS err;
 
-        assert(parent);
+        assert(parent_image);
         assert(iovec_is_set(kernel));
         assert(iovec_is_valid(initrd));
 
@@ -141,7 +146,7 @@ EFI_STATUS linux_exec(
                 /* Kernel is too old to support LINUX_INITRD_MEDIA_GUID, try the deprecated EFI handover
                  * protocol. */
                 return linux_exec_efi_handover(
-                                parent,
+                                parent_image,
                                 cmdline,
                                 kernel,
                                 initrd,
@@ -150,9 +155,14 @@ EFI_STATUS linux_exec(
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Bad kernel image: %m");
 
+        /* Re-use the parent_image(_handle) and parent_loaded_image for the kernel image we are about to execute.
+         * We have to do this, because if kernel stub code passes its own handle to certain firmware functions,
+         * the firmware could cast EFI_LOADED_IMAGE_PROTOCOL * to a larger struct to access its own private data,
+         * and if we allocated a smaller struct, that could cause problems.
+         * This is modeled exactly after GRUB behaviour, which has proven to be functional. */
         EFI_LOADED_IMAGE_PROTOCOL *parent_loaded_image;
         err = BS->HandleProtocol(
-                        parent, MAKE_GUID_PTR(EFI_LOADED_IMAGE_PROTOCOL), (void **) &parent_loaded_image);
+                        parent_image, MAKE_GUID_PTR(EFI_LOADED_IMAGE_PROTOCOL), (void **) &parent_loaded_image);
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Cannot get parent loaded image: %m");
 
@@ -177,7 +187,7 @@ EFI_STATUS linux_exec(
          */
         if (secure_boot_enabled() && (shim_loader_available() || (shim_loaded() && security_override_available())))
                 return load_via_boot_services(
-                                parent,
+                                parent_image,
                                 parent_loaded_image,
                                 compat_entry_point,
                                 cmdline,
@@ -217,34 +227,32 @@ EFI_STATUS linux_exec(
                         h->VirtualSize - h->SizeOfRawData);
         }
 
-        _cleanup_free_ EFI_LOADED_IMAGE_PROTOCOL* loaded_image = xnew(EFI_LOADED_IMAGE_PROTOCOL, 1);
-
-        VENDOR_DEVICE_PATH device_node = {
-                     .Header = {
-                             .Type = MEDIA_DEVICE_PATH,
-                             .SubType = MEDIA_VENDOR_DP,
-                             .Length = sizeof(device_node),
-                     },
-                     .Guid = STUB_PAYLOAD_GUID,
+        _cleanup_free_ KERNEL_FILE_PATH *kernel_file_path = xnew(KERNEL_FILE_PATH, 1);
+
+        *kernel_file_path = (KERNEL_FILE_PATH) {
+                .memmap_path = {
+                        .Header = {
+                                .Type = HARDWARE_DEVICE_PATH,
+                                .SubType = HW_MEMMAP_DP,
+                                .Length = sizeof(MEMMAP_DEVICE_PATH),
+                        },
+                        .MemoryType = EfiLoaderData,
+                        .StartingAddress = POINTER_TO_PHYSICAL_ADDRESS(kernel->iov_base),
+                        .EndingAddress = POINTER_TO_PHYSICAL_ADDRESS(kernel->iov_base) + kernel->iov_len,
+                },
+                .end_path = {
+                        .Type = END_DEVICE_PATH_TYPE,
+                        .SubType = END_ENTIRE_DEVICE_PATH_SUBTYPE,
+                        .Length = sizeof(EFI_DEVICE_PATH),
+                },
         };
 
-        _cleanup_free_ EFI_DEVICE_PATH* file_path = device_path_replace_node(parent_loaded_image->FilePath, NULL, &device_node.Header);
-
-        *loaded_image = (EFI_LOADED_IMAGE_PROTOCOL) {
-                .Revision = 0x1000,
-                .ParentHandle = parent,
-                .SystemTable = ST,
-                .DeviceHandle = parent_loaded_image->DeviceHandle,
-                .FilePath = file_path,
-                .ImageBase = loaded_kernel,
-                .ImageSize = kernel_size_in_memory,
-                .ImageCodeType = 1 /* EFI_LOADER_CODE */,
-                .ImageDataType = 2 /* EFI_LOADER_DATA */,
-        };
+        parent_loaded_image->ImageBase = loaded_kernel;
+        parent_loaded_image->ImageSize = kernel_size_in_memory;
 
         if (cmdline) {
-                loaded_image->LoadOptions = (void *) cmdline;
-                loaded_image->LoadOptionsSize = strsize16(loaded_image->LoadOptions);
+                parent_loaded_image->LoadOptions = (void *) cmdline;
+                parent_loaded_image->LoadOptionsSize = strsize16(parent_loaded_image->LoadOptions);
         }
 
         _cleanup_(cleanup_initrd) EFI_HANDLE initrd_handle = NULL;
@@ -252,32 +260,18 @@ EFI_STATUS linux_exec(
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Error registering initrd: %m");
 
-        EFI_HANDLE kernel_image = NULL;
-
-        err = BS->InstallMultipleProtocolInterfaces(
-                        &kernel_image, MAKE_GUID_PTR(EFI_LOADED_IMAGE_PROTOCOL), loaded_image,
-                        NULL);
-        if (err != EFI_SUCCESS)
-                return log_error_status(err, "Cannot install loaded image protocol: %m");
-
         log_wait();
 
         if (entry_point > 0) {
                 EFI_IMAGE_ENTRY_POINT entry =
-                        (EFI_IMAGE_ENTRY_POINT) ((const uint8_t *) loaded_image->ImageBase + entry_point);
-                err = entry(kernel_image, ST);
+                        (EFI_IMAGE_ENTRY_POINT) ((const uint8_t *) parent_loaded_image->ImageBase + entry_point);
+                err = entry(parent_image, ST);
         } else if (compat_entry_point > 0) {
                 /* Try calling the kernel compat entry point if one exists. */
                 EFI_IMAGE_ENTRY_POINT compat_entry =
-                                (EFI_IMAGE_ENTRY_POINT) ((const uint8_t *) loaded_image->ImageBase + compat_entry_point);
-                err = compat_entry(kernel_image, ST);
+                                (EFI_IMAGE_ENTRY_POINT) ((const uint8_t *) parent_loaded_image->ImageBase + compat_entry_point);
+                err = compat_entry(parent_image, ST);
         }
 
-        EFI_STATUS uninstall_err = BS->UninstallMultipleProtocolInterfaces(
-                        kernel_image, MAKE_GUID_PTR(EFI_LOADED_IMAGE_PROTOCOL), loaded_image,
-                        NULL);
-        if (uninstall_err != EFI_SUCCESS)
-                return log_error_status(uninstall_err, "Cannot uninstall loaded image protocol: %m");
-
         return log_error_status(err, "Error starting kernel image: %m");
 }

diff --git a/src/boot/proto/device-path.h b/src/boot/proto/device-path.h
index a394b0aa6d23a256addfc19fa214fb1f1147d913..b56c217082dd87b9dae4dc305052f0d1abaaa864 100644 (file)
--- a/src/boot/proto/device-path.h
+++ b/src/boot/proto/device-path.h
@@ -25,6 +25,8 @@ enum {
         END_INSTANCE_DEVICE_PATH_SUBTYPE = 0x01,
         END_ENTIRE_DEVICE_PATH_SUBTYPE   = 0xff,
 
+        HW_MEMMAP_DP                     = 0x03,
+
         MEDIA_HARDDRIVE_DP               = 0x01,
         MEDIA_VENDOR_DP                  = 0x03,
         MEDIA_FILEPATH_DP                = 0x04,
@@ -45,6 +47,13 @@ typedef struct {
         EFI_GUID Guid;
 } _packed_ VENDOR_DEVICE_PATH;
 
+typedef struct {
+        EFI_DEVICE_PATH Header;
+        uint32_t MemoryType;
+        EFI_PHYSICAL_ADDRESS StartingAddress;
+        EFI_PHYSICAL_ADDRESS EndingAddress;
+} _packed_ MEMMAP_DEVICE_PATH;
+
 #define MBR_TYPE_PCAT                        0x01U
 #define MBR_TYPE_EFI_PARTITION_TABLE_HEADER  0x02U
 #define NO_DISK_SIGNATURE    0x00U