| Message ID | 1465904958-27233-1-git-send-email-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Le 14/06/2016 à 13:49, Peter Maydell a écrit : > The kernel and libc have different ideas about what a sigset_t > is -- for the kernel it is only _NSIG / 8 bytes in size (usually > 8 bytes), but for libc it is much larger, 128 bytes. In most > situations the difference doesn't matter, because if you pass a > pointer to a libc sigset_t to the kernel it just acts on the first > 8 bytes of it, but for the ucontext_t* argument to a signal handler > it trips us up. The kernel allocates this ucontext_t on the stack > according to its idea of the sigset_t type, but the type of the > ucontext_t defined by the libc headers uses the libc type, and > so do the manipulator functions like sigfillset(). This means that > (1) sizeof(uc->uc_sigmask) is much larger than the actual > space used on the stack > (2) sigfillset(&uc->uc_sigmask) will write garbage 0xff bytes > off the end of the structure, which can trash data that > was on the stack before the signal handler was invoked, > and may result in a crash after the handler returns > > To avoid this, we use a memset() of the correct size to fill > the signal mask rather than using the libc function. > > This fixes a problem where we would crash at least some of the > time on an i386 host when a signal was taken. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> The "WARNING" in the comments is really welcome... Reviewed-by: Laurent Vivier <laurent@vivier.eu> > --- > linux-user/qemu.h | 5 +++++ > linux-user/signal.c | 10 +++++++++- > linux-user/syscall.c | 5 ----- > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 56f29c3..e8a5aed 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -20,6 +20,11 @@ > > #define THREAD __thread > > +/* This is the size of the host kernel's sigset_t, needed where we make > + * direct system calls that take a sigset_t pointer and a size. > + */ > +#define SIGSET_T_SIZE (_NSIG / 8) > + > /* This struct is used to hold certain information about the image. > * Basically, it replicates in user space what would be certain > * task_struct fields in the kernel > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 37fb60f..26e5e94 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -639,8 +639,16 @@ static void host_signal_handler(int host_signum, siginfo_t *info, > * code in case the guest code provokes one in the window between > * now and it getting out to the main loop. Signals will be > * unblocked again in process_pending_signals(). > + * > + * WARNING: we cannot use sigfillset() here because the uc_sigmask > + * field is a kernel sigset_t, which is much smaller than the > + * libc sigset_t which sigfillset() operates on. Using sigfillset() > + * would write 0xff bytes off the end of the structure and trash > + * data on the struct. > + * We can't use sizeof(uc->uc_sigmask) either, because the libc > + * headers define the struct field with the wrong (too large) type. > */ > - sigfillset(&uc->uc_sigmask); > + memset(&uc->uc_sigmask, 0xff, SIGSET_T_SIZE); > sigdelset(&uc->uc_sigmask, SIGSEGV); > sigdelset(&uc->uc_sigmask, SIGBUS); > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 121b89f..202e387 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -124,11 +124,6 @@ int __clone2(int (*fn)(void *), void *child_stack_base, > #define VFAT_IOCTL_READDIR_BOTH _IOR('r', 1, struct linux_dirent [2]) > #define VFAT_IOCTL_READDIR_SHORT _IOR('r', 2, struct linux_dirent [2]) > > -/* This is the size of the host kernel's sigset_t, needed where we make > - * direct system calls that take a sigset_t pointer and a size. > - */ > -#define SIGSET_T_SIZE (_NSIG / 8) > - > #undef _syscall0 > #undef _syscall1 > #undef _syscall2 >
diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 56f29c3..e8a5aed 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -20,6 +20,11 @@ #define THREAD __thread +/* This is the size of the host kernel's sigset_t, needed where we make + * direct system calls that take a sigset_t pointer and a size. + */ +#define SIGSET_T_SIZE (_NSIG / 8) + /* This struct is used to hold certain information about the image. * Basically, it replicates in user space what would be certain * task_struct fields in the kernel diff --git a/linux-user/signal.c b/linux-user/signal.c index 37fb60f..26e5e94 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -639,8 +639,16 @@ static void host_signal_handler(int host_signum, siginfo_t *info, * code in case the guest code provokes one in the window between * now and it getting out to the main loop. Signals will be * unblocked again in process_pending_signals(). + * + * WARNING: we cannot use sigfillset() here because the uc_sigmask + * field is a kernel sigset_t, which is much smaller than the + * libc sigset_t which sigfillset() operates on. Using sigfillset() + * would write 0xff bytes off the end of the structure and trash + * data on the struct. + * We can't use sizeof(uc->uc_sigmask) either, because the libc + * headers define the struct field with the wrong (too large) type. */ - sigfillset(&uc->uc_sigmask); + memset(&uc->uc_sigmask, 0xff, SIGSET_T_SIZE); sigdelset(&uc->uc_sigmask, SIGSEGV); sigdelset(&uc->uc_sigmask, SIGBUS); diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 121b89f..202e387 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -124,11 +124,6 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #define VFAT_IOCTL_READDIR_BOTH _IOR('r', 1, struct linux_dirent [2]) #define VFAT_IOCTL_READDIR_SHORT _IOR('r', 2, struct linux_dirent [2]) -/* This is the size of the host kernel's sigset_t, needed where we make - * direct system calls that take a sigset_t pointer and a size. - */ -#define SIGSET_T_SIZE (_NSIG / 8) - #undef _syscall0 #undef _syscall1 #undef _syscall2
The kernel and libc have different ideas about what a sigset_t is -- for the kernel it is only _NSIG / 8 bytes in size (usually 8 bytes), but for libc it is much larger, 128 bytes. In most situations the difference doesn't matter, because if you pass a pointer to a libc sigset_t to the kernel it just acts on the first 8 bytes of it, but for the ucontext_t* argument to a signal handler it trips us up. The kernel allocates this ucontext_t on the stack according to its idea of the sigset_t type, but the type of the ucontext_t defined by the libc headers uses the libc type, and so do the manipulator functions like sigfillset(). This means that (1) sizeof(uc->uc_sigmask) is much larger than the actual space used on the stack (2) sigfillset(&uc->uc_sigmask) will write garbage 0xff bytes off the end of the structure, which can trash data that was on the stack before the signal handler was invoked, and may result in a crash after the handler returns To avoid this, we use a memset() of the correct size to fill the signal mask rather than using the libc function. This fixes a problem where we would crash at least some of the time on an i386 host when a signal was taken. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/qemu.h | 5 +++++ linux-user/signal.c | 10 +++++++++- linux-user/syscall.c | 5 ----- 3 files changed, 14 insertions(+), 6 deletions(-)