All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Dave Hansen <dave@sr71.net>
Cc: linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
	sfr@canb.auug.org.au, akpm@linux-foundation.org,
	tglx@linutronix.de, mingo@elte.hu, hpa@zytor.com,
	peterz@infradead.org, linux-next@vger.kernel.org, deller@gmx.de
Subject: Re: [PATCH] [v3] x86, pkeys: fix siginfo ABI breakage from new field
Date: Tue, 1 Mar 2016 10:39:06 +0100	[thread overview]
Message-ID: <20160301093906.GA10360@gmail.com> (raw)
In-Reply-To: <20160301074052.GA7201@gmail.com>


> > A u64 was used for the protection key field in siginfo.  When the
> > containing union was aligned, this u64 unioned nicely with the
> > two 'void *'s in _addr_bnd.  But, on 32-bit, if the union was
> > unaligned, the u64 might grow the size of the union, breaking the
> > ABI for subsequent fields.

Btw., I think this explanation is incorrect, the layout of _addr_bnd is 
irrelevant.

What happened on some 32-bit platforms is the following: if u64 has a natural 
alignment of 8 bytes (this is rare, most 32-bit platforms align it to 4 bytes), 
then the leadup to the _sifields union matters:

typedef struct siginfo {
        int si_signo;
        int si_errno;
        int si_code;

        union {
	...
        } _sifields;
} __ARCH_SI_ATTRIBUTES siginfo_t;

Note how the first 3 fields give us 12 bytes, so _sifields is not 8 naturally 
bytes aligned.

Before the _pkey field addition the largest element of _sifields (on 32-bit 
platforms) was 32 bits. With the u64 added, the minimum alignment requirement 
increased to 8 bytes on those (rare) 32-bit platforms. Thus GCC padded the space 
after si_code with 4 extra bytes, and shifted all _sifields offsets by 4 bytes - 
breaking the ABI of all of those remaining fields.

On 64-bit platforms this problem was hidden due to _sifields already having 
numerous fields with natural 8 bytes alignment (pointers).

If you agree with this analysis then mind updating the changelog accordingly?

Thanks,

	Ingo

      parent reply	other threads:[~2016-03-01  9:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-29 22:17 [PATCH] [v3] x86, pkeys: fix siginfo ABI breakage from new field Dave Hansen
2016-02-29 22:33 ` Stephen Rothwell
2016-03-01  7:40 ` Ingo Molnar
2016-03-01  8:09   ` Ingo Molnar
2016-03-01 12:48     ` Dave Hansen
2016-03-01  9:39   ` Ingo Molnar [this message]

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=20160301093906.GA10360@gmail.com \
    --to=mingo@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@sr71.net \
    --cc=deller@gmx.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=sfr@canb.auug.org.au \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.