LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Andy Lutomirski <luto@kernel.org>
Cc: christian@brauner.io,
	"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: Fri, 30 Nov 2018 23:09:58 +0100
Message-ID: <CAK8P3a1isSRCd-OLJK15C+gSJdpeR=zEXghc8+AMM5QYYw3Tyg@mail.gmail.com> (raw)
In-Reply-To: <CALCETrW2aphWwEY4=hUwo_JBCvkQyMjxzxGd9FCW017kMLaMOQ@mail.gmail.com>

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.
>
> If everyone else is okay with it, I can get on board with three
> variants on x86.  What I can't get on board with is *five* variants on
> x86, which would be:
>
> procfd_signal via int80 / the 32-bit vDSO: the ia32 structure
>
> syscall64 with nr == 335 (presumably): 64-bit

These seem unavoidable

> syscall64 with nr == 548 | 0x40000000: x32
>
> syscall64 with nr == 548: 64-bit entry but in_compat_syscall() ==
> true, behavior is arbitrary
>
> syscall64 with nr == 335 | 0x40000000: x32 entry, but
> in_compat_syscall() == false, behavior is arbitrary

Am I misreading the code? The way I understand it, setting the
0x40000000 bit means that both in_compat_syscall() and
in_x32_syscall become() true, based on

static inline bool in_x32_syscall(void)
{
#ifdef CONFIG_X86_X32_ABI
        if (task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT)
                return true;
#endif
        return false;
}

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?

> This mess isn't really Christian's fault -- it's been there for a
> while, but it's awful and I don't think we want to perpetuate it.

I'm not convinced that not assigning an x32 syscall number
improves the situation, it just means that we now have one
syscall that behaves completely differently from all others,
in that the x32 version requires being called through a
SYSCALL_DEFINE() entry point rather than a
COMPAT_SYSCALL_DEFINE() one, 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().

Of course, the nicest option would be to completely remove
x32 so we can stop worrying about it.

      Arnd

  parent 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 [this message]
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='CAK8P3a1isSRCd-OLJK15C+gSJdpeR=zEXghc8+AMM5QYYw3Tyg@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=akpm@linux-foundation.org \
    --cc=christian@brauner.io \
    --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