]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
shared/options: fix crash by aligning struct to sizeof(void*)
authorLuca Boccassi <luca.boccassi@gmail.com>
Sun, 24 May 2026 11:01:24 +0000 (12:01 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Sun, 24 May 2026 18:36:56 +0000 (20:36 +0200)
On some architectures like m68k, alignof(void*) is 2, not sizeof(void*)
(which is 4). So the natural alignment of struct Option is 2 and
sizeof(Option) == 26.

However, each variable placed in the SYSTEMD_OPTIONS ELF section via
_OPTION() carries _alignptr_ (= __attribute__((aligned(sizeof(void*))))),
which forces each entry to start at a 4-byte boundary. The linker
therefore inserts 2 bytes of padding between adjacent entries, producing
an actual stride of 28 in the section.

option_parse() iterates over the section with pointer arithmetic
("opt++"), which advances by sizeof(Option) == 26 and lands inside the
padding. The fields read back as zero, and since commit cf88903637
("tree-wide: get rid of most uses of ALIGN_PTR") added the ordering
assertion below, the resulting "0 < 0" trips it when running --help:

 Assertion 'opt->id < (opt + 1)->id' failed at src/shared/options.c:116, function option_parse(). Aborting.

 #0  0xc0a7b248 in ?? () from /usr/lib/m68k-linux-gnu/libc.so.6
 #1  0xc0a7b2ce in pthread_kill () from /usr/lib/m68k-linux-gnu/libc.so.6
 #2  0xc0a2edc6 in raise () from /usr/lib/m68k-linux-gnu/libc.so.6
 #3  0xc0a1c128 in abort () from /usr/lib/m68k-linux-gnu/libc.so.6
 #4  0xc05c6f78 in option_parse (options=0x0, options_end=0x0, state=0xc09ca968) at ../src/shared/options.c:116

Fix this by applying _alignptr_ to the struct definition itself, so that
sizeof(Option) is padded up to a multiple of sizeof(void*) and matches
the actual on-disk stride. Add an assert_cc() so any future regression
is caught at compile time.

The same latent bug applies to Verb and TestFunc, which use the same
section-placement pattern. Their natural sizeof was already a multiple
of sizeof(void*) so no crash was observed, but apply the same fix
defensively.

Follow-up for cf889036377092cbec6c5ec86fdf0dc1c9326032

Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
src/basic/static-destruct.h
src/shared/options.h
src/shared/tests.h
src/shared/verbs.h

index fdc4c7c41ec37506e83815b812ffea826f31ee73..8e71c9b235aec985c1f587101b5be1d2d4d1b164 100644 (file)
@@ -36,7 +36,8 @@ typedef struct SimpleCleanup {
         free_func_t destroy;
 } SimpleCleanup;
 
-typedef struct StaticDestructor {
+/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
+typedef struct _alignptr_ StaticDestructor {
         StaticDestructorType type;
         union {
                 SimpleCleanup simple;
index fd3ab008d36f281c48af624bfba46d7c39a0222d..f02eb30027f24f3ebf98b57572776f1cb1309fa2 100644 (file)
@@ -29,7 +29,12 @@ typedef enum OptionFlags {
         OPTION_HELP_ENTRY_VERBATIM = 1U << 6,  /* Same, but use the long_code in the first column as written */
 } OptionFlags;
 
-typedef struct Option {
+/* Note: the alignment attribute must match the one applied to each variable via _alignptr_ in
+ * _OPTION() below. Otherwise the struct's sizeof and the actual stride between consecutive entries
+ * placed in the SYSTEMD_OPTIONS section would not match on architectures where the natural
+ * alignment of the struct is smaller than sizeof(void*) (e.g. m68k). That would cause the
+ * pointer-arithmetic-based iteration over the section to read from padding bytes. */
+typedef struct _alignptr_ Option {
         int id;
         OptionFlags flags;
         char short_code;
@@ -38,6 +43,7 @@ typedef struct Option {
         uintptr_t data;
         const char *help;
 } Option;
+assert_cc(sizeof(Option) % sizeof(void*) == 0);
 
 #define _OPTION(counter, fl, sc, lc, mv, d, h)                          \
         _section_("SYSTEMD_OPTIONS")                                    \
index 9a2b7c0412825f9d324e0c6756ad5f0d01212cf9..3f8fdf7fa76948c109915c4633d371c4050cda3a 100644 (file)
@@ -89,7 +89,8 @@ int define_hex_ptr_internal(const char *hex, void **name, size_t *name_len);
 /* Provide a convenient way to check if we're running in CI. */
 const char* ci_environment(void);
 
-typedef struct TestFunc {
+/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
+typedef struct _alignptr_ TestFunc {
         union f {
                 void (*void_func)(void);
                 int (*int_func)(void);
@@ -98,6 +99,7 @@ typedef struct TestFunc {
         bool has_ret:1;
         bool sd_booted:1;
 } TestFunc;
+assert_cc(sizeof(TestFunc) % sizeof(void*) == 0);
 
 /* See static-destruct.h for an explanation of how this works. */
 #define REGISTER_TEST(func, ...)                                                                        \
index 2a9df84e0c33bc628c284e175baf4013fc260e33..253181e8c47f2c86ee5aa2c95a949b85a619b706 100644 (file)
@@ -11,7 +11,8 @@ typedef enum VerbFlags {
         VERB_GROUP_MARKER = 1 << 2,  /* Fake verb entry to separate groups */
 } VerbFlags;
 
-typedef struct {
+/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
+typedef struct _alignptr_ {
         const char *verb;
         unsigned min_args, max_args;
         VerbFlags flags;
@@ -20,6 +21,7 @@ typedef struct {
         const char *argspec;
         const char *help;
 } Verb;
+assert_cc(sizeof(Verb) % sizeof(void*) == 0);
 
 #define _VERB_DATA(d, v, a, amin, amax, f, dat, h)                      \
         _section_("SYSTEMD_VERBS")                                      \