* Andy Lutomirski <luto@xxxxxxxxxx> wrote: > i386 glibc is buggy and calls the sigaction syscall incorrectly. > This is asymptomatic for normal programs, but it blows up on > programs that do evil things with segmentation. ldt_gdt an example > of such an evil program. > > This doesn't appear to be a regression -- I think I just got lucky > with the uninitialized memory that glibc threw at the kernel when I > wrote the test. > > This hackish fix manually issues sigaction(2) syscalls to undo the > damage. Without the fix, ldt_gdt_32 segfaults; with the fix, it > passes for me. > > See https://sourceware.org/bugzilla/show_bug.cgi?id=21269 > > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx> > --- > > I'll see about factoring out sethandler(), etc into a separate file > soon. In the mean time, this at least makes the test pass. > > tools/testing/selftests/x86/ldt_gdt.c | 36 +++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/tools/testing/selftests/x86/ldt_gdt.c b/tools/testing/selftests/x86/ldt_gdt.c > index f6121612e769..18e6ae1f1bb6 100644 > --- a/tools/testing/selftests/x86/ldt_gdt.c > +++ b/tools/testing/selftests/x86/ldt_gdt.c > @@ -409,6 +409,24 @@ static void *threadproc(void *ctx) > } > } > > +#ifdef __i386__ > + > +#ifndef SA_RESTORE > +#define SA_RESTORER 0x04000000 > +#endif This looks nicer IMHO: #ifndef SA_RESTORE # define SA_RESTORER 0x04000000 #endif > + > +/* > + * The UAPI header calls this 'struct sigaction', which conflicts with > + * glibc. Sigh. > + */ > +struct fake_ksigaction { > + void *handler; /* the real type is nasty */ > + unsigned long sa_flags; > + void (*sa_restorer)(void); > + unsigned long sigset1, sigset2; > +}; Please use tabs, not spaces. Also, don't merge types on the same line. I.e. something like: struct fake_ksigaction { void *handler; /* the real type is nasty */ unsigned long sa_flags; void (*sa_restorer)(void); unsigned long sigset1; unsigned long sigset2; }; > +#ifdef __i386__ > + struct fake_ksigaction ksa; Please either move this into a helper function or add a new block, we shouldn't declare new local variables C++ style. How come the compiler didn't warn about this? We should use the kernel build warnings. > + if (syscall(SYS_rt_sigaction, sig, NULL, &ksa, 8) == 0) { > + /* > + * glibc has a nasty bug: it sometimes writes garbage to > + * sa_restorer. This interacts quite badly with anything > + * that fiddles with SS because it can trigger legacy > + * stack switching. Patch it up. > + */ > + printf("%d asdf %lx %p\n", sig, ksa.sa_flags, ksa.sa_restorer); > + if (!(ksa.sa_flags & SA_RESTORER) && ksa.sa_restorer) { > + printf("asdffff\n"); > + ksa.sa_restorer = NULL; > + if (syscall(SYS_rt_sigaction, sig, &ksa, NULL, 8) != 0) > + err(1, "rt_sigaction"); What does the '8' stand for? Thanks, Ingo