LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Christian Brauner <christian@brauner.io>
To: Arnd Bergmann <arnd@arndb.de>, Andy Lutomirski <luto@kernel.org>
Cc: "Eric W . Biederman" <ebiederm@xmission.com>,
	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: Sat, 01 Dec 2018 22:17:40 +1300
Message-ID: <9D19D003-EA65-4D05-A3A6-10EA8F506DFF@brauner.io> (raw)
In-Reply-To: <CAK8P3a1OX1Hb17=NbTYqZxgEM-sk5-dh_VeKa0bXJpq=k=KxHA@mail.gmail.com>

On December 1, 2018 9:51:18 PM GMT+13:00, Arnd Bergmann <arnd@arndb.de> wrote:
>On Sat, Dec 1, 2018 at 12:54 AM Andy Lutomirski <luto@kernel.org>
>wrote:
>> On Fri, Nov 30, 2018 at 2:10 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Fri, Nov 30, 2018 at 5:36 PM Andy Lutomirski <luto@kernel.org>
>wrote:
>> > > On Fri, Nov 30, 2018 at 3:41 AM Arnd Bergmann <arnd@arndb.de>
>wrote:
>> > > > siginfo_t as it is now still has a number of other downsides,
>and Andy in
>> > > > particular didn't like the idea of having three new variants on
>x86
>> > > > (depending on how you count). His alternative suggestion of
>having
>> > > > a single syscall entry point that takes a 'signfo_t __user *'
>but interprets
>> > > > it as compat_siginfo depending on
>in_compat_syscall()/in_x32_syscall()
>> > > > should work correctly, but feels wrong to me, or at least
>inconsistent
>> > > > with how we do this elsewhere.
>
>> > The '548 | 0x40000000' part seems to be the only sensible
>> > way to handle x32 here. What exactly would you propose to
>> > avoid defining the other entry points?
>>
>> I would propose that it should be 335 | 0x40000000.  I can't see any
>> reasonable way to teach the kernel to reject 335 | 0x40000000 that
>> wouldn't work just as well to accept it and make it do the right
>> thing.  Currently we accept it and do the *wrong* thing, which is no
>> good.
>>
>> > and we have to
>> > add more complexity to the copy_siginfo_from_user()
>> > implementation to duplicate the hack that exists in
>> > copy_siginfo_from_user32().
>>
>> What hack are you referring to here?
>
>I mean this part:
>
>#ifdef CONFIG_COMPAT
>int copy_siginfo_to_user32(struct compat_siginfo __user *to,
>                           const struct kernel_siginfo *from)
>#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
>{
>        return __copy_siginfo_to_user32(to, from, in_x32_syscall());
>}
>int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
>                       const struct kernel_siginfo *from, bool x32_ABI)
>#endif
>{
>...
>        case SIL_CHLD:
>                new.si_pid    = from->si_pid;
>                new.si_uid    = from->si_uid;
>                new.si_status = from->si_status;
>#ifdef CONFIG_X86_X32_ABI
>                if (x32_ABI) {
>                    new._sifields._sigchld_x32._utime = from->si_utime;
>                    new._sifields._sigchld_x32._stime = from->si_stime;
>                } else
>#endif
>                {
>                        new.si_utime = from->si_utime;
>                        new.si_stime = from->si_stime;
>                }
>                break;
>...
>}
>#endif
>
>If we have a '548 | 0x40000000' entry pointing to
>__x32_compat_sys_procfd_kill, then that will do the right
>thing. If you instead try to have x32 call into the native
>sys_procfd_kill, then copy_siginfo_to_user() will also have
>to know about x32, effectively duplicating that mess above,
>unless you want to also change all users of
>copy_siginfo_to_user32() to use copy_siginfo_to_user()
>and handle all cases in one function.

I've been looking into having siginfo64_t
with the new copy_siginfo_to_user64()
function. It looks like a pretty intricate 
task. Are we sure that we want to go
down this road? I'm not sure that it'll be
worth it. Especially since we force yet
another signal struct on user space.


  reply index

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-20 10:51 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
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 [this message]
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=9D19D003-EA65-4D05-A3A6-10EA8F506DFF@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@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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git