From: Andy Lutomirski <luto@kernel.org>
To: Daniel Colascione <dancol@google.com>
Cc: Christian Brauner <christian@brauner.io>,
"Eric W. Biederman" <ebiederm@xmission.com>,
LKML <linux-kernel@vger.kernel.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
Jann Horn <jannh@google.com>, Andrew Lutomirski <luto@kernel.org>,
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>,
Linux API <linux-api@vger.kernel.org>,
Tim Murray <timmurray@google.com>,
Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH] proc: allow killing processes via file descriptors
Date: Sun, 18 Nov 2018 07:38:09 -0800 [thread overview]
Message-ID: <CALCETrUd2Y2MmO8Qx8aUvQDOTNiyj35Y7bctPBZe0p2i1EUiLw@mail.gmail.com> (raw)
In-Reply-To: <CAKOZuet3s0WxxPgrtrNkq8YiOWhYAiK6yEio6PKVV9J_H4_TSA@mail.gmail.com>
On Sun, Nov 18, 2018 at 5:59 AM Daniel Colascione <dancol@google.com> wrote:
>
> I had been led to believe that the proposal would be a comprehensive
> process API, not an ioctl basically equivalent to my previous patch.
> If you had a more comprehensive proposal, please just share it on LKML
> instead of limiting the discussion to those able to attend these
> various conferences. If there's some determined opposition to a
> general new process API, this opposition needs a fair and full airing,
> as not everyone can attend these conferences.
>
> On Sun, Nov 18, 2018 at 3:17 AM, Christian Brauner <christian@brauner.io> wrote:
> > With this patch an open() call on /proc/<pid> will give userspace a handle
> > to struct pid of the process associated with /proc/<pid>. This allows to
> > maintain a stable handle on a process.
> > I have been discussing various approaches extensively during technical
> > conferences this year culminating in a long argument with Eric at Linux
> > Plumbers. The general consensus was that having a handle on a process
> > will be something that is very simple and easy to maintain
>
> ioctls are the opposite of "easy to maintain". Their
> file-descriptor-specific behavior makes it difficult to use the things
> safely. If you want to take this approach, please make a new system
> call. An ioctl is just a system call with a very strange spelling and
> unfortunate collision semantics.
>
> > with the
> > option of being extensible via a more advanced api if the need arises.
>
> The need *has* arisen; see my exithand patch.
>
> > I
> > believe that this patch is the most simple, dumb, and therefore
> > maintainable solution.
> >
> > The need for this has arisen in order to reliably kill a process without
> > running into issues of the pid being recycled as has been described in the
> > rejected patch [1].
>
> That patch was not "rejected". It was tabled pending the more
> comprehensive process API proposal that was supposed to have emerged.
> This patch is just another variant of the sort of approach we
> discussed on that patch's thread here. As I mentioned on that thread,
> the right approach option is a new system call, not an ioctl.
>
> To fulfill the need described in that patchset a new
> > ioctl() PROC_FD_SIGNAL is added. It can be used to send signals to a
> > process via a file descriptor:
> >
> > int fd = open("/proc/1234", O_DIRECTORY | O_CLOEXEC);
> > ioctl(fd, PROC_FD_SIGNAL, SIGKILL);
> > close(fd);
> >
> > Note, the stable handle will allow us to carefully extend this feature in
> > the future.
>
> We still need the ability to synchronously wait on a process's death,
> as in my patch set. I will be refreshing that patch set.
I fully agree that a more comprehensive, less expensive API for
managing processes would be nice. But I also think that this patch
(using the directory fd and ioctl) is better from a security
perspective than using a new file in /proc.
I have an old patch to make proc directory fds pollable:
https://lore.kernel.org/patchwork/patch/345098/
That patch plus the one in this thread might make a nice addition to
the kernel even if we expect something much better to come along
later.
This patch that uses ioctl
next prev parent reply other threads:[~2018-11-18 15:38 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-18 11:17 [PATCH] proc: allow killing processes via file descriptors Christian Brauner
2018-11-18 13:59 ` Daniel Colascione
2018-11-18 15:38 ` Andy Lutomirski [this message]
2018-11-18 15:53 ` Daniel Colascione
2018-11-18 16:17 ` Andy Lutomirski
2018-11-18 16:29 ` Daniel Colascione
2018-11-18 17:13 ` Andy Lutomirski
2018-11-18 17:17 ` Daniel Colascione
2018-11-18 17:43 ` Eric W. Biederman
2018-11-18 17:45 ` Andy Lutomirski
2018-11-18 17:56 ` Daniel Colascione
2018-11-18 16:33 ` Randy Dunlap
2018-11-18 16:48 ` Daniel Colascione
2018-11-18 17:09 ` Andy Lutomirski
2018-11-18 17:24 ` Daniel Colascione
2018-11-18 17:42 ` Andy Lutomirski
2018-11-18 17:51 ` Daniel Colascione
2018-11-18 18:28 ` Andy Lutomirski
2018-11-18 18:43 ` Daniel Colascione
2018-11-18 19:05 ` Aleksa Sarai
2018-11-18 19:44 ` Daniel Colascione
2018-11-18 20:15 ` Christian Brauner
2018-11-18 20:21 ` Daniel Colascione
2018-11-18 20:28 ` Andy Lutomirski
2018-11-18 20:32 ` Daniel Colascione
2018-11-19 1:43 ` Andy Lutomirski
2018-11-18 20:43 ` Christian Brauner
2018-11-18 20:54 ` Daniel Colascione
2018-11-18 21:23 ` Christian Brauner
2018-11-18 21:30 ` Christian Brauner
2018-11-19 0:31 ` Daniel Colascione
2018-11-19 0:40 ` Christian Brauner
2018-11-19 0:09 ` Aleksa Sarai
2018-11-19 0:53 ` Daniel Colascione
2018-11-19 1:16 ` Daniel Colascione
2018-11-19 16:13 ` Dmitry Safonov
2018-11-19 16:26 ` [PATCH] proc: allow killing processes via file descriptors (Larger pids) Eric W. Biederman
2018-11-19 16:27 ` [PATCH] proc: allow killing processes via file descriptors Daniel Colascione
2018-11-19 20:21 ` Aleksa Sarai
2018-11-19 2:47 ` Al Viro
2018-11-19 3:01 ` Andy Lutomirski
2018-11-18 17:41 ` Christian Brauner
2018-11-18 17:44 ` Andy Lutomirski
2018-11-18 18:07 ` Daniel Colascione
2018-11-18 18:15 ` Andy Lutomirski
2018-11-18 18:31 ` Daniel Colascione
2018-11-18 19:24 ` Christian Brauner
2018-11-19 0:08 ` Aleksa Sarai
2018-11-19 1:14 ` Daniel Colascione
2018-11-18 16:03 ` Daniel Colascione
2018-11-19 10:56 ` kbuild test robot
2018-11-19 14:15 ` David Laight
2018-11-19 15:49 ` Dave Martin
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=CALCETrUd2Y2MmO8Qx8aUvQDOTNiyj35Y7bctPBZe0p2i1EUiLw@mail.gmail.com \
--to=luto@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=christian@brauner.io \
--cc=cyphar@cyphar.com \
--cc=dancol@google.com \
--cc=ebiederm@xmission.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=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).