| Message ID | 20100209.165006.142022647.davem@davemloft.net |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
On Tuesday 09 February 2010 06:50:06 pm David Miller wrote: > [ Josip, when I hit this I thought that if it were a gcc bug it might > explain your gcc-4.4 kernel failures. That's why I haven't played > with your machines yet. Now that I know this is actually a kernel > bug effecting only user applications, I will go pursue your > issues. ] > > I originally thought this was a gcc bug: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43004 > > But it's actually a kernel issue. > > When we clone or create a signal stack we only 8-byte align the stack > frame, it should be 16-byte aligned on 64-bit. Use 16-byte alignment > for 32-bit too since that's harmless and it's cheaper that way. > > Distribution folks you really want this in your kernels, otherwise > 64-bit threads and signal handlers will have broken alloca() and > corrupt their stacks. I thought that maybe this would help with a localdef generation issue that is being seen while building glibc, at least for debian and fedora. however it seems not. I have built the latest Fedora 12 stable kernel with your patch applied and glibc failed to build still. patches had been submitted to glibc http://sources.redhat.com/bugzilla/show_bug.cgi?id=10855 http://sources.redhat.com/bugzilla/show_bug.cgi?id=10283 however one had been outright rejected and the sparc specific one has not been looked at. Is this something that should be fixed in the kernel? Dennis
From: Dennis Gilmore <dennis@ausil.us> Date: Wed, 10 Feb 2010 13:59:00 -0600 > http://sources.redhat.com/bugzilla/show_bug.cgi?id=10855 As the patch in this bug does, it should be fixed in glibc. GCC had the same exact issues when creating precompiled headers in c++. > http://sources.redhat.com/bugzilla/show_bug.cgi?id=10283 Hey, Ulrich Drepper unreasonably flames someone for trying to fix a bug, I've never seen that before! :-) His comment about "all of userland expects page-size alignment to be sufficient" is completely wrong, you only need look at the precompiled header support code in GCC to see but one example. Simple sys_errlist versioning build fixes for Sparc from mid-January still have not been applied, and I supposed STT_GNU_IFUNC support for sparc last week to the glibc lists and that hasn't gotten looked at either. So the lack of any real looking into this issue is no surprise. This is not a kernel bug, shared mappings have stringent, architecture specific requirement wrt. mmap() area alignment in order to handle cache aliasing issues properly. MAP_SHARED mappings are special, you have to be aware of the issues if you're going to use them. You can't just map them wherever you like. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller <davem@davemloft.net> Date: Wed, 10 Feb 2010 12:12:17 -0800 (PST) > From: Dennis Gilmore <dennis@ausil.us> > Date: Wed, 10 Feb 2010 13:59:00 -0600 > >> http://sources.redhat.com/bugzilla/show_bug.cgi?id=10855 > > As the patch in this bug does, it should be fixed in glibc. As an update I just learned about this commit: http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=6a147cc6e402e7167fa1213c31dc42a6fa38f52b which should fix this problem. It's in the feora branch, but not mainline. But hopefully it will propagate to mainline soon. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c index 2830b41..f23c8fd 100644 --- a/arch/sparc/kernel/process_32.c +++ b/arch/sparc/kernel/process_32.c @@ -526,7 +526,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, * Set some valid stack frames to give to the child. */ childstack = (struct sparc_stackf __user *) - (sp & ~0x7UL); + (sp & ~0x15UL); parentstack = (struct sparc_stackf __user *) regs->u_regs[UREG_FP]; diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c index 18d6785..6679eeb 100644 --- a/arch/sparc/kernel/process_64.c +++ b/arch/sparc/kernel/process_64.c @@ -406,11 +406,11 @@ static unsigned long clone_stackframe(unsigned long csp, unsigned long psp) } else __get_user(fp, &(((struct reg_window32 __user *)psp)->ins[6])); - /* Now 8-byte align the stack as this is mandatory in the - * Sparc ABI due to how register windows work. This hides - * the restriction from thread libraries etc. -DaveM + /* Now align the stack as this is mandatory in the Sparc ABI + * due to how register windows work. This hides the + * restriction from thread libraries etc. */ - csp &= ~7UL; + csp &= ~15UL; distance = fp - psp; rval = (csp - distance); diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c index ba5b09a..ea22cd3 100644 --- a/arch/sparc/kernel/signal32.c +++ b/arch/sparc/kernel/signal32.c @@ -120,8 +120,8 @@ struct rt_signal_frame32 { }; /* Align macros */ -#define SF_ALIGNEDSZ (((sizeof(struct signal_frame32) + 7) & (~7))) -#define RT_ALIGNEDSZ (((sizeof(struct rt_signal_frame32) + 7) & (~7))) +#define SF_ALIGNEDSZ (((sizeof(struct signal_frame32) + 15) & (~15))) +#define RT_ALIGNEDSZ (((sizeof(struct rt_signal_frame32) + 15) & (~15))) int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from) { @@ -420,15 +420,17 @@ static void __user *get_sigframe(struct sigaction *sa, struct pt_regs *regs, uns sp = current->sas_ss_sp + current->sas_ss_size; } + sp -= framesize; + /* Always align the stack frame. This handles two cases. First, * sigaltstack need not be mindful of platform specific stack * alignment. Second, if we took this signal because the stack * is not aligned properly, we'd like to take the signal cleanly * and report that. */ - sp &= ~7UL; + sp &= ~15UL; - return (void __user *)(sp - framesize); + return (void __user *) sp; } static int save_fpu_state32(struct pt_regs *regs, __siginfo_fpu_t __user *fpu) diff --git a/arch/sparc/kernel/signal_32.c b/arch/sparc/kernel/signal_32.c index 7ce1a10..9882df9 100644 --- a/arch/sparc/kernel/signal_32.c +++ b/arch/sparc/kernel/signal_32.c @@ -267,15 +267,17 @@ static inline void __user *get_sigframe(struct sigaction *sa, struct pt_regs *re sp = current->sas_ss_sp + current->sas_ss_size; } + sp -= framesize; + /* Always align the stack frame. This handles two cases. First, * sigaltstack need not be mindful of platform specific stack * alignment. Second, if we took this signal because the stack * is not aligned properly, we'd like to take the signal cleanly * and report that. */ - sp &= ~7UL; + sp &= ~15UL; - return (void __user *)(sp - framesize); + return (void __user *) sp; } static inline int diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c index 647afbd..9fa48c3 100644 --- a/arch/sparc/kernel/signal_64.c +++ b/arch/sparc/kernel/signal_64.c @@ -353,7 +353,7 @@ segv: /* Checks if the fp is valid */ static int invalid_frame_pointer(void __user *fp, int fplen) { - if (((unsigned long) fp) & 7) + if (((unsigned long) fp) & 15) return 1; return 0; } @@ -396,15 +396,17 @@ static inline void __user *get_sigframe(struct k_sigaction *ka, struct pt_regs * sp = current->sas_ss_sp + current->sas_ss_size; } + sp -= framesize; + /* Always align the stack frame. This handles two cases. First, * sigaltstack need not be mindful of platform specific stack * alignment. Second, if we took this signal because the stack * is not aligned properly, we'd like to take the signal cleanly * and report that. */ - sp &= ~7UL; + sp &= ~15UL; - return (void __user *)(sp - framesize); + return (void __user *) sp; } static inline void
[ Josip, when I hit this I thought that if it were a gcc bug it might explain your gcc-4.4 kernel failures. That's why I haven't played with your machines yet. Now that I know this is actually a kernel bug effecting only user applications, I will go pursue your issues. ] I originally thought this was a gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43004 But it's actually a kernel issue. When we clone or create a signal stack we only 8-byte align the stack frame, it should be 16-byte aligned on 64-bit. Use 16-byte alignment for 32-bit too since that's harmless and it's cheaper that way. Distribution folks you really want this in your kernels, otherwise 64-bit threads and signal handlers will have broken alloca() and corrupt their stacks. I'll push this to Linus and -stable. -------------------- sparc: Align clone and signal stacks to 16 bytes. This is mandatory for 64-bit processes, and doing it also for 32-bit processes saves a conditional in the compat case. This fixes the glibc/nptl/tst-stdio1 test case, as well as many others, on 64-bit. Signed-off-by: David S. Miller <davem@davemloft.net> --- arch/sparc/kernel/process_32.c | 2 +- arch/sparc/kernel/process_64.c | 8 ++++---- arch/sparc/kernel/signal32.c | 10 ++++++---- arch/sparc/kernel/signal_32.c | 6 ++++-- arch/sparc/kernel/signal_64.c | 8 +++++--- 5 files changed, 20 insertions(+), 14 deletions(-)