From: Christian Brauner <christian@brauner.io>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
Andy Lutomirski <luto@amacapital.net>,
Andy Lutomirski <luto@kernel.org>,
Florian Weimer <fweimer@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
Jann Horn <jannh@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Oleg Nesterov <oleg@redhat.com>,
cyphar@cyphar.com, Al Viro <viro@zeniv.linux.org.uk>,
Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Daniel Colascione <dancol@google.com>,
Tim Murray <timmurray@google.com>,
linux-man@vger.kernel.org, Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH v2] signal: add procfd_signal() syscall
Date: Fri, 30 Nov 2018 07:56:09 +0100 [thread overview]
Message-ID: <20181130065606.kmilbbq46oeycjp5@brauner.io> (raw)
In-Reply-To: <87y39b2lm2.fsf@xmission.com>
On Thu, Nov 29, 2018 at 11:13:57PM -0600, Eric W. Biederman wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
> > On Thu, Nov 29, 2018 at 9:14 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >> > On Nov 29, 2018, at 11:55 AM, Christian Brauner <christian@brauner.io> wrote:
> >> >> On Thu, Nov 29, 2018 at 11:22:58AM -0800, Andy Lutomirski wrote:
> >> >>> On Thu, Nov 29, 2018 at 11:17 AM Christian Brauner <christian@brauner.io> wrote:
> >> >>>> On November 30, 2018 5:54:18 AM GMT+13:00, Andy Lutomirski <luto@amacapital.net> wrote:
> >> >>
> >> >> The #1 fix would add a copy_siginfo_from_user64() or similar.
> >> >
> >> > Thanks very much! That all helped a bunch already! I'll try to go the
> >> > copy_siginfo_from_user64() way first and see if I can make this work. If
> >> > we do this I would however only want to use it for the new syscall first
> >> > and not change all other signal syscalls over to it too. I'd rather keep
> >> > this patchset focussed and small and do such conversions caused by the
> >> > new approach later. Does that sound reasonable?
> >>
> >> Absolutely. I don’t think we can change old syscalls — the ABI is set in stone.
> >> But for new syscalls, I think the always-64-bit behavior makes sense.
> >
> > It looks like we already have a 'struct signalfd_siginfo' that is defined in a
> > sane architecture-independent way, so I'd suggest we use that.
>
> Unfortunately it isn't maintained very well. What you can
> express with signalfd_siginfo is a subset what you can express with
> siginfo. Many of the linux extensions to siginfo for exception
> information add pointers and have integers right after those pointers.
> Not all of those linux specific extentions for exceptions are handled
> by signalfd_siginfo (it needs new fields).
>
> As originally defined siginfo had the sigval_t union at the end so it
> was perfectly fine on both 32bit and 64bit as it only had a single
> pointer in the structure and the other fields were 32bits in size.
>
> Although I do feel the pain as x86_64 has to deal with 3 versions
> of siginfo. A 64bit one. A 32bit one for ia32. A 32bit one for x32
> with a 64bit si_clock_t.
>
> > We may then also want to make sure that any system call that takes a
> > siginfo has a replacement that takes a signalfd_siginfo, and that this
> > replacement can be used to implement the old version purely in
> > user space.
>
> If you are not implementing CRIU and reinserting exceptions to yourself.
> At most user space wants the ability to implement sigqueue:
>
> AKA:
> sigqueue(pid_t pid, int sig, const union sigval value);
>
> Well sigqueue with it's own si_codes so the following would cover all
> the use cases I know of:
> int sendnewsig(pid_t pid, int sig, int si_code, const union sigval value);
>
> The si_code could even be set to SI_USER to request that the normal
> trusted SI_USER values be filled in. si_code values of < 0 if not
> recognized could reasonably safely be treated as the _rt member of
> the siginfo union. Or perhaps better we error out in such a case.
>
> If we want to be flexible and not have N kinds of system calls that
> is the direction I would go. That is simple, and you don't need any of
> the rest.
>
>
> Alternatively we abandon the mistake of sigqueueinfo and not allow
> a signal number in the arguments that differs from the si_signo in the
> siginfo and allow passing the entire thing unchanged from sender to
> receiver. That is maximum flexibility.
>
> signalfd_siginfo just sucks in practice. It is larger that siginfo 104
> bytes versus 48. We must deliver it to userspace as a siginfo so it
> must be translated. Because of the translation signalfd_siginfo adds
> no flexiblity in practice, because it can not just be passed through.
> Finallay signalfd_siginfo does not have encodings for all of the
> siginfo union members, so it fails to be fully general.
>
> Personally if I was to define signalfd_siginfo today I would make it:
> struct siginfo_subset {
> __u32 sis_signo;
> __s32 sis_errno;
> __s32 sis_code;
> __u32 sis_pad;
> __u32 sis_pid;
> __u32 sis_uid;
> __u64 sis_data (A pointer or integer data field);
> };
>
> That is just 32bytes, and is all that is needed for everything
> except for synchronous exceptions. Oh and that happens to be a proper
> subset of a any sane siginfo layout, on both 32bit and 64bit.
>
> This is one of those rare times where POSIX is sane and what linux
> has implemented is not.
Thanks for the in-depth explanation. So your point is that we are better
off if we stick with siginfo_t instead of struct signalfd_siginfo in
procfd_signal()?
next prev parent reply other threads:[~2018-11-30 6:56 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-20 10:51 [PATCH v2] signal: add procfd_signal() syscall Christian Brauner
2018-11-20 10:51 ` [PATCH v2] procfd_signal.2: document procfd_signal syscall Christian Brauner
2018-11-22 8:00 ` [PATCH v2] signal: add procfd_signal() syscall Serge E. Hallyn
2018-11-22 8:23 ` Aleksa Sarai
2018-11-28 14:05 ` Arnd Bergmann
2018-11-29 12:28 ` Florian Weimer
2018-11-29 16:54 ` Andy Lutomirski
2018-11-29 19:16 ` Christian Brauner
2018-11-29 19:22 ` Andy Lutomirski
2018-11-29 19:55 ` Christian Brauner
2018-11-29 20:14 ` Andy Lutomirski
2018-11-29 21:02 ` Arnd Bergmann
2018-11-29 21:35 ` Christian Brauner
2018-11-29 21:40 ` Arnd Bergmann
2018-11-30 2:40 ` Aleksa Sarai
2018-12-01 1:25 ` Christian Brauner
2018-11-30 5:13 ` Eric W. Biederman
2018-11-30 6:56 ` Christian Brauner [this message]
2018-11-30 11:41 ` Arnd Bergmann
2018-11-30 16:35 ` Andy Lutomirski
2018-11-30 21:57 ` Christian Brauner
2018-11-30 22:09 ` Arnd Bergmann
2018-11-30 22:26 ` Christian Brauner
2018-11-30 23:05 ` Daniel Colascione
2018-11-30 23:12 ` Arnd Bergmann
2018-11-30 23:15 ` Arnd Bergmann
2018-11-30 23:37 ` Christian Brauner
2018-11-30 23:46 ` Andy Lutomirski
2018-12-01 1:20 ` Christian Brauner
2018-11-30 23:53 ` Andy Lutomirski
2018-12-01 8:51 ` Arnd Bergmann
2018-12-01 9:17 ` Christian Brauner
2018-12-01 10:27 ` Arnd Bergmann
2018-12-01 13:41 ` Eric W. Biederman
2018-12-01 14:46 ` Eric W. Biederman
2018-12-01 15:28 ` Eric W. Biederman
2018-12-01 15:52 ` Andy Lutomirski
2018-12-01 16:27 ` Christian Brauner
2018-12-02 0:06 ` Eric W. Biederman
2018-12-02 1:14 ` Andy Lutomirski
2018-12-02 8:52 ` Christian Brauner
2018-11-30 23:52 ` Christian Brauner
2018-12-02 10:03 ` Christian Brauner
2018-12-03 16:57 ` Florian Weimer
2018-12-03 18:02 ` Christian Brauner
2018-12-04 6:03 ` Aleksa Sarai
2018-12-04 12:55 ` Florian Weimer
2018-12-04 13:26 ` Christian Brauner
2018-12-06 18:54 ` Andy Lutomirski
2018-12-06 18:56 ` Florian Weimer
2018-12-06 19:03 ` Christian Brauner
2018-12-25 5:32 ` Lai Jiangshan
2018-12-25 7:11 ` Lai Jiangshan
2018-12-25 12:07 ` Aleksa Sarai
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=20181130065606.kmilbbq46oeycjp5@brauner.io \
--to=christian@brauner.io \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=cyphar@cyphar.com \
--cc=dancol@google.com \
--cc=ebiederm@xmission.com \
--cc=fweimer@redhat.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-man@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=oleg@redhat.com \
--cc=serge@hallyn.com \
--cc=timmurray@google.com \
--cc=viro@zeniv.linux.org.uk \
/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).