From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AD8E6C433B4 for ; Wed, 5 May 2021 17:53:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8205D611EE for ; Wed, 5 May 2021 17:53:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235075AbhEERyd (ORCPT ); Wed, 5 May 2021 13:54:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55640 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234686AbhEERyK (ORCPT ); Wed, 5 May 2021 13:54:10 -0400 Received: from mail-ot1-x335.google.com (mail-ot1-x335.google.com [IPv6:2607:f8b0:4864:20::335]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 12BEEC034631 for ; Wed, 5 May 2021 10:25:12 -0700 (PDT) Received: by mail-ot1-x335.google.com with SMTP id 92-20020a9d02e50000b029028fcc3d2c9eso2432081otl.0 for ; Wed, 05 May 2021 10:25:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mhdimRmcWFxSoj8AFDYbNHsikBJUsUucL4YCEQ/2njw=; b=XwV87+f8d7jiYw4U+qEweujviEosUJz4mImc4wpkn1QEZXlev2sM2d4pHmB9ofh0n9 Yq3P7e/x1h5Ku11y/LkqEy7Tg83m26DuXSv/6b8ycMo0LPll3ANtPBbvosNpyd5xtL+E ENILOdjTnsjJQmXGdhRXu2Hpb+fNUHY+BTdNjf4x7S3P/lDuy7sGcKn1KV1P05zZkPaU +uqSz3jnlE/qY9W1RdsdkdaNVel7e9fhAlzMSfmNdykR+OeoIqGnuw6gDl9eOGrhMUs6 6lniOEx+5gWlfLei1GD2lVdJcYtfvvZwcta7uSNLTtcrObbDfRzjRogS9wh7FlfsBfeP l5Nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mhdimRmcWFxSoj8AFDYbNHsikBJUsUucL4YCEQ/2njw=; b=LXJ6sSxA1oeKWBKcxqVP9L64OSHIL9z+aPQSWznCbgEOmXuPziBjc9r46XoAHxwS9R 88i4ilyPcGkYCLUg7ONGwrREkMNfLZKjPa++1Qf387Oyt/aLhdU3Rg12JfOnPnIH4odv YPYVt9ZY0BtEcVnBkXdHxaGsGM11cJkRC7AZNUQd77yzFIyLId36aDN+YhLaP+lyTFHn XCmo0OUKESLrdlGnrnk46+sC2oE4rIp2k/0BVk5p5xnf1Dc2Jg7wPNRQPgpDy6ZIfBbZ W8enh7geXEA5EjnbQOCXvSlIJloU5/wnozNa+JjQ3rBb6aVrHRAZ5MQlLdtQLDi5v30H Wz/w== X-Gm-Message-State: AOAM531AIgMZNNBa5aUl/pVNvVvyAnKc6fsSb1gJngbQ8/EYxwxXMqsn 1KdKfEcOpBS3O312kI+QzkHFmrxayCblpJXZsylzwQ== X-Google-Smtp-Source: ABdhPJyRmdxXpkBjJhgLypJ27cbqA5yX2resHSeWAzU/LvydLIlTFHc1MP/3HSXWQhhwdgCDtzeYFR3RP7khraLUkho= X-Received: by 2002:a05:6830:410e:: with SMTP id w14mr23863201ott.251.1620235511237; Wed, 05 May 2021 10:25:11 -0700 (PDT) MIME-Version: 1.0 References: <20210505141101.11519-1-ebiederm@xmission.com> <20210505141101.11519-4-ebiederm@xmission.com> In-Reply-To: <20210505141101.11519-4-ebiederm@xmission.com> From: Marco Elver Date: Wed, 5 May 2021 19:24:00 +0200 Message-ID: Subject: Re: [PATCH v3 04/12] signal: Verify the alignment and size of siginfo_t To: "Eric W. Beiderman" Cc: Arnd Bergmann , Florian Weimer , "David S. Miller" , Peter Zijlstra , Ingo Molnar , Thomas Gleixner , Peter Collingbourne , Dmitry Vyukov , Alexander Potapenko , sparclinux , linux-arch , Linux Kernel Mailing List , Linux API , kasan-dev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 5 May 2021 at 16:11, Eric W. Beiderman wrote: > From: "Eric W. Biederman" > > 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@elver.google.com > Signed-off-by: "Eric W. Biederman" Acked-by: Marco Elver > --- > 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 >