Re: [PATCH 02/30] bsd-user/signal.c: implement force_sig_fault

[Peter Maydell <peter.maydell@linaro.org>]

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