| Message ID | 20220109161923.85683-3-imp@bsdimp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bsd-user: upstream our signal implementation | expand |
On Sun, 9 Jan 2022 at 16:23, Warner Losh <imp@bsdimp.com> wrote: > > Start to implement the force_sig_fault code. This currently just calls > queue_signal(). The bsd-user fork version of that will handle this the > synchronous nature of this call. Add signal-common.h to hold signal > helper functions like force_sig_fault. > > Signed-off-by: Stacey Son <sson@FreeBSD.org> > Signed-off-by: Kyle Evans <kevans@freebsd.org> > Signed-off-by: Warner Losh <imp@bsdimp.com> > +/* > + * Force a synchronously taken QEMU_SI_FAULT signal. For QEMU the > + * 'force' part is handled in process_pending_signals(). > + */ > +void force_sig_fault(int sig, int code, abi_ulong addr) > +{ > + CPUState *cpu = thread_cpu; > + CPUArchState *env = cpu->env_ptr; > + target_siginfo_t info = {}; > + > + info.si_signo = sig; > + info.si_errno = 0; > + info.si_code = code; > + info.si_addr = addr; > + queue_signal(env, sig, &info); > +} In the linux-user implementation of this function, we pass in an extra argument to queue_signal(), which is a QEMU_SI_* value (in this case QEMU_SI_FAULT). The reason we do this is that the siginfo_t struct, at least on Linux, is a pain: it has a union, and which field of the union is the valid one is awkward to determine. Within the real Linux kernel, the high bits of si_code are used to track what field of the union is live, but those are masked out before handing the signal to userspace. The effect is that QEMU sometimes has to deal with siginfo_t structures where it knows exactly which field of the union is valid because it generated the structure itself, and sometimes with ones it got from guest userspace where it has to make a best-guess. linux-user code deals with that using the QEMU_SI_* codes: when we generate a siginfo_t ourselves and know what field of the union is valid, we put the QEMU_SI_* into the top part of si_code, and later when we need to byteswap it for the guest we use tswap_siginfo(), which uses that to make a known-correct choice. When we have to byteswap a siginfo_t which we got from guest userspace, we use host_to_target_siginfo_noswap(), which makes a best-guess based on things like the signal number. And when we hand a siginfo_t to guest userspace, we mask out the top part of si_code again, like the real kernel. I'm not sure how the BSDs handle this, but at the moment in this patchset you add both a host_to_target_siginfo_noswap() (patch 16) and a tswap_siginfo(), but you've given them both the guess-which-union-field-is-valid logic. You don't need to address this in this patch series, but I wanted to lay out the logic of why linux-user is doing things the way it does so you can determine whether bsd-user needs to do the same or not. (That might depend on which BSD: judging by the target_siginfo definitions in bsd_user, freebsd puts si_pid/si_uid in top-level struct fields, but netbsd and openbsd put them in sub-fields of the union the same way Linux does. For eg a SIGSEGV sent via kill() you want to swap the pid/uid fields, but for a SIGSEGV generated by QEMU you want to swap the si_addr. The other thing to check is to what extent the BSD kernel ABI lets userspace spoof the si_code field in a siginfo_t: Linux's rt_sigqueueinfo syscall is quite lax in this regard.) Anyway, for this patch: Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 1/10/22 3:18 AM, Warner Losh wrote: > Start to implement the force_sig_fault code. This currently just calls > queue_signal(). The bsd-user fork version of that will handle this the > synchronous nature of this call. Add signal-common.h to hold signal > helper functions like force_sig_fault. > > Signed-off-by: Stacey Son<sson@FreeBSD.org> > Signed-off-by: Kyle Evans<kevans@freebsd.org> > Signed-off-by: Warner Losh<imp@bsdimp.com> > --- > bsd-user/signal-common.h | 14 ++++++++++++++ > bsd-user/signal.c | 18 ++++++++++++++++++ > 2 files changed, 32 insertions(+) > create mode 100644 bsd-user/signal-common.h Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/bsd-user/signal-common.h b/bsd-user/signal-common.h new file mode 100644 index 00000000000..6207417d39e --- /dev/null +++ b/bsd-user/signal-common.h @@ -0,0 +1,14 @@ +/* + * Emulation of BSD signals + * + * Copyright (c) 2013 Stacey Son + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef SIGNAL_COMMON_H +#define SIGNAL_COMMON_H + +void force_sig_fault(int sig, int code, abi_ulong addr); + +#endif diff --git a/bsd-user/signal.c b/bsd-user/signal.c index 05b277c6422..1206d0d728c 100644 --- a/bsd-user/signal.c +++ b/bsd-user/signal.c @@ -19,6 +19,7 @@ #include "qemu/osdep.h" #include "qemu.h" +#include "signal-common.h" /* * Stubbed out routines until we merge signal support from bsd-user @@ -34,6 +35,23 @@ void queue_signal(CPUArchState *env, int sig, target_siginfo_t *info) qemu_log_mask(LOG_UNIMP, "No signal queueing, dropping signal %d\n", sig); } +/* + * Force a synchronously taken QEMU_SI_FAULT signal. For QEMU the + * 'force' part is handled in process_pending_signals(). + */ +void force_sig_fault(int sig, int code, abi_ulong addr) +{ + CPUState *cpu = thread_cpu; + CPUArchState *env = cpu->env_ptr; + target_siginfo_t info = {}; + + info.si_signo = sig; + info.si_errno = 0; + info.si_code = code; + info.si_addr = addr; + queue_signal(env, sig, &info); +} + void signal_init(void) { }