On Wed, 5 May 2021 at 16:11, Eric W. Beiderman <ebiederm@xxxxxxxxxxxx> wrote: > From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > > Update the static assertions about siginfo_t to also describe > it's alignment and size. > > While investigating if it was possible to add a 64bit field into > siginfo_t[1] it became apparent that the alignment of siginfo_t > is as much a part of the ABI as the size of the structure. > > If the alignment changes siginfo_t when embedded in another structure > can move to a different offset. Which is not acceptable from an ABI > structure. > > So document that fact and add static assertions to notify developers > if they change change the alignment by accident. > > [1] https://lkml.kernel.org/r/YJEZdhe6JGFNYlum@xxxxxxxxxxxxxxxx > Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> Acked-by: Marco Elver <elver@xxxxxxxxxx> > --- > arch/arm/kernel/signal.c | 2 ++ > arch/arm64/kernel/signal.c | 2 ++ > arch/arm64/kernel/signal32.c | 2 ++ > arch/sparc/kernel/signal32.c | 2 ++ > arch/sparc/kernel/signal_64.c | 2 ++ > arch/x86/kernel/signal_compat.c | 6 ++++++ > include/uapi/asm-generic/siginfo.h | 5 +++++ > 7 files changed, 21 insertions(+) > > diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c > index 2dac5d2c5cf6..643bcb0f091b 100644 > --- a/arch/arm/kernel/signal.c > +++ b/arch/arm/kernel/signal.c > @@ -737,6 +737,8 @@ static_assert(NSIGBUS == 5); > static_assert(NSIGTRAP == 6); > static_assert(NSIGCHLD == 6); > static_assert(NSIGSYS == 2); > +static_assert(sizeof(siginfo_t) == 128); > +static_assert(__alignof__(siginfo_t) == 4); > static_assert(offsetof(siginfo_t, si_signo) == 0x00); > static_assert(offsetof(siginfo_t, si_errno) == 0x04); > static_assert(offsetof(siginfo_t, si_code) == 0x08); > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index af8bd2af1298..ad4bd27fc044 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -985,6 +985,8 @@ static_assert(NSIGBUS == 5); > static_assert(NSIGTRAP == 6); > static_assert(NSIGCHLD == 6); > static_assert(NSIGSYS == 2); > +static_assert(sizeof(siginfo_t) == 128); Would using SI_MAX_SIZE be appropriate? Perhaps not.. in case somebody changes it, given these static asserts are meant to double-check. I leave it to you to decide what makes more sense. > +static_assert(__alignof__(siginfo_t) == 8); > static_assert(offsetof(siginfo_t, si_signo) == 0x00); > static_assert(offsetof(siginfo_t, si_errno) == 0x04); > static_assert(offsetof(siginfo_t, si_code) == 0x08); > diff --git a/arch/arm64/kernel/signal32.c b/arch/arm64/kernel/signal32.c > index b6afb646515f..ee6c7484e130 100644 > --- a/arch/arm64/kernel/signal32.c > +++ b/arch/arm64/kernel/signal32.c > @@ -469,6 +469,8 @@ static_assert(NSIGBUS == 5); > static_assert(NSIGTRAP == 6); > static_assert(NSIGCHLD == 6); > static_assert(NSIGSYS == 2); > +static_assert(sizeof(compat_siginfo_t) == 128); > +static_assert(__alignof__(compat_siginfo_t) == 4); > static_assert(offsetof(compat_siginfo_t, si_signo) == 0x00); > static_assert(offsetof(compat_siginfo_t, si_errno) == 0x04); > static_assert(offsetof(compat_siginfo_t, si_code) == 0x08); > diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c > index 778ed5c26d4a..32b977f253e3 100644 > --- a/arch/sparc/kernel/signal32.c > +++ b/arch/sparc/kernel/signal32.c > @@ -757,6 +757,8 @@ static_assert(NSIGBUS == 5); > static_assert(NSIGTRAP == 6); > static_assert(NSIGCHLD == 6); > static_assert(NSIGSYS == 2); > +static_assert(sizeof(compat_siginfo_t) == 128); > +static_assert(__alignof__(compat_siginfo_t) == 4); > static_assert(offsetof(compat_siginfo_t, si_signo) == 0x00); > static_assert(offsetof(compat_siginfo_t, si_errno) == 0x04); > static_assert(offsetof(compat_siginfo_t, si_code) == 0x08); > diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c > index c9bbf5f29078..e9dda9db156c 100644 > --- a/arch/sparc/kernel/signal_64.c > +++ b/arch/sparc/kernel/signal_64.c > @@ -567,6 +567,8 @@ static_assert(NSIGBUS == 5); > static_assert(NSIGTRAP == 6); > static_assert(NSIGCHLD == 6); > static_assert(NSIGSYS == 2); > +static_assert(sizeof(siginfo_t) == 128); > +static_assert(__alignof__(siginfo_t) == 8); > static_assert(offsetof(siginfo_t, si_signo) == 0x00); > static_assert(offsetof(siginfo_t, si_errno) == 0x04); > static_assert(offsetof(siginfo_t, si_code) == 0x08); > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c > index 0e5d0a7e203b..e735bc129331 100644 > --- a/arch/x86/kernel/signal_compat.c > +++ b/arch/x86/kernel/signal_compat.c > @@ -34,7 +34,13 @@ static inline void signal_compat_build_tests(void) > BUILD_BUG_ON(NSIGSYS != 2); > > /* This is part of the ABI and can never change in size: */ > + BUILD_BUG_ON(sizeof(siginfo_t) != 128); > BUILD_BUG_ON(sizeof(compat_siginfo_t) != 128); > + > + /* This is a part of the ABI and can never change in alignment */ > + BUILD_BUG_ON(__alignof__(siginfo_t) != 8); > + BUILD_BUG_ON(__alignof__(compat_siginfo_t) != 4); > + > /* > * The offsets of all the (unioned) si_fields are fixed > * in the ABI, of course. Make sure none of them ever > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h > index 03d6f6d2c1fe..91c80d0c10c5 100644 > --- a/include/uapi/asm-generic/siginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -29,6 +29,11 @@ typedef union sigval { > #define __ARCH_SI_ATTRIBUTES > #endif > > +/* > + * Be careful when extending this union. On 32bit siginfo_t is 32bit > + * aligned. Which means that a 64bit field or any other field that > + * would increase the alignment of siginfo_t will break the ABI. > + */ > union __sifields { > /* kill() */ > struct { > -- > 2.30.1 >
