Re: [Qemu-devel] [PATCH 2/2] signal: sigsegv protection on do_sigprocmask

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

On 29 September 2012 17:11, Alex Barcelo <abarcelo@ac.upc.edu> wrote:

> Re: [Qemu-devel] [PATCH 2/2] signal: sigsegv protection on do_sigprocmask

The convention for the initial summary line of a patch is that
it starts with an indication of the subsystem being patched. For
instance, here it might be:

"linux-user: Don't allow guest to block SIGSEGV"

> The sigsegv protection is done by forcing the catch (needed in qemu-user)
> and then taking it off from the return mask (well, adding it in fact)

I'm afraid I couldn't really understand what you're trying to
say in this commit message. Perhaps you could expand it a bit?

(missing Signed-off-by:.)
>
> ---
>  linux-user/signal.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index b8b8268..8764f57 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5468,7 +5468,14 @@ long do_rt_sigreturn(CPUArchState *env)
>   */
>  int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset)
>  {
> -    return sigprocmask(how, set, oldset);
> +    int ret;
> +    sigset_t temp = *set;

This is dereferencing set, which will crash if it is NULL.

> +    if (set) {
> +        sigdelset(&temp, SIGSEGV);
> +    }
> +    ret = sigprocmask(how, &temp, oldset);

You need to pass NULL in here rather than &temp if the 'set'
argument was NULL.

> +    sigaddset(oldset, SIGSEGV);
> +    return ret;
>  }

So, we don't permit the guest to block SIGSEGV; that makes sense.
Does it make sense to always tell the guest SIGSEGV is blocked?
(what this patch currently does).
The other options would be "tell the guest SIGSEGV is never blocked"
and "actually maintain somewhere the state of whether the guest
thinks SIGSEGV is blocked so we can return the right answer".

I'm just wondering which would be generally better for guests
and why you picked this approach rather than one of the other
options. (it might be the best, I haven't thought too hard
about it...)

thanks
-- PMM