Merge pull request #11578 from keszybz/gcc-9-fixes

Packed struct alignment workarounds for gcc-9

9 files changed, 214 insertions, 143 deletions

diff --git a/TODO b/TODO
index 77a8447233..462db57a3a 100644
--- a/TODO
+++ b/TODO
@@ -1114,3 +1114,5 @@ Regularly:
 * use secure_getenv() instead of getenv() where appropriate
 
 * link up selected blog stories from man pages and unit files Documentation= fields
+String is not UTF-8 clean, ignoring assignment
+ timedatex.service: Consumed 26ms CPU time.
diff --git a/src/journal/catalog.c b/src/journal/catalog.c
index 3556a101bf..4062f12c2d 100644
--- a/src/journal/catalog.c
+++ b/src/journal/catalog.c
@@ -46,7 +46,8 @@ typedef struct CatalogHeader {
 
 typedef struct CatalogItem {
         sd_id128_t id;
-        char language[32];
+        char language[32]; /* One byte is used for termination, so the maximum allowed
+                            * length of the string is actually 31 bytes. */
         le64_t offset;
 } CatalogItem;
 
@@ -556,25 +557,44 @@ static const char *find_id(void *p, sd_id128_t id) {
         const char *loc;
 
         loc = setlocale(LC_MESSAGES, NULL);
-        if (loc && loc[0] && !streq(loc, "C") && !streq(loc, "POSIX")) {
-                strncpy(key.language, loc, sizeof(key.language));
-                key.language[strcspn(key.language, ".@")] = 0;
-
-                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
-                if (!f) {
-                        char *e;
-
-                        e = strchr(key.language, '_');
-                        if (e) {
-                                *e = 0;
-                                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
+        if (!isempty(loc) && !STR_IN_SET(loc, "C", "POSIX")) {
+                size_t len;
+
+                len = strcspn(loc, ".@");
+                if (len > sizeof(key.language) - 1)
+                        log_debug("LC_MESSAGES value too long, ignoring: \"%.*s\"", (int) len, loc);
+                else {
+                        strncpy(key.language, loc, len);
+                        key.language[len] = '\0';
+
+                        f = bsearch(&key,
+                                    (const uint8_t*) p + le64toh(h->header_size),
+                                    le64toh(h->n_items),
+                                    le64toh(h->catalog_item_size),
+                                    (comparison_fn_t) catalog_compare_func);
+                        if (!f) {
+                                char *e;
+
+                                e = strchr(key.language, '_');
+                                if (e) {
+                                        *e = 0;
+                                        f = bsearch(&key,
+                                                    (const uint8_t*) p + le64toh(h->header_size),
+                                                    le64toh(h->n_items),
+                                                    le64toh(h->catalog_item_size),
+                                                    (comparison_fn_t) catalog_compare_func);
+                                }
                         }
                 }
         }
 
         if (!f) {
                 zero(key.language);
-                f = bsearch(&key, (const uint8_t*) p + le64toh(h->header_size), le64toh(h->n_items), le64toh(h->catalog_item_size), (comparison_fn_t) catalog_compare_func);
+                f = bsearch(&key,
+                            (const uint8_t*) p + le64toh(h->header_size),
+                            le64toh(h->n_items),
+                            le64toh(h->catalog_item_size),
+                            (comparison_fn_t) catalog_compare_func);
         }
 
         if (!f)
diff --git a/src/journal/journal-def.h b/src/journal/journal-def.h
index e48260206f..d68ce3894b 100644
--- a/src/journal/journal-def.h
+++ b/src/journal/journal-def.h
@@ -59,16 +59,20 @@ struct ObjectHeader {
         uint8_t payload[];
 } _packed_;
 
-struct DataObject {
-        ObjectHeader object;
-        le64_t hash;
-        le64_t next_hash_offset;
-        le64_t next_field_offset;
-        le64_t entry_offset; /* the first array entry we store inline */
-        le64_t entry_array_offset;
-        le64_t n_entries;
-        uint8_t payload[];
-} _packed_;
+#define DataObject__contents {                                          \
+        ObjectHeader object;                                            \
+        le64_t hash;                                                    \
+        le64_t next_hash_offset;                                        \
+        le64_t next_field_offset;                                       \
+        le64_t entry_offset; /* the first array entry we store inline */ \
+        le64_t entry_array_offset;                                      \
+        le64_t n_entries;                                               \
+        uint8_t payload[];                                              \
+        }
+
+struct DataObject DataObject__contents;
+struct DataObject__packed DataObject__contents _packed_;
+assert_cc(sizeof(struct DataObject) == sizeof(struct DataObject__packed));
 
 struct FieldObject {
         ObjectHeader object;
@@ -83,15 +87,20 @@ struct EntryItem {
         le64_t hash;
 } _packed_;
 
-struct EntryObject {
-        ObjectHeader object;
-        le64_t seqnum;
-        le64_t realtime;
-        le64_t monotonic;
-        sd_id128_t boot_id;
-        le64_t xor_hash;
-        EntryItem items[];
-} _packed_;
+#define EntryObject__contents { \
+        ObjectHeader object;    \
+        le64_t seqnum;          \
+        le64_t realtime;        \
+        le64_t monotonic;       \
+        sd_id128_t boot_id;     \
+        le64_t xor_hash;        \
+        EntryItem items[];      \
+        }
+
+struct EntryObject EntryObject__contents;
+struct EntryObject__packed EntryObject__contents _packed_;
+assert_cc(sizeof(struct EntryObject) == sizeof(struct EntryObject__packed));
+
 
 struct HashItem {
         le64_t head_hash_offset;
@@ -166,40 +175,43 @@ enum {
 
 #define HEADER_SIGNATURE ((char[]) { 'L', 'P', 'K', 'S', 'H', 'H', 'R', 'H' })
 
-struct Header {
-        uint8_t signature[8]; /* "LPKSHHRH" */
-        le32_t compatible_flags;
-        le32_t incompatible_flags;
-        uint8_t state;
-        uint8_t reserved[7];
-        sd_id128_t file_id;
-        sd_id128_t machine_id;
-        sd_id128_t boot_id;    /* last writer */
-        sd_id128_t seqnum_id;
-        le64_t header_size;
-        le64_t arena_size;
-        le64_t data_hash_table_offset;
-        le64_t data_hash_table_size;
-        le64_t field_hash_table_offset;
-        le64_t field_hash_table_size;
-        le64_t tail_object_offset;
-        le64_t n_objects;
-        le64_t n_entries;
-        le64_t tail_entry_seqnum;
-        le64_t head_entry_seqnum;
-        le64_t entry_array_offset;
-        le64_t head_entry_realtime;
-        le64_t tail_entry_realtime;
-        le64_t tail_entry_monotonic;
-        /* Added in 187 */
-        le64_t n_data;
-        le64_t n_fields;
-        /* Added in 189 */
-        le64_t n_tags;
-        le64_t n_entry_arrays;
-
-        /* Size: 240 */
-} _packed_;
+#define struct_Header__contents {                       \
+        uint8_t signature[8]; /* "LPKSHHRH" */          \
+        le32_t compatible_flags;                        \
+        le32_t incompatible_flags;                      \
+        uint8_t state;                                  \
+        uint8_t reserved[7];                            \
+        sd_id128_t file_id;                             \
+        sd_id128_t machine_id;                          \
+        sd_id128_t boot_id;    /* last writer */        \
+        sd_id128_t seqnum_id;                           \
+        le64_t header_size;                             \
+        le64_t arena_size;                              \
+        le64_t data_hash_table_offset;                  \
+        le64_t data_hash_table_size;                    \
+        le64_t field_hash_table_offset;                 \
+        le64_t field_hash_table_size;                   \
+        le64_t tail_object_offset;                      \
+        le64_t n_objects;                               \
+        le64_t n_entries;                               \
+        le64_t tail_entry_seqnum;                       \
+        le64_t head_entry_seqnum;                       \
+        le64_t entry_array_offset;                      \
+        le64_t head_entry_realtime;                     \
+        le64_t tail_entry_realtime;                     \
+        le64_t tail_entry_monotonic;                    \
+        /* Added in 187 */                              \
+        le64_t n_data;                                  \
+        le64_t n_fields;                                \
+        /* Added in 189 */                              \
+        le64_t n_tags;                                  \
+        le64_t n_entry_arrays;                          \
+        }
+
+struct Header struct_Header__contents;
+struct Header__packed struct_Header__contents _packed_;
+assert_cc(sizeof(struct Header) == sizeof(struct Header__packed));
+assert_cc(sizeof(struct Header) == 240);
 
 #define FSS_HEADER_SIGNATURE ((char[]) { 'K', 'S', 'H', 'H', 'R', 'H', 'L', 'P' })
 
diff --git a/src/libsystemd-network/radv-internal.h b/src/libsystemd-network/radv-internal.h
index cd44352307..66f49ed44e 100644
--- a/src/libsystemd-network/radv-internal.h
+++ b/src/libsystemd-network/radv-internal.h
@@ -63,19 +63,34 @@ struct sd_radv {
         struct sd_radv_opt_dns *dnssl;
 };
 
+#define radv_prefix_opt__contents {             \
+        uint8_t type;                           \
+        uint8_t length;                         \
+        uint8_t prefixlen;                      \
+        uint8_t flags;                          \
+        be32_t valid_lifetime;                  \
+        be32_t preferred_lifetime;              \
+        uint32_t reserved;                      \
+        struct in6_addr in6_addr;               \
+}
+
+struct radv_prefix_opt radv_prefix_opt__contents;
+
+/* We need the opt substructure to be packed, because we use it in send(). But
+ * if we use _packed_, this means that the structure cannot be used directly in
+ * normal code in general, because the fields might not be properly aligned.
+ * But in this particular case, the structure is defined in a way that gives
+ * proper alignment, even without the explicit _packed_ attribute. To appease
+ * the compiler we use the "unpacked" structure, but we also verify that
+ * structure contains no holes, so offsets are the same when _packed_ is used.
+ */
+struct radv_prefix_opt__packed radv_prefix_opt__contents _packed_;
+assert_cc(sizeof(struct radv_prefix_opt) == sizeof(struct radv_prefix_opt__packed));
+
 struct sd_radv_prefix {
         unsigned n_ref;
 
-        struct {
-                uint8_t type;
-                uint8_t length;
-                uint8_t prefixlen;
-                uint8_t flags;
-                be32_t valid_lifetime;
-                be32_t preferred_lifetime;
-                uint32_t reserved;
-                struct in6_addr in6_addr;
-        } _packed_ opt;
+        struct radv_prefix_opt opt;
 
         LIST_FIELDS(struct sd_radv_prefix, prefix);
 
diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index 3a46faf769..d340487025 100644
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -1178,7 +1178,6 @@ int dissected_image_decrypt_interactively(
 
 #if HAVE_LIBCRYPTSETUP
 static int deferred_remove(DecryptedPartition *p) {
-
         struct dm_ioctl dm = {
                 .version = {
                         DM_VERSION_MAJOR,
@@ -1199,6 +1198,9 @@ static int deferred_remove(DecryptedPartition *p) {
         if (fd < 0)
                 return -errno;
 
+        if (strlen(p->name) > sizeof(dm.name))
+                return -ENAMETOOLONG;
+
         strncpy(dm.name, p->name, sizeof(dm.name));
 
         if (ioctl(fd, DM_DEV_REMOVE, &dm))
diff --git a/src/shared/efivars.c b/src/shared/efivars.c
index 23d38fb484..26f905bfaa 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;
@@ -55,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)
@@ -354,32 +364,46 @@ 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 */
 }
 
+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));
 }
 
@@ -394,7 +418,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 +430,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);
@@ -494,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) {
@@ -541,7 +559,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)
@@ -561,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 */
diff --git a/src/test/meson.build b/src/test/meson.build
index d9d87e02c5..08026ea8c4 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -929,7 +929,8 @@ tests += [
 
         [['src/libsystemd/sd-resolve/test-resolve.c'],
          [],
-         [threads]],
+         [threads],
+         '', 'timeout=120'],
 
         [['src/libsystemd/sd-login/test-login.c'],
          [],
diff --git a/src/test/test-sizeof.c b/src/test/test-sizeof.c
index 7a1e496ed2..35b087653e 100644
--- a/src/test/test-sizeof.c
+++ b/src/test/test-sizeof.c
@@ -13,11 +13,12 @@
 
 #pragma GCC diagnostic ignored "-Wtype-limits"
 
-#define info(t)                                                 \
-        printf("%s → %zu bits%s\n", STRINGIFY(t),               \
-               sizeof(t)*CHAR_BIT,                              \
-               strstr(STRINGIFY(t), "signed") ? "" :            \
-               ((t)-1 < (t)0 ? ", signed" : ", unsigned"));
+#define info(t)                                                         \
+        printf("%s → %zu bits%s, %zu byte alignment\n", STRINGIFY(t),   \
+               sizeof(t)*CHAR_BIT,                                      \
+               strstr(STRINGIFY(t), "signed") ? "" :                    \
+               (t)-1 < (t)0 ? ", signed" : ", unsigned",                \
+               __alignof__(t))
 
 enum Enum {
         enum_value,
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 3c1b5f9b41..40c1f4a3aa 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -139,11 +139,11 @@ static void test_container_of(void) {
                 uint64_t v1;
                 uint8_t pad2[2];
                 uint32_t v2;
-        } _packed_ myval = { };
+        } myval = { };
 
         log_info("/* %s */", __func__);
 
-        assert_cc(sizeof(myval) == 17);
+        assert_cc(sizeof(myval) >= 17);
         assert_se(container_of(&myval.v1, struct mytype, v1) == &myval);
         assert_se(container_of(&myval.v2, struct mytype, v2) == &myval);
         assert_se(container_of(&container_of(&myval.v2,