linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Will Drewry <wad@chromium.org>
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-doc@vger.kernel.org, kernel-hardening@lists.openwall.com,
	netdev@vger.kernel.org, x86@kernel.org, arnd@arndb.de,
	davem@davemloft.net, hpa@zytor.com, mingo@redhat.com,
	peterz@infradead.org, rdunlap@xenotime.net,
	mcgrathr@chromium.org, tglx@linutronix.de, luto@mit.edu,
	eparis@redhat.com, serge.hallyn@canonical.com, djm@mindrot.org,
	scarybeasts@gmail.com, indan@nul.nu, pmoore@redhat.com,
	akpm@linux-foundation.org, corbet@lwn.net,
	eric.dumazet@gmail.com, markus@chromium.org,
	coreyb@linux.vnet.ibm.com, keescook@chromium.org
Subject: Re: [PATCH v11 08/12] signal, x86: add SIGSYS info and make it synchronous.
Date: Tue, 28 Feb 2012 17:02:57 +0100	[thread overview]
Message-ID: <20120228160257.GB3664@redhat.com> (raw)
In-Reply-To: <CABqD9hbBj_c1qxD7N0ed3LMy_DXn-163LuNc1E7mSZbGO7oqyw@mail.gmail.com>

On 02/27, Will Drewry wrote:
>
> On Mon, Feb 27, 2012 at 11:22 AM, Oleg Nesterov <oleg@redhat.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.


  reply	other threads:[~2012-02-28 16:10 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-25  3:21 [PATCH v11 01/12] sk_run_filter: add support for custom load_pointer Will Drewry
2012-02-25  3:21 ` [PATCH v11 02/12] net/compat.c,linux/filter.h: share compat_sock_fprog Will Drewry
2012-02-25  3:21 ` [PATCH v11 03/12] seccomp: kill the seccomp_t typedef Will Drewry
2012-02-25  3:21 ` [PATCH v11 04/12] asm/syscall.h: add syscall_get_arch Will Drewry
2012-02-25  3:21 ` [PATCH v11 05/12] arch/x86: add syscall_get_arch to syscall.h Will Drewry
2012-02-25  3:21 ` [PATCH v11 06/12] seccomp: add system call filtering using BPF Will Drewry
2012-02-26 20:28   ` Kees Cook
2012-02-27 16:23     ` Will Drewry
2012-02-27 16:49       ` Eric Paris
2012-02-27 18:55         ` Kees Cook
2012-02-27 19:25           ` Eric Paris
2012-02-27 20:00             ` Kees Cook
2012-02-27 20:34               ` Eric Paris
2012-02-27 20:49                 ` Kees Cook
2012-02-27 17:09   ` Oleg Nesterov
2012-02-27 19:54     ` Will Drewry
2012-02-27 20:15       ` Kees Cook
2012-02-28 15:13       ` Oleg Nesterov
2012-02-28 17:18         ` Will Drewry
2012-02-28  6:51   ` Indan Zupancic
2012-02-28  7:52     ` Kees Cook
2012-02-28 17:17     ` Will Drewry
2012-02-28 17:47       ` Markus Gutschke
2012-02-25  3:21 ` [PATCH v11 07/12] seccomp: add SECCOMP_RET_ERRNO Will Drewry
2012-02-25 20:20   ` Kees Cook
2012-02-27 16:22     ` Will Drewry
2012-02-27 17:11   ` Oleg Nesterov
2012-02-27 18:09     ` Kees Cook
2012-02-27 18:14       ` Oleg Nesterov
2012-02-27 18:35         ` Andrew Lutomirski
2012-02-27 19:14           ` Kees Cook
2012-02-27 19:54             ` Will Drewry
2012-02-25  3:21 ` [PATCH v11 08/12] signal, x86: add SIGSYS info and make it synchronous Will Drewry
2012-02-27 17:22   ` Oleg Nesterov
2012-02-27 17:34     ` Roland McGrath
2012-02-27 18:08       ` Oleg Nesterov
2012-02-27 20:24     ` Will Drewry
2012-02-28 16:02       ` Oleg Nesterov [this message]
2012-02-28 17:06         ` Will Drewry
2012-02-25  3:21 ` [PATCH v11 09/12] seccomp: Add SECCOMP_RET_TRAP Will Drewry
2012-02-25  3:21 ` [PATCH v11 10/12] ptrace,seccomp: Add PTRACE_SECCOMP support Will Drewry
2012-02-27 17:54   ` Oleg Nesterov
2012-02-27 19:47     ` Will Drewry
2012-02-28 16:43       ` Oleg Nesterov
2012-02-28 17:04         ` Will Drewry
2012-02-28 18:34           ` Will Drewry
2012-02-29 16:14             ` Oleg Nesterov
2012-02-29 16:33               ` Will Drewry
2012-02-29 17:09                 ` Oleg Nesterov
2012-02-29 17:41                   ` Roland McGrath
2012-02-29 17:51                     ` Will Drewry
2012-02-25  3:21 ` [PATCH v11 11/12] x86: Enable HAVE_ARCH_SECCOMP_FILTER Will Drewry
2012-02-25  3:21 ` [PATCH v11 12/12] Documentation: prctl/seccomp_filter Will Drewry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120228160257.GB3664@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=corbet@lwn.net \
    --cc=coreyb@linux.vnet.ibm.com \
    --cc=davem@davemloft.net \
    --cc=djm@mindrot.org \
    --cc=eparis@redhat.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hpa@zytor.com \
    --cc=indan@nul.nu \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@mit.edu \
    --cc=markus@chromium.org \
    --cc=mcgrathr@chromium.org \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmoore@redhat.com \
    --cc=rdunlap@xenotime.net \
    --cc=scarybeasts@gmail.com \
    --cc=serge.hallyn@canonical.com \
    --cc=tglx@linutronix.de \
    --cc=wad@chromium.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).