efivars: make sure that _packed_ structure members are actually aligned as expected

When looking for the terminating double-NUL, don't just read the memory
until the terminator is found, but use the information we got about the
buffer size.

The length parameter passed to utf16_to_utf8() would include the terminator, so
the converted string would end up with two terminators (the original one
converted to "utf8", still 0, and then the one that was always added anyway).
Instead let's pass just the length of the actual data to utf16_to_utf8().

1 files changed, 28 insertions, 14 deletions

diff --git a/src/shared/efivars.c b/src/shared/efivars.c
index 23d38fb484..0c7adb30fb 100644
--- a/src/shared/efivars.c
+++ b/src/shared/efivars.c
@@ -40,11 +40,17 @@
 #define END_ENTIRE_DEVICE_PATH_SUBTYPE      0xff
 #define EFI_OS_INDICATIONS_BOOT_TO_FW_UI    0x0000000000000001
 
-struct boot_option {
-        uint32_t attr;
-        uint16_t path_len;
-        uint16_t title[];
-} _packed_;
+#define boot_option__contents {                 \
+        uint32_t attr;                          \
+        uint16_t path_len;                      \
+        uint16_t title[];                       \
+        }
+
+struct boot_option boot_option__contents;
+struct boot_option__packed boot_option__contents _packed_;
+assert_cc(offsetof(struct boot_option, title) == offsetof(struct boot_option__packed, title));
+/* sizeof(struct boot_option) != sizeof(struct boot_option__packed), so
+ * the *size* of the structure should not be used anywhere below. */
 
 struct drive_path {
         uint32_t part_nr;
@@ -354,13 +360,21 @@ int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *v)
         return efi_set_variable(vendor, name, u16, (char16_strlen(u16) + 1) * sizeof(char16_t));
 }
 
-static size_t utf16_size(const uint16_t *s) {
+static ssize_t utf16_size(const uint16_t *s, size_t buf_len_bytes) {
         size_t l = 0;
 
-        while (s[l] > 0)
+        /* Returns the size of the string in bytes without the terminating two zero bytes */
+
+        if (buf_len_bytes % sizeof(uint16_t) != 0)
+                return -EINVAL;
+
+        while (l < buf_len_bytes / sizeof(uint16_t)) {
+                if (s[l] == 0)
+                        return (l + 1) * sizeof(uint16_t);
                 l++;
+        }
 
-        return (l+1) * sizeof(uint16_t);
+        return -EINVAL; /* The terminator was not found */
 }
 
 static void efi_guid_to_id128(const void *guid, sd_id128_t *id128) {
@@ -394,7 +408,7 @@ int efi_get_boot_option(
         _cleanup_free_ uint8_t *buf = NULL;
         size_t l;
         struct boot_option *header;
-        size_t title_size;
+        ssize_t title_size;
         _cleanup_free_ char *s = NULL, *p = NULL;
         sd_id128_t p_uuid = SD_ID128_NULL;
         int r;
@@ -406,13 +420,13 @@ int efi_get_boot_option(
         r = efi_get_variable(EFI_VENDOR_GLOBAL, boot_id, NULL, (void **)&buf, &l);
         if (r < 0)
                 return r;
-        if (l < sizeof(struct boot_option))
+        if (l < offsetof(struct boot_option, title))
                 return -ENOENT;
 
         header = (struct boot_option *)buf;
-        title_size = utf16_size(header->title);
-        if (title_size > l - offsetof(struct boot_option, title))
-                return -EINVAL;
+        title_size = utf16_size(header->title, l - offsetof(struct boot_option, title));
+        if (title_size < 0)
+                return title_size;
 
         if (title) {
                 s = utf16_to_utf8(header->title, title_size);
@@ -541,7 +555,7 @@ int efi_add_boot_option(
         title_len = (strlen(title)+1) * 2;
         path_len = (strlen(path)+1) * 2;
 
-        buf = malloc0(sizeof(struct boot_option) + title_len +
+        buf = malloc0(offsetof(struct boot_option, title) + title_len +
                       sizeof(struct drive_path) +
                       sizeof(struct device_path) + path_len);
         if (!buf)