[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120228160257.GB3664@redhat.com>
Date: Tue, 28 Feb 2012 17:02:57 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Will Drewry <wad@...omium.org>
Cc: linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
linux-doc@...r.kernel.org, kernel-hardening@...ts.openwall.com,
netdev@...r.kernel.org, x86@...nel.org, arnd@...db.de,
davem@...emloft.net, hpa@...or.com, mingo@...hat.com,
peterz@...radead.org, rdunlap@...otime.net, mcgrathr@...omium.org,
tglx@...utronix.de, luto@....edu, eparis@...hat.com,
serge.hallyn@...onical.com, djm@...drot.org, scarybeasts@...il.com,
indan@....nu, pmoore@...hat.com, akpm@...ux-foundation.org,
corbet@....net, eric.dumazet@...il.com, markus@...omium.org,
coreyb@...ux.vnet.ibm.com, keescook@...omium.org
Subject: Re: [PATCH v11 08/12] signal, x86: add SIGSYS info and make it
synchronous.
On 02/27, Will Drewry wrote:
>
> On Mon, Feb 27, 2012 at 11:22 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> > On 02/24, Will Drewry wrote:
> >>
> >> To ensure that SIGSYS delivery occurs on return from the triggering
> >> system call, SIGSYS is added to the SYNCHRONOUS_MASK macro.
> >
> > Hmm. Can't understand... please help.
> >
> >> #define SYNCHRONOUS_MASK \
> >> (sigmask(SIGSEGV) | sigmask(SIGBUS) | sigmask(SIGILL) | \
> >> - sigmask(SIGTRAP) | sigmask(SIGFPE))
> >> + sigmask(SIGTRAP) | sigmask(SIGFPE) | sigmask(SIGSYS))
> >
> > Why?
> >
> > SYNCHRONOUS_MASK just tells dequeue_signal() "pick them first".
> > This is needed to make sure that the handler for, say SIGSEGV,
> > can use ucontext->ip as a faulting addr.
>
> I think that Roland covered this. (Since the syscall_rollback was
> called it's nice to let our handler get first go.)
OK, except I do not really understand the "our handler get first go".
Suppose SIGSYS "races" with the pending SIGHUP. With this change
the caller for SIGHUP will be called first. But yes, setup_frame()
will be called for SIGSYS first. Hopefully this is what you want.
> > But seccomp adds info->si_call_addr, this looks unneeded.
>
> True enough. I can drop it.
Hmm. I meant, the change in SYNCHRONOUS_MASK looks unneeded. Please
keep ->si_call_addr, it is much more convenient than ucontext_t in
userspace.
> It'd only be useful if the SIGSYS wasn't
> being forced and the signal was being handled without ucontext_t
> access.
No, force_ doesn't make any difference in this sense...
In short, the patch looks fine to me, but if you resend it may be
you can update the changelog.
Oleg.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
