Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1343790 > unrolled thread

linux-next: build failure after merge of the akpm-current tree

Started byStephen Rothwell <sfr@canb.auug.org.au>
First post2016-02-26 06:50 +0100
Last post2016-02-26 23:50 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  linux-next: build failure after merge of the akpm-current tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-02-26 06:50 +0100
    Re: linux-next: build failure after merge of the akpm-current tree Dave Hansen <dave.hansen@linux.intel.com> - 2016-02-26 17:20 +0100
    Re: linux-next: build failure after merge of the akpm-current tree Dave Hansen <dave.hansen@linux.intel.com> - 2016-02-26 21:20 +0100
      Re: linux-next: build failure after merge of the akpm-current tree Andrew Morton <akpm@linux-foundation.org> - 2016-02-26 22:40 +0100
        Re: linux-next: build failure after merge of the akpm-current tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-02-26 23:50 +0100

#1343790 — linux-next: build failure after merge of the akpm-current tree

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2016-02-26 06:50 +0100
Subjectlinux-next: build failure after merge of the akpm-current tree
Message-ID<r6jrP-6ZV-1@gated-at.bofh.it>
Hi all,

After merging the akpm-current tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:

In file included from /home/sfr/next/next/include/linux/linkage.h:4:0,
                 from /home/sfr/next/next/arch/arm/include/asm/bug.h:4,
                 from /home/sfr/next/next/include/linux/bug.h:4,
                 from /home/sfr/next/next/include/linux/mmdebug.h:4,
                 from /home/sfr/next/next/include/linux/gfp.h:4,
                 from /home/sfr/next/next/include/linux/slab.h:14,
                 from /home/sfr/next/next/kernel/signal.c:13:
/home/sfr/next/next/kernel/signal.c: In function 'signals_init':
/home/sfr/next/next/include/linux/compiler.h:506:38: error: call to '__compiletime_assert_3610' declared with attribute error: BUILD_BUG_ON failed: __ARCH_SI_PREAMBLE_SIZE != offsetof(struct siginfo, _sifields._pad)

Caused by commit

  21c0826ab237 ("kernel/signal.c: add compile-time check for __ARCH_SI_PREAMBLE_SIZE")

interacting with commit

  cd0ea35ff551 ("signals, pkeys: Notify userspace about protection key faults")

from the tip tree.

The addition of the "u64 _pkey" has presumably changed the alignment of
the enclosing union on (some) 32 bit platforms and so added padding
after the si_code field.  This is a user API issue. :-(

[As an aside, I am pretty sure that we should not be using "u64" in a
uapi header in any case.]

I have added the following patch for today (and will add it to the tip
tree merge from Monday unless some other fix comes along).

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 26 Feb 2016 16:31:36 +1100
Subject: [PATCH] signals, pkeys: make si_pkey 32 bits for now

to prevent a change of alignment in the siginfo structure

Fixes: cd0ea35ff551 ("signals, pkeys: Notify userspace about protection key faults")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/ia64/include/uapi/asm/siginfo.h | 2 +-
 arch/mips/include/uapi/asm/siginfo.h | 2 +-
 include/uapi/asm-generic/siginfo.h   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/uapi/asm/siginfo.h b/arch/ia64/include/uapi/asm/siginfo.h
index 0151cfab929d..0c660bbccd3f 100644
--- a/arch/ia64/include/uapi/asm/siginfo.h
+++ b/arch/ia64/include/uapi/asm/siginfo.h
@@ -70,7 +70,7 @@ typedef struct siginfo {
 					void __user *_upper;
 				} _addr_bnd;
 				/* used when si_code=SEGV_PKUERR */
-				u64 _pkey;
+				int _pkey;
 			};
 		} _sigfault;
 
diff --git a/arch/mips/include/uapi/asm/siginfo.h b/arch/mips/include/uapi/asm/siginfo.h
index 6f4edf0d794c..d2adebc0023d 100644
--- a/arch/mips/include/uapi/asm/siginfo.h
+++ b/arch/mips/include/uapi/asm/siginfo.h
@@ -93,7 +93,7 @@ typedef struct siginfo {
 					void __user *_upper;
 				} _addr_bnd;
 				/* used when si_code=SEGV_PKUERR */
-				u64 _pkey;
+				int _pkey;
 			};
 		} _sigfault;
 
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 90384d55225b..0f23ec63c291 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -98,7 +98,7 @@ typedef struct siginfo {
 					void __user *_upper;
 				} _addr_bnd;
 				/* used when si_code=SEGV_PKUERR */
-				u64 _pkey;
+				int _pkey;
 			};
 		} _sigfault;
 
-- 
2.7.0

-- 
Cheers,
Stephen Rothwell

[toc] | [next] | [standalone]


#1344410

FromDave Hansen <dave.hansen@linux.intel.com>
Date2016-02-26 17:20 +0100
Message-ID<r6thw-5Uo-7@gated-at.bofh.it>
In reply to#1343790
On 02/25/2016 09:44 PM, Stephen Rothwell wrote:
> The addition of the "u64 _pkey" has presumably changed the alignment of
> the enclosing union on (some) 32 bit platforms and so added padding
> after the si_code field.  This is a user API issue. :-(
> 
> [As an aside, I am pretty sure that we should not be using "u64" in a
> uapi header in any case.]

Ahh, so if _addr_bnd wasn't 64-bit-aligned, the compiler is free to
align it and enlarge the structure.  But, this only applied to
architectures where _addr_bnd wasn't 64-bit-aligned:

Would anybody object to _pkey being an 'unsigned long'?  It would at
least keep existing 64-bit userspace (mostly my tests) from having to
change.

Here's the snippet of the struct in question:

>                         union {
>                                 /* used when si_code=SEGV_BNDERR */
>                                 struct {
>                                         void __user *_lower;
>                                         void __user *_upper;
>                                 } _addr_bnd;
>                                 /* used when si_code=SEGV_PKUERR */
>                                 u64 _pkey;
>                         };

[toc] | [prev] | [next] | [standalone]


#1344650

FromDave Hansen <dave.hansen@linux.intel.com>
Date2016-02-26 21:20 +0100
Message-ID<r6x1M-6H-21@gated-at.bofh.it>
In reply to#1343790
On 02/25/2016 09:44 PM, Stephen Rothwell wrote:
> Fixes: cd0ea35ff551 ("signals, pkeys: Notify userspace about protection key faults")
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  arch/ia64/include/uapi/asm/siginfo.h | 2 +-
>  arch/mips/include/uapi/asm/siginfo.h | 2 +-
>  include/uapi/asm-generic/siginfo.h   | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/ia64/include/uapi/asm/siginfo.h b/arch/ia64/include/uapi/asm/siginfo.h
> index 0151cfab929d..0c660bbccd3f 100644
> --- a/arch/ia64/include/uapi/asm/siginfo.h
> +++ b/arch/ia64/include/uapi/asm/siginfo.h
> @@ -70,7 +70,7 @@ typedef struct siginfo {
>  					void __user *_upper;
>  				} _addr_bnd;
>  				/* used when si_code=SEGV_PKUERR */
> -				u64 _pkey;
> +				int _pkey;
>  			};
>  		} _sigfault;

Considering Peter's feedback on my 'unsigned long' approach, I'm fine
with your patch, Stephen.  Can it just be picked up in to the tip tree?

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

[toc] | [prev] | [next] | [standalone]


#1344702

FromAndrew Morton <akpm@linux-foundation.org>
Date2016-02-26 22:40 +0100
Message-ID<r6yhc-RJ-37@gated-at.bofh.it>
In reply to#1344650
On Fri, 26 Feb 2016 12:10:06 -0800 Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 02/25/2016 09:44 PM, Stephen Rothwell wrote:
> > Fixes: cd0ea35ff551 ("signals, pkeys: Notify userspace about protection key faults")
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > ---
> >  arch/ia64/include/uapi/asm/siginfo.h | 2 +-
> >  arch/mips/include/uapi/asm/siginfo.h | 2 +-
> >  include/uapi/asm-generic/siginfo.h   | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/ia64/include/uapi/asm/siginfo.h b/arch/ia64/include/uapi/asm/siginfo.h
> > index 0151cfab929d..0c660bbccd3f 100644
> > --- a/arch/ia64/include/uapi/asm/siginfo.h
> > +++ b/arch/ia64/include/uapi/asm/siginfo.h
> > @@ -70,7 +70,7 @@ typedef struct siginfo {
> >  					void __user *_upper;
> >  				} _addr_bnd;
> >  				/* used when si_code=SEGV_PKUERR */
> > -				u64 _pkey;
> > +				int _pkey;
> >  			};
> >  		} _sigfault;
> 
> Considering Peter's feedback on my 'unsigned long' approach, I'm fine
> with your patch, Stephen.  Can it just be picked up in to the tip tree?
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

I've moved add-compile-time-check-for-__arch_si_preamble_size.patch to
be after linux-next and have added this patch (as
add-compile-time-check-for-__arch_si_preamble_size-fix.patch) to be
folded into add-compile-time-check-for-__arch_si_preamble_size.patch
later on.

So no -tip changes should be needed.

[toc] | [prev] | [next] | [standalone]


#1344803

FromStephen Rothwell <sfr@canb.auug.org.au>
Date2016-02-26 23:50 +0100
Message-ID<r6zmW-1EG-3@gated-at.bofh.it>
In reply to#1344702
Hi Andrew,

On Fri, 26 Feb 2016 13:33:50 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> I've moved add-compile-time-check-for-__arch_si_preamble_size.patch to
> be after linux-next and have added this patch (as
> add-compile-time-check-for-__arch_si_preamble_size-fix.patch) to be
> folded into add-compile-time-check-for-__arch_si_preamble_size.patch
> later on.
> 
> So no -tip changes should be needed.

But the fix should go into the tip tree since the tip tree patch is not
correct even without this new build time check.

-- 
Cheers,
Stephen Rothwell

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web