From: Daniel Colascione <dancol@google.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Christian Brauner <christian@brauner.io>,
linux-kernel <linux-kernel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
"Serge E. Hallyn" <serge@hallyn.com>,
Jann Horn <jannh@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Oleg Nesterov <oleg@redhat.com>, Aleksa Sarai <cyphar@cyphar.com>,
Al Viro <viro@zeniv.linux.org.uk>,
Linux FS Devel <linux-fsdevel@vger.kernel.org>,
Tim Murray <timmurray@google.com>,
linux-man <linux-man@vger.kernel.org>,
Kees Cook <keescook@chromium.org>,
Florian Weimer <fweimer@redhat.com>,
tglx@linutronix.de, x86@kernel.org
Subject: Re: [PATCH v4] signal: add taskfd_send_signal() syscall
Date: Thu, 6 Dec 2018 12:37:20 -0800 [thread overview]
Message-ID: <CAKOZuevgPv1CbAZF-ha0njdq6rd6QkU7T8qmmERJLsL45CeVzg@mail.gmail.com> (raw)
In-Reply-To: <871s6u9z6u.fsf@xmission.com>
On Thu, Dec 6, 2018 at 12:29 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
> Christian Brauner <christian@brauner.io> writes:
>
> > On Thu, Dec 06, 2018 at 01:17:24PM -0600, Eric W. Biederman wrote:
> >> Christian Brauner <christian@brauner.io> writes:
> >>
> >> > On December 7, 2018 4:01:19 AM GMT+13:00, ebiederm@xmission.com wrote:
> >> >>Christian Brauner <christian@brauner.io> writes:
> >> >>
> >> >>> The kill() syscall operates on process identifiers (pid). After a
> >> >>process
> >> >>> has exited its pid can be reused by another process. If a caller
> >> >>sends a
> >> >>> signal to a reused pid it will end up signaling the wrong process.
> >> >>This
> >> >>> issue has often surfaced and there has been a push [1] to address
> >> >>this
> >> >>> problem.
> >> >>>
> >> >>> This patch uses file descriptors (fd) from proc/<pid> as stable
> >> >>handles on
> >> >>> struct pid. Even if a pid is recycled the handle will not change. The
> >> >>fd
> >> >>> can be used to send signals to the process it refers to.
> >> >>> Thus, the new syscall taskfd_send_signal() is introduced to solve
> >> >>this
> >> >>> problem. Instead of pids it operates on process fds (taskfd).
> >> >>
> >> >>I am not yet thrilled with the taskfd naming.
> >> >
> >> > Userspace cares about what does this thing operate on?
> >> > It operates on processes and threads.
> >> > The most common term people use is "task".
> >> > I literally "polled" ten non-kernel people for that purpose and asked:
> >> > "What term would you use to refer to a process and a thread?"
> >> > Turns out it is task. So if find this pretty apt.
> >> > Additionally, the proc manpage uses task in the exact same way (also see the commit message).
> >> > If you can get behind that name even if feeling it's not optimal it would be great.
> >>
> >> Once I understand why threads and not process groups. I don't see that
> >> logic yet.
> >
> > The point is: userspace takes "task" to be a generic term for processes
> > and tasks. Which is what is important. The term also covers process
> > groups for all that its worth. Most of userspace isn't even aware of
> > that distinction necessarily.
> >
> > fd_send_signal() makes the syscall name meaningless: what is userspace
> > signaling too? The point being that there's a lot more that you require
> > userspace to infer from fd_send_signal() than from task_send_signal()
> > where most people get the right idea right away: "signals to a process
> > or thread".
> >
> >>
> >> >>Is there any plan to support sesssions and process groups?
> >> >
> >> > I don't see the necessity.
> >> > As I said in previous mails:
> >> > we can emulate all interesting signal syscalls with this one.
> >>
> >> I don't know what you mean by all of the interesting signal system
> >> calls. I do know you can not replicate kill(2).
> >
> > [1]: You cannot replicate certain aspects of kill *yet*. We have
> > established this before. If we want process group support later we do
> > have the flags argument to extend the sycall.
>
> Then you have horrible contradiction in the API.
>
> Either the grouping is a property of your file descriptor or the
> grouping comes from the flags argument.
>
> If the grouping is specified in the flags argument then pidfd is the
> proper name for your file descriptors, and the appropriate prefix
> for your system call.
Yes and no. "taskfd" is fine, since even if we do add a
kill-process-group capability, the general facility under discussion
is still *about* tasks in general, so "taskfd" still tells you in a
general sense what the thing does. "pidfd" would be wrong, and for the
same reason that the kernel's "struct pid" is badly-named: the object
being named is a *task*, and signaling a particular task instead of
whatever task happens to be labeled with a particular numeric PID at
the time of all is the whole point of this change.
> If the grouping is a property of your file descriptor it does not make
> sense to talk about using the flags argument later.
>
> Your intention is to add the thread case to support pthreads once the
> process case is sorted out. So this is something that needs to be made
> clear. Did I miss how you plan to handle threads?
>
> And this fundamentally and definitely gets into all of my concerns about
> proper handling of pid_task and PIDTYPE_TGID etc.
To the extent that it's possible, this system call should mimic the
behavior of a signal-send to a positive numeric PID (i.e., a specific
task), so if we change one, we should change both.
next prev parent reply other threads:[~2018-12-06 20:37 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-06 12:18 [PATCH v4] signal: add taskfd_send_signal() syscall Christian Brauner
2018-12-06 12:30 ` Florian Weimer
2018-12-06 12:45 ` Jürg Billeter
2018-12-06 13:12 ` Florian Weimer
2018-12-06 13:18 ` Jürg Billeter
2018-12-06 13:20 ` Florian Weimer
2018-12-06 13:40 ` Eric W. Biederman
2018-12-06 13:44 ` Florian Weimer
2018-12-06 14:27 ` Aleksa Sarai
2018-12-06 14:46 ` Eric W. Biederman
2018-12-06 12:53 ` Christian Brauner
2018-12-06 13:17 ` Florian Weimer
2018-12-06 15:01 ` Eric W. Biederman
2018-12-06 16:17 ` Daniel Colascione
2018-12-06 17:24 ` Eric W. Biederman
2018-12-06 17:41 ` Christian Brauner
2018-12-06 18:30 ` Kees Cook
2018-12-06 22:27 ` Serge E. Hallyn
2018-12-06 17:14 ` Christian Brauner
2018-12-06 19:17 ` Eric W. Biederman
2018-12-06 19:30 ` Christian Brauner
2018-12-06 20:29 ` Eric W. Biederman
2018-12-06 20:37 ` Daniel Colascione [this message]
2018-12-06 22:22 ` Eric W. Biederman
2018-12-06 22:43 ` Daniel Colascione
2018-12-06 21:31 ` Christian Brauner
2018-12-06 21:46 ` Eric W. Biederman
2018-12-06 22:01 ` Daniel Colascione
2018-12-06 22:39 ` Christian Brauner
2018-12-06 23:17 ` Christian Brauner
2018-12-07 0:31 ` Serge E. Hallyn
2018-12-07 0:34 ` Daniel Colascione
2018-12-07 0:59 ` Serge E. Hallyn
2018-12-07 1:39 ` Daniel Colascione
2018-12-07 1:54 ` Christian Brauner
2018-12-07 16:49 ` Serge E. Hallyn
2018-12-07 16:47 ` Serge E. Hallyn
2018-12-08 21:46 ` kbuild test robot
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=CAKOZuevgPv1CbAZF-ha0njdq6rd6QkU7T8qmmERJLsL45CeVzg@mail.gmail.com \
--to=dancol@google.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=christian@brauner.io \
--cc=cyphar@cyphar.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=tglx@linutronix.de \
--cc=timmurray@google.com \
--cc=viro@zeniv.linux.org.uk \
--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).