boot: Fix device path unaligned access

diff --git a/src/boot/efi/disk.c b/src/boot/efi/disk.c
index 5f889a365cad2a10cb1bf604fa62ca394dc20703..524662603c6659d5031f495397453b0f28b8bbdf 100644 (file)
--- a/src/boot/efi/disk.c
+++ b/src/boot/efi/disk.c
@@ -25,15 +25,14 @@ EFI_STATUS disk_get_part_uuid(EFI_HANDLE *handle, char16_t uuid[static 37]) {
                 if (DevicePathSubType(dp) != MEDIA_HARDDRIVE_DP)
                         continue;
 
-                HARDDRIVE_DEVICE_PATH *hd = (HARDDRIVE_DEVICE_PATH *) dp;
-                if (hd->SignatureType != SIGNATURE_TYPE_GUID)
-                        continue;
+                /* The HD device path may be misaligned. */
+                HARDDRIVE_DEVICE_PATH hd;
+                memcpy(&hd, dp, MIN(sizeof(hd), (size_t) DevicePathNodeLength(dp)));
 
-                /* Use memcpy in case the device path node is misaligned. */
-                EFI_GUID sig;
-                memcpy(&sig, hd->Signature, sizeof(hd->Signature));
+                if (hd.SignatureType != SIGNATURE_TYPE_GUID)
+                        continue;
 
-                GuidToString(uuid, &sig);
+                GuidToString(uuid, (EFI_GUID *) &hd.Signature);
                 return EFI_SUCCESS;
         }
 

diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c
index ce1f981817f448b7b1a983f32a27e8518eaa5962..e17a8e18a4cfcfe8828527f8f029149313f67963 100644 (file)
--- a/src/boot/efi/util.c
+++ b/src/boot/efi/util.c
@@ -757,12 +757,12 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE
         *ret_dp = xmalloc(dp_size + file_size + SIZE_OF_FILEPATH_DEVICE_PATH + END_DEVICE_PATH_LENGTH);
         dp = mempcpy(*ret_dp, dp, dp_size);
 
-        /* Replace end node with file media device path. */
-        FILEPATH_DEVICE_PATH *file_dp = (FILEPATH_DEVICE_PATH *) dp;
-        file_dp->Header.Type = MEDIA_DEVICE_PATH;
-        file_dp->Header.SubType = MEDIA_FILEPATH_DP;
-        memcpy(&file_dp->PathName, file, file_size);
-        SetDevicePathNodeLength(&file_dp->Header, SIZE_OF_FILEPATH_DEVICE_PATH + file_size);
+        /* Replace end node with file media device path. Use memcpy() in case dp is unaligned (if accessed as
+         * FILEPATH_DEVICE_PATH). */
+        dp->Type = MEDIA_DEVICE_PATH;
+        dp->SubType = MEDIA_FILEPATH_DP;
+        memcpy((uint8_t *) dp + offsetof(FILEPATH_DEVICE_PATH, PathName), file, file_size);
+        SetDevicePathNodeLength(dp, offsetof(FILEPATH_DEVICE_PATH, PathName) + file_size);
 
         dp = NextDevicePathNode(dp);
         SetDevicePathEndNode(dp);

diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c
index 3ccf9fa8b1cb2af806ffc2811f01807e81ffe391..2433816b4ef973ca5aa95a52476a0737c4ef270f 100644 (file)
--- a/src/boot/efi/xbootldr.c
+++ b/src/boot/efi/xbootldr.c
@@ -12,33 +12,29 @@ union GptHeaderBuffer {
         uint8_t space[CONST_ALIGN_TO(sizeof(EFI_PARTITION_TABLE_HEADER), 512)];
 };
 
-static EFI_DEVICE_PATH *path_chop(EFI_DEVICE_PATH *path, EFI_DEVICE_PATH *node) {
+static EFI_DEVICE_PATH *path_replace_hd(
+                const EFI_DEVICE_PATH *path,
+                const EFI_DEVICE_PATH *node,
+                const HARDDRIVE_DEVICE_PATH *new_node) {
+
+        /* Create a new device path as a copy of path, while chopping off the remainder starting at the given
+         * node. If new_node is provided, it is appended at the end of the new path. */
+
         assert(path);
         assert(node);
 
-        UINTN len = (uint8_t *) node - (uint8_t *) path;
-        EFI_DEVICE_PATH *chopped = xmalloc(len + END_DEVICE_PATH_LENGTH);
+        size_t len = (uint8_t *) node - (uint8_t *) path, new_node_len = 0;
+        if (new_node)
+                new_node_len = DevicePathNodeLength(&new_node->Header);
 
-        memcpy(chopped, path, len);
-        SetDevicePathEndNode((EFI_DEVICE_PATH *) ((uint8_t *) chopped + len));
-
-        return chopped;
-}
+        EFI_DEVICE_PATH *ret = xmalloc(len + new_node_len + END_DEVICE_PATH_LENGTH);
+        EFI_DEVICE_PATH *end = mempcpy(ret, path, len);
 
-static EFI_DEVICE_PATH *path_dup(const EFI_DEVICE_PATH *dp) {
-        assert(dp);
+        if (new_node)
+                end = mempcpy(end, new_node, new_node_len);
 
-        const EFI_DEVICE_PATH *node = dp;
-        size_t size = 0;
-        while (!IsDevicePathEnd(node)) {
-                size += DevicePathNodeLength(node);
-                node = NextDevicePathNode(node);
-        }
-        size += DevicePathNodeLength(node);
-
-        EFI_DEVICE_PATH *dup = xmalloc(size);
-        memcpy(dup, dp, size);
-        return dup;
+        SetDevicePathEndNode(end);
+        return ret;
 }
 
 static bool verify_gpt(union GptHeaderBuffer *gpt_header_buffer, EFI_LBA lba_expected) {
@@ -149,13 +145,24 @@ static EFI_STATUS try_gpt(
                 if (end < start) /* Bogus? */
                         continue;
 
-                ret_hd->PartitionNumber = i + 1;
-                ret_hd->PartitionStart = start;
-                ret_hd->PartitionSize = end - start + 1;
-                ret_hd->MBRType = MBR_TYPE_EFI_PARTITION_TABLE_HEADER;
-                ret_hd->SignatureType = SIGNATURE_TYPE_GUID;
+                *ret_hd = (HARDDRIVE_DEVICE_PATH) {
+                        .Header = {
+                                .Type = MEDIA_DEVICE_PATH,
+                                .SubType = MEDIA_HARDDRIVE_DP,
+                        },
+                        .PartitionNumber = i + 1,
+                        .PartitionStart = start,
+                        .PartitionSize = end - start + 1,
+                        .MBRType = MBR_TYPE_EFI_PARTITION_TABLE_HEADER,
+                        .SignatureType = SIGNATURE_TYPE_GUID,
+                };
                 memcpy(ret_hd->Signature, &entry->UniquePartitionGUID, sizeof(ret_hd->Signature));
 
+                /* HARDDRIVE_DEVICE_PATH has padding, which at least OVMF does not like. */
+                SetDevicePathNodeLength(
+                                &ret_hd->Header,
+                                offsetof(HARDDRIVE_DEVICE_PATH, SignatureType) + sizeof(ret_hd->SignatureType));
+
                 return EFI_SUCCESS;
         }
 
@@ -192,7 +199,7 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p
 
         /* Chop off the partition part, leaving us with the full path to the disk itself. */
         _cleanup_free_ EFI_DEVICE_PATH *disk_path = NULL;
-        EFI_DEVICE_PATH *p = disk_path = path_chop(partition_path, part_node);
+        EFI_DEVICE_PATH *p = disk_path = path_replace_hd(partition_path, part_node, NULL);
 
         EFI_HANDLE disk_handle;
         EFI_BLOCK_IO_PROTOCOL *block_io;
@@ -214,7 +221,6 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p
 
         /* Try several copies of the GPT header, in case one is corrupted */
         EFI_LBA backup_lba = 0;
-        HARDDRIVE_DEVICE_PATH hd = *((HARDDRIVE_DEVICE_PATH *) part_node);
         for (UINTN nr = 0; nr < 3; nr++) {
                 EFI_LBA lba;
 
@@ -230,6 +236,7 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p
                 else
                         continue;
 
+                HARDDRIVE_DEVICE_PATH hd;
                 err = try_gpt(
                         block_io, lba,
                         nr == 0 ? &backup_lba : NULL, /* Only get backup LBA location from first GPT header. */
@@ -243,9 +250,7 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p
                 }
 
                 /* Patch in the data we found */
-                EFI_DEVICE_PATH *xboot_path = path_dup(partition_path);
-                memcpy((uint8_t *) xboot_path + ((uint8_t *) part_node - (uint8_t *) partition_path), &hd, sizeof(hd));
-                *ret_device_path = xboot_path;
+                *ret_device_path = path_replace_hd(partition_path, part_node, &hd);
                 return EFI_SUCCESS;
         }