From: Geert Uytterhoeven <geert@linux-m68k.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Eugene Syromiatnikov <esyr@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux/m68k <linux-m68k@vger.kernel.org>
Subject: Re: [GIT PULL] siginfo fix for v4.16-rc5
Date: Tue, 3 Apr 2018 09:30:36 +0200 [thread overview]
Message-ID: <CAMuHMdVtbhGJ=nGbB21O73DcYM-4csyuJbx2NksN379ww6c1Gw@mail.gmail.com> (raw)
In-Reply-To: <87woxpz7k9.fsf@xmission.com>
Hi Eric,
On Mon, Apr 2, 2018 at 10:17 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Eugene Syromiatnikov <esyr@redhat.com> writes:
>
>> So, the offset of the si_lower field is 20 at the current HEAD and was 18 at
>> commits v4.16-rc3~17^2 and v4.16-rc1~159^2~20. I believe this is due to
>> the fact that m68k uses 2-byte default alignment and not 4-byte.
>
> A 2-byte alignment for 4 byte pointers. That is a new one to me.
Not just for pointers, also for int and long.
And m68k is not the only architecture having such alignment rules.
> Euguene can you test the patch below. It should be fully robust against
> this kind of craziness. It certainly passes my BUILD_BUG_ON tests for
> m68k.
>
> Eric
>
> From: "Eric W. Biederman" <ebiederm@xmission.com>
> Date: Mon, 2 Apr 2018 14:45:42 -0500
> Subject: [PATCH] signal: Correct the offset of si_pkey and si_lower in struct siginfo on m68k
>
> The change moving addr_lsb into the _sigfault union failed to take
> into account that _sigfault._addr_bnd._lower being a pointer forced
> the entire union to have pointer alignment. The fix for
> _sigfault._addr_bnd._lower having pointer alignment failed to take
> into account that m68k has a pointer alignment less than the size
> of a pointer. So simply making the padding members pointers changed
> the location of later members in the structure.
>
> Fix this by directly computing the needed size of the padding members,
> and making the padding members char arrays of the needed size. AKA
> if __alignof__(void *) is 1 sizeof(short) otherwise __alignof__(void *).
> Which should be exactly the same rules the compiler whould have
> used when computing the padding.
__alignof__(void *) is 2 not 1 on m68k.
> I have tested this change by adding BUILD_BUG_ONs to m68k to verify
> the offset of every member of struct siginfo, and with those testing
> that the offsets of the fields in struct siginfo is the same before
> I changed the generic _sigfault member and after the correction
> to the _sigfault member.
>
> I have also verified that the x86 with it's own BUILD_BUG_ONs to verify
> the offsets of the siginfo members also compiles cleanly.
>
> Cc: stable@vger.kernel.org
> Reported-by: Eugene Syromiatnikov <esyr@redhat.com>
> Fixes: 859d880cf544 ("signal: Correct the offset of si_pkey in struct siginfo")
> Fixes: b68a68d3dcc1 ("signal: Move addr_lsb into the _sigfault union for clarity")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> include/linux/compat.h | 6 ++++--
> include/uapi/asm-generic/siginfo.h | 7 +++++--
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index e16d07eb08cf..d770e62632d7 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -221,6 +221,8 @@ typedef struct compat_siginfo {
> #ifdef __ARCH_SI_TRAPNO
> int _trapno; /* TRAP # which caused the signal */
> #endif
> +#define __COMPAT_ADDR_BND_PKEY_PAD (__alignof__(compat_uptr_t) < sizeof(short) ? \
> + sizeof(short) : __alignof__(compat_uptr_t))
On m68k, __alignof__(compat_uptr_t) == 2, so it will use
__alignof__(compat_uptr_t) padding bytes.
Note that while the test is wrong, the end result is correct :-)
Hence you could just use __alignof__(compat_uptr_t) padding bytes
unconditionally?
> union {
> /*
> * used when si_code=BUS_MCEERR_AR or
> @@ -229,13 +231,13 @@ typedef struct compat_siginfo {
> short int _addr_lsb; /* Valid LSB of the reported address. */
> /* used when si_code=SEGV_BNDERR */
> struct {
> - compat_uptr_t _dummy_bnd;
> + char _dummy_bnd[__COMPAT_ADDR_BND_PKEY_PAD];
> compat_uptr_t _lower;
> compat_uptr_t _upper;
> } _addr_bnd;
> /* used when si_code=SEGV_PKUERR */
> struct {
> - compat_uptr_t _dummy_pkey;
> + char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
> u32 _pkey;
> } _addr_pkey;
> };
> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> index 4b3520bf67ba..6d789648473d 100644
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -94,6 +94,9 @@ typedef struct siginfo {
> unsigned int _flags; /* see ia64 si_flags */
> unsigned long _isr; /* isr */
> #endif
> +
> +#define __ADDR_BND_PKEY_PAD (__alignof__(void *) < sizeof(short) ? \
> + sizeof(short) : __alignof__(void *))
Likewise.
> union {
> /*
> * used when si_code=BUS_MCEERR_AR or
> @@ -102,13 +105,13 @@ typedef struct siginfo {
> short _addr_lsb; /* LSB of the reported address */
> /* used when si_code=SEGV_BNDERR */
> struct {
> - void *_dummy_bnd;
> + char _dummy_bnd[__ADDR_BND_PKEY_PAD];
> void __user *_lower;
> void __user *_upper;
> } _addr_bnd;
> /* used when si_code=SEGV_PKUERR */
> struct {
> - void *_dummy_pkey;
> + char _dummy_pkey[__ADDR_BND_PKEY_PAD];
> __u32 _pkey;
> } _addr_pkey;
> };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2018-04-03 7:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <87woypy8zc.fsf@xmission.com>
2018-03-31 10:56 ` [GIT PULL] siginfo fix for v4.16-rc5 Eugene Syromiatnikov
2018-04-02 20:17 ` Eric W. Biederman
2018-04-03 7:30 ` Geert Uytterhoeven [this message]
2018-04-03 14:27 ` Eric W. Biederman
2018-04-03 15:24 ` Josh Juran
2018-04-03 17:26 ` Andreas Schwab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMuHMdVtbhGJ=nGbB21O73DcYM-4csyuJbx2NksN379ww6c1Gw@mail.gmail.com' \
--to=geert@linux-m68k.org \
--cc=ebiederm@xmission.com \
--cc=esyr@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).