Skip to content

Commit a7ecad7

Browse files
blucadaandemeyer
authored andcommitted
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 cf88903 ("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 systemd#2 0xc0a2edc6 in raise () from /usr/lib/m68k-linux-gnu/libc.so.6 systemd#3 0xc0a1c128 in abort () from /usr/lib/m68k-linux-gnu/libc.so.6 systemd#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 cf88903 Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 112453b commit a7ecad7

4 files changed

Lines changed: 15 additions & 4 deletions

File tree

src/basic/static-destruct.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ typedef struct SimpleCleanup {
3636
free_func_t destroy;
3737
} SimpleCleanup;
3838

39-
typedef struct StaticDestructor {
39+
/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
40+
typedef struct _alignptr_ StaticDestructor {
4041
StaticDestructorType type;
4142
union {
4243
SimpleCleanup simple;

src/shared/options.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ typedef enum OptionFlags {
2929
OPTION_HELP_ENTRY_VERBATIM = 1U << 6, /* Same, but use the long_code in the first column as written */
3030
} OptionFlags;
3131

32-
typedef struct Option {
32+
/* Note: the alignment attribute must match the one applied to each variable via _alignptr_ in
33+
* _OPTION() below. Otherwise the struct's sizeof and the actual stride between consecutive entries
34+
* placed in the SYSTEMD_OPTIONS section would not match on architectures where the natural
35+
* alignment of the struct is smaller than sizeof(void*) (e.g. m68k). That would cause the
36+
* pointer-arithmetic-based iteration over the section to read from padding bytes. */
37+
typedef struct _alignptr_ Option {
3338
int id;
3439
OptionFlags flags;
3540
char short_code;
@@ -38,6 +43,7 @@ typedef struct Option {
3843
uintptr_t data;
3944
const char *help;
4045
} Option;
46+
assert_cc(sizeof(Option) % sizeof(void*) == 0);
4147

4248
#define _OPTION(counter, fl, sc, lc, mv, d, h) \
4349
_section_("SYSTEMD_OPTIONS") \

src/shared/tests.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ int define_hex_ptr_internal(const char *hex, void **name, size_t *name_len);
8989
/* Provide a convenient way to check if we're running in CI. */
9090
const char* ci_environment(void);
9191

92-
typedef struct TestFunc {
92+
/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
93+
typedef struct _alignptr_ TestFunc {
9394
union f {
9495
void (*void_func)(void);
9596
int (*int_func)(void);
@@ -98,6 +99,7 @@ typedef struct TestFunc {
9899
bool has_ret:1;
99100
bool sd_booted:1;
100101
} TestFunc;
102+
assert_cc(sizeof(TestFunc) % sizeof(void*) == 0);
101103

102104
/* See static-destruct.h for an explanation of how this works. */
103105
#define REGISTER_TEST(func, ...) \

src/shared/verbs.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ typedef enum VerbFlags {
1111
VERB_GROUP_MARKER = 1 << 2, /* Fake verb entry to separate groups */
1212
} VerbFlags;
1313

14-
typedef struct {
14+
/* Note: see the comment on struct Option in options.h for why _alignptr_ is required here. */
15+
typedef struct _alignptr_ {
1516
const char *verb;
1617
unsigned min_args, max_args;
1718
VerbFlags flags;
@@ -20,6 +21,7 @@ typedef struct {
2021
const char *argspec;
2122
const char *help;
2223
} Verb;
24+
assert_cc(sizeof(Verb) % sizeof(void*) == 0);
2325

2426
#define _VERB_DATA(d, v, a, amin, amax, f, dat, h) \
2527
_section_("SYSTEMD_VERBS") \

0 commit comments

Comments
 (0)