From: Luca Boccassi Date: Sun, 24 May 2026 11:01:24 +0000 (+0100) Subject: shared/options: fix crash by aligning struct to sizeof(void*) X-Git-Tag: v261-rc2~38 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=a7ecad77cbb00ecd388d6d4bbc924704ecbe1ebc;p=thirdparty%2Fsystemd.git shared/options: fix crash by aligning struct to sizeof(void*) 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 --- diff --git a/src/basic/static-destruct.h b/src/basic/static-destruct.h index fdc4c7c41ec..8e71c9b235a 100644 --- a/src/basic/static-destruct.h +++ b/src/basic/static-destruct.h @@ -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; diff --git a/src/shared/options.h b/src/shared/options.h index fd3ab008d36..f02eb30027f 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -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") \ diff --git a/src/shared/tests.h b/src/shared/tests.h index 9a2b7c04128..3f8fdf7fa76 100644 --- a/src/shared/tests.h +++ b/src/shared/tests.h @@ -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, ...) \ diff --git a/src/shared/verbs.h b/src/shared/verbs.h index 2a9df84e0c3..253181e8c47 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -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") \