efivars: remove direct access to unaligned structure members

Most of the accesses *were* aligned. The only one that definetely wasn't was to
drive_path->part_start and drive_path->part_size, because those both expect
8 byte alignment, and are at offsets 4 and 12 in the packed structure.

Because of the way that device_path structure is defined and used, we expect
that device_path.length is always two-byte aligned.

This adds asserts in various places to ensure the proper alignment, and uses
memcpy in other places where the alignment might be off.

diff --git a/src/shared/efivars.c b/src/shared/efivars.c
index 0c7adb30fb9a9226d65e3f05b2bbb73b77637c2d..26f905bfaa6c2c39405e95846b7c2e4265458a21 100644 (file)
--- a/src/shared/efivars.c
+++ b/src/shared/efivars.c
@@ -61,15 +61,19 @@ struct drive_path {
         uint8_t signature_type;
 } _packed_;
 
-struct device_path {
-        uint8_t type;
-        uint8_t sub_type;
-        uint16_t length;
-        union {
-                uint16_t path[0];
-                struct drive_path drive;
-        };
-} _packed_;
+#define device_path__contents {                 \
+        uint8_t type;                           \
+        uint8_t sub_type;                       \
+        uint16_t length;                        \
+        union {                                 \
+                uint16_t path[0];               \
+                struct drive_path drive;        \
+        };                                      \
+        }
+
+struct device_path device_path__contents;
+struct device_path__packed device_path__contents _packed_;
+assert_cc(sizeof(struct device_path) == sizeof(struct device_path__packed));
 
 bool is_efi_boot(void) {
         if (detect_container() > 0)
@@ -377,23 +381,29 @@ static ssize_t utf16_size(const uint16_t *s, size_t buf_len_bytes) {
         return -EINVAL; /* The terminator was not found */
 }
 
+struct guid {
+        uint32_t u1;
+        uint16_t u2;
+        uint16_t u3;
+        uint8_t u4[8];
+} _packed_;
+
 static void efi_guid_to_id128(const void *guid, sd_id128_t *id128) {
-        struct uuid {
-                uint32_t u1;
-                uint16_t u2;
-                uint16_t u3;
-                uint8_t u4[8];
-        } _packed_;
-        const struct uuid *uuid = guid;
-
-        id128->bytes[0] = (uuid->u1 >> 24) & 0xff;
-        id128->bytes[1] = (uuid->u1 >> 16) & 0xff;
-        id128->bytes[2] = (uuid->u1 >> 8) & 0xff;
-        id128->bytes[3] = (uuid->u1) & 0xff;
-        id128->bytes[4] = (uuid->u2 >> 8) & 0xff;
-        id128->bytes[5] = (uuid->u2) & 0xff;
-        id128->bytes[6] = (uuid->u3 >> 8) & 0xff;
-        id128->bytes[7] = (uuid->u3) & 0xff;
+        uint32_t u1;
+        uint16_t u2, u3;
+        const struct guid *uuid = guid;
+
+        memcpy(&u1, &uuid->u1, sizeof(uint32_t));
+        id128->bytes[0] = (u1 >> 24) & 0xff;
+        id128->bytes[1] = (u1 >> 16) & 0xff;
+        id128->bytes[2] = (u1 >> 8) & 0xff;
+        id128->bytes[3] = u1 & 0xff;
+        memcpy(&u2, &uuid->u2, sizeof(uint16_t));
+        id128->bytes[4] = (u2 >> 8) & 0xff;
+        id128->bytes[5] = u2 & 0xff;
+        memcpy(&u3, &uuid->u3, sizeof(uint16_t));
+        id128->bytes[6] = (u3 >> 8) & 0xff;
+        id128->bytes[7] = u3 & 0xff;
         memcpy(&id128->bytes[8], uuid->u4, sizeof(uuid->u4));
 }
 
@@ -508,20 +518,14 @@ static void to_utf16(uint16_t *dest, const char *src) {
         dest[i] = '\0';
 }
 
-struct guid {
-        uint32_t u1;
-        uint16_t u2;
-        uint16_t u3;
-        uint8_t u4[8];
-} _packed_;
-
 static void id128_to_efi_guid(sd_id128_t id, void *guid) {
-        struct guid *uuid = guid;
-
-        uuid->u1 = id.bytes[0] << 24 | id.bytes[1] << 16 | id.bytes[2] << 8 | id.bytes[3];
-        uuid->u2 = id.bytes[4] << 8 | id.bytes[5];
-        uuid->u3 = id.bytes[6] << 8 | id.bytes[7];
-        memcpy(uuid->u4, id.bytes+8, sizeof(uuid->u4));
+        struct guid uuid = {
+                .u1 = id.bytes[0] << 24 | id.bytes[1] << 16 | id.bytes[2] << 8 | id.bytes[3],
+                .u2 = id.bytes[4] << 8 | id.bytes[5],
+                .u3 = id.bytes[6] << 8 | id.bytes[7],
+        };
+        memcpy(uuid.u4, id.bytes+8, sizeof(uuid.u4));
+        memcpy(guid, &uuid, sizeof(uuid));
 }
 
 static uint16_t *tilt_slashes(uint16_t *s) {
@@ -575,12 +579,12 @@ int efi_add_boot_option(
         devicep->type = MEDIA_DEVICE_PATH;
         devicep->sub_type = MEDIA_HARDDRIVE_DP;
         devicep->length = offsetof(struct device_path, drive) + sizeof(struct drive_path);
-        devicep->drive.part_nr = part;
-        devicep->drive.part_start = pstart;
-        devicep->drive.part_size = psize;
-        devicep->drive.signature_type = SIGNATURE_TYPE_GUID;
-        devicep->drive.mbr_type = MBR_TYPE_EFI_PARTITION_TABLE_HEADER;
+        memcpy(&devicep->drive.part_nr, &part, sizeof(uint32_t));
+        memcpy(&devicep->drive.part_start, &pstart, sizeof(uint64_t));
+        memcpy(&devicep->drive.part_size, &psize, sizeof(uint64_t));
         id128_to_efi_guid(part_uuid, devicep->drive.signature);
+        devicep->drive.mbr_type = MBR_TYPE_EFI_PARTITION_TABLE_HEADER;
+        devicep->drive.signature_type = SIGNATURE_TYPE_GUID;
         size += devicep->length;
 
         /* path to loader */