From 462d287bc9edc395d65503ee0ce24277304ef932 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 May 2025 02:25:36 +0900 Subject: [PATCH 1/6] sd-journal: variable sized array cannot be used in ObjectHeader The struct ObjectHeader is embedded in real object structs. Hence, the existence of the flex array in ObjectHeader violates the requirement that flex array must be at the end of struct. --- src/libsystemd/sd-journal/journal-def.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/journal-def.h b/src/libsystemd/sd-journal/journal-def.h index f3fef931d0..8cd0843197 100644 --- a/src/libsystemd/sd-journal/journal-def.h +++ b/src/libsystemd/sd-journal/journal-def.h @@ -55,7 +55,8 @@ struct ObjectHeader { uint8_t flags; uint8_t reserved[6]; le64_t size; - uint8_t payload[]; + uint8_t payload[0]; /* The struct is embedded in other objects, hence flex array (i.e. payload[]) + * cannot be used. */ } _packed_; #define DataObject__contents { \ From 545814714ce7872c537e9b8dbc7d60d33d68055b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Jun 2025 12:35:39 +0900 Subject: [PATCH 2/6] sd-journal: replace sizeof(ObjectHeader) with offsetof(ObjectHeader, payload) Note, the preceding fields have size that is a multiple of 8 bytes, so the end of `.size` is aligned, thus `offsetof(ObjectHeader, payload) == sizeof(ObjectHeader)`. --- src/libsystemd/sd-journal/journal-file.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index ec06ea8e20..2a1acefc3b 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -595,7 +595,7 @@ static int journal_file_verify_header(JournalFile *f) { return -ENODATA; if (header_size + arena_size < tail_object_offset) return -ENODATA; - if (header_size + arena_size - tail_object_offset < sizeof(ObjectHeader)) + if (header_size + arena_size - tail_object_offset < offsetof(ObjectHeader, payload)) return -ENODATA; if (!hash_table_is_valid(le64toh(f->header->data_hash_table_offset), @@ -661,7 +661,7 @@ static int journal_file_verify_header(JournalFile *f) { /* Verify number of objects */ uint64_t n_objects = le64toh(f->header->n_objects); - if (n_objects > arena_size / sizeof(ObjectHeader)) + if (n_objects > arena_size / offsetof(ObjectHeader, payload)) return -ENODATA; uint64_t n_entries = le64toh(f->header->n_entries); @@ -883,7 +883,7 @@ static uint64_t minimum_header_size(JournalFile *f, Object *o) { return journal_file_data_payload_offset(f); if (o->object.type >= ELEMENTSOF(table) || table[o->object.type] <= 0) - return sizeof(ObjectHeader); + return offsetof(ObjectHeader, payload); return table[o->object.type]; } @@ -900,7 +900,7 @@ static int check_object_header(JournalFile *f, Object *o, ObjectType type, uint6 "Attempt to move to uninitialized object: %" PRIu64, offset); - if (s < sizeof(ObjectHeader)) + if (s < offsetof(ObjectHeader, payload)) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Attempt to move to overly short object with size %"PRIu64": %" PRIu64, s, offset); @@ -1106,7 +1106,7 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset journal_object_type_to_string(type), offset); - r = journal_file_move_to(f, type, false, offset, sizeof(ObjectHeader), (void**) &o); + r = journal_file_move_to(f, type, false, offset, offsetof(ObjectHeader, payload), (void**) &o); if (r < 0) return r; @@ -1238,7 +1238,7 @@ int journal_file_append_object( assert(f); assert(f->header); assert(type > OBJECT_UNUSED && type < _OBJECT_TYPE_MAX); - assert(size >= sizeof(ObjectHeader)); + assert(size >= offsetof(ObjectHeader, payload)); r = journal_file_set_online(f); if (r < 0) From ddfe2afbd3b4ceb86e30a13296dd097f15988657 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 May 2025 03:44:18 +0900 Subject: [PATCH 3/6] sd-journal: drop unnecessary use of dummy_t Then, make flex arrays to zero-length arrays, as union cannot have flex arrays, even though flex and zero-length arrays are effectively equivalent. --- src/libsystemd/sd-journal/journal-def.h | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-def.h b/src/libsystemd/sd-journal/journal-def.h index 8cd0843197..96c193c3fd 100644 --- a/src/libsystemd/sd-journal/journal-def.h +++ b/src/libsystemd/sd-journal/journal-def.h @@ -104,18 +104,13 @@ assert_cc(sizeof(struct FieldObject) == sizeof(struct FieldObject__packed)); le64_t xor_hash; \ union { \ struct { \ - dummy_t __empty__regular; \ - struct { \ - le64_t object_offset; \ - le64_t hash; \ - } regular[]; \ - }; \ + le64_t object_offset; \ + le64_t hash; \ + } regular[0]; /* this is an array; since we are not allowed to place a variable sized array + * in a union, we just zero-size it, even if it is generally longer. */ \ struct { \ - dummy_t __empty_compact; \ - struct { \ - le32_t object_offset; \ - } compact[]; \ - }; \ + le32_t object_offset; \ + } compact[0]; \ } items; \ } From dbef4dd4f23517abfc73b35f0bdf004d2f8f4805 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 May 2025 02:45:01 +0900 Subject: [PATCH 4/6] sd-event: move flex array in struct inotify_data to the end --- src/libsystemd/sd-event/event-source.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 3be9a0e6c1..5fef3b1ac9 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -220,10 +220,6 @@ struct inotify_data { Hashmap *inodes; /* The inode_data structures keyed by dev+ino */ Hashmap *wd; /* The inode_data structures keyed by the watch descriptor for each */ - /* The buffer we read inotify events into */ - union inotify_event_buffer buffer; - size_t buffer_filled; /* fill level of the buffer */ - /* How many event sources are currently marked pending for this inotify. We won't read new events off the * inotify fd as long as there are still pending events on the inotify (because we have no strategy of queuing * the events locally if they can't be coalesced). */ @@ -237,4 +233,9 @@ struct inotify_data { /* A linked list of all inotify objects with data already read, that still need processing. We keep this list * to make it efficient to figure out what inotify objects to process data on next. */ LIST_FIELDS(struct inotify_data, buffered); + + /* The buffer we read inotify events into */ + size_t buffer_filled; /* fill level of the buffer */ + union inotify_event_buffer buffer; /* struct inotify_event in union inotify_event_buffer has flex + * array. Hence, this must be at the end. */ }; From fda47cd92b1bb41bb23b6aa3abcab1cc4c6a51c0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 4 Jun 2025 12:52:12 +0900 Subject: [PATCH 5/6] sd-dhcp: explicitly set buffer size of each type --- src/libsystemd-network/dhcp-client-id-internal.h | 4 +++- src/libsystemd-network/dhcp-duid-internal.h | 10 +++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/dhcp-client-id-internal.h b/src/libsystemd-network/dhcp-client-id-internal.h index f43d278cdd..2f51968758 100644 --- a/src/libsystemd-network/dhcp-client-id-internal.h +++ b/src/libsystemd-network/dhcp-client-id-internal.h @@ -32,7 +32,7 @@ typedef struct sd_dhcp_client_id { } _packed_ eth; struct { /* 2 - 254: ARP/Link-Layer (RFC 2132) */ - uint8_t haddr[0]; + uint8_t haddr[HW_ADDR_MAX_SIZE]; } _packed_ ll; struct { /* 255: Node-specific (RFC 4361) */ @@ -46,6 +46,8 @@ typedef struct sd_dhcp_client_id { }; } sd_dhcp_client_id; +assert_cc(sizeof_field(sd_dhcp_client_id, id) <= MAX_CLIENT_ID_LEN); + static inline bool client_id_size_is_valid(size_t size) { return size >= MIN_CLIENT_ID_LEN && size <= MAX_CLIENT_ID_LEN; } diff --git a/src/libsystemd-network/dhcp-duid-internal.h b/src/libsystemd-network/dhcp-duid-internal.h index 061786cd02..c2af1fa4dd 100644 --- a/src/libsystemd-network/dhcp-duid-internal.h +++ b/src/libsystemd-network/dhcp-duid-internal.h @@ -36,17 +36,19 @@ struct duid { /* DUID_TYPE_LLT */ be16_t htype; be32_t time; - uint8_t haddr[]; + uint8_t haddr[HW_ADDR_MAX_SIZE]; } _packed_ llt; struct { /* DUID_TYPE_EN */ be32_t pen; - uint8_t id[]; + /* The maximum length of vendor ID is not provided in RFC 8415, but we use 8 bytes. + * See https://datatracker.ietf.org/doc/html/rfc8415#section-11.3 */ + uint8_t id[8]; } _packed_ en; struct { /* DUID_TYPE_LL */ be16_t htype; - uint8_t haddr[]; + uint8_t haddr[HW_ADDR_MAX_SIZE]; } _packed_ ll; struct { /* DUID_TYPE_UUID */ @@ -56,6 +58,8 @@ struct duid { }; } _packed_; +assert_cc(sizeof(struct duid) == MAX_DUID_LEN); + typedef struct sd_dhcp_duid { size_t size; union { From e311402d4a0e45e1499014820abe7e2210ab8c3b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 17 May 2025 02:18:32 +0900 Subject: [PATCH 6/6] meson: enable -Wgnu-variable-sized-type-not-at-end again Follow-up for ab29e77aa9efc0863216e9415481a786158fd60a. For gcc, flex-array-member-not-at-end was enabled by the commit, but the option for clang with the same effect was still disabled. Let's reenable it. Prompted by #37497. --- meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/meson.build b/meson.build index f8e7fcade2..f31cdca6ab 100644 --- a/meson.build +++ b/meson.build @@ -489,7 +489,6 @@ endif if cc.get_id() == 'clang' possible_common_cc_flags += [ '-Wno-typedef-redefinition', - '-Wno-gnu-variable-sized-type-not-at-end', ] endif