Message ID | 1348935086-11336-3-git-send-email-abarcelo@ac.upc.edu |
---|---|
State | New |
Headers | show |
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
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; + if (set) { + sigdelset(&temp, SIGSEGV); + } + ret = sigprocmask(how, &temp, oldset); + sigaddset(oldset, SIGSEGV); + return ret; } void process_pending_signals(CPUArchState *cpu_env)