From 55d72a7eb32858d50ba0777cfde2027d007010b2 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 13 Jun 2016 11:22:05 +0100 Subject: linux-user: Avoid possible misalignment in host_to_target_siginfo() host_to_target_siginfo() is implemented by a combination of host_to_target_siginfo_noswap() followed by tswap_siginfo(). The first of these two functions assumes that the target_siginfo_t it is writing to is correctly aligned, but the pointer passed into host_to_target_siginfo() is directly from the guest and might be misaligned. Use a local variable to avoid this problem. (tswap_siginfo() does now correctly handle a misaligned destination.) We have to add a memset() to host_to_target_siginfo_noswap() to avoid some false positive "may be used uninitialized" warnings from gcc about subfields of the _sifields union if it chooses to inline both tswap_siginfo() and host_to_target_siginfo_noswap() into host_to_target_siginfo(). Signed-off-by: Peter Maydell Reviewed-by: Laurent Vivier Signed-off-by: Peter Maydell --- linux-user/signal.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'linux-user/signal.c') diff --git a/linux-user/signal.c b/linux-user/signal.c index 1dadddf2dd..e2d55fff97 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -278,6 +278,14 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo, tinfo->si_errno = 0; tinfo->si_code = info->si_code; + /* This memset serves two purposes: + * (1) ensure we don't leak random junk to the guest later + * (2) placate false positives from gcc about fields + * being used uninitialized if it chooses to inline both this + * function and tswap_siginfo() into host_to_target_siginfo(). + */ + memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad)); + /* This is awkward, because we have to use a combination of * the si_code and si_signo to figure out which of the union's * members are valid. (Within the host kernel it is always possible @@ -397,8 +405,9 @@ static void tswap_siginfo(target_siginfo_t *tinfo, void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info) { - host_to_target_siginfo_noswap(tinfo, info); - tswap_siginfo(tinfo, tinfo); + target_siginfo_t tgt_tmp; + host_to_target_siginfo_noswap(&tgt_tmp, info); + tswap_siginfo(tinfo, &tgt_tmp); } /* XXX: we support only POSIX RT signals are used. */ -- cgit 1.4.1 From 1d48fdd9d84aab1bd32c1f70947932f5d90f92aa Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 14 Jun 2016 12:49:18 +0100 Subject: linux-user: Don't use sigfillset() on uc->uc_sigmask 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 Reviewed-by: Laurent Vivier Signed-off-by: Riku Voipio --- linux-user/qemu.h | 5 +++++ linux-user/signal.c | 10 +++++++++- linux-user/syscall.c | 5 ----- 3 files changed, 14 insertions(+), 6 deletions(-) (limited to 'linux-user/signal.c') diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 56f29c35b5..e8a5aede95 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 e2d55fff97..9d980456ec 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -636,8 +636,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 3dfaea9c8e..5166ff9b62 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -123,11 +123,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 -- cgit 1.4.1