From: Joel Fernandes <joel@joelfernandes.org>
To: Daniel Colascione <dancol@google.com>
Cc: Aleksa Sarai <cyphar@cyphar.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Tim Murray <timmurray@google.com>,
Suren Baghdasaryan <surenb@google.com>
Subject: Re: [RFC PATCH] Implement /proc/pid/kill
Date: Tue, 30 Oct 2018 17:57:23 -0700 [thread overview]
Message-ID: <20181031005723.GD224709@google.com> (raw)
In-Reply-To: <CAKOZueuBdY_b=CVk29pzn0NAt9sJtzr6yqx9Y00WCppCm1JFWQ@mail.gmail.com>
On Tue, Oct 30, 2018 at 11:10:47PM +0000, Daniel Colascione wrote:
> On Tue, Oct 30, 2018 at 10:33 PM, Joel Fernandes <joel@joelfernandes.org> wrote:
> > On Wed, Oct 31, 2018 at 09:23:39AM +1100, Aleksa Sarai wrote:
> >> On 2018-10-30, Joel Fernandes <joel@joelfernandes.org> wrote:
> >> > On Wed, Oct 31, 2018 at 07:45:01AM +1100, Aleksa Sarai wrote:
> >> > [...]
> >> > > > > (Unfortunately
> >> > > > > there are lots of things that make it a bit difficult to use /proc/$pid
> >> > > > > exclusively for introspection of a process -- especially in the context
> >> > > > > of containers.)
> >> > > >
> >> > > > Tons of things already break without a working /proc. What do you have in mind?
> >> > >
> >> > > Heh, if only that was the only blocker. :P
> >> > >
> >> > > The basic problem is that currently container runtimes either depend on
> >> > > some non-transient on-disk state (which becomes invalid on machine
> >> > > reboots or dead processes and so on), or on long-running processes that
> >> > > keep file descriptors required for administration of a container alive
> >> > > (think O_PATH to /dev/pts/ptmx to avoid malicious container filesystem
> >> > > attacks). Usually both.
> >> > >
> >> > > What would be really useful would be having some way of "hiding away" a
> >> > > mount namespace (of the pid1 of the container) that has all of the
> >> > > information and bind-mounts-to-file-descriptors that are necessary for
> >> > > administration. If the container's pid1 dies all of the transient state
> >> > > has disappeared automatically -- because the stashed mount namespace has
> >> > > died. In addition, if this was done the way I'm thinking with (and this
> >> > > is the contentious bit) hierarchical mount namespaces you could make it
> >> > > so that the pid1 could not manipulate its current mount namespace to
> >> > > confuse the administrative process. You would also then create an
> >> > > intermediate user namespace to help with several race conditions (that
> >> > > have caused security bugs like CVE-2016-9962) we've seen when joining
> >> > > containers.
> >> > >
> >> > > Unfortunately this all depends on hierarchical mount namespaces (and
> >> > > note that this would just be that NS_GET_PARENT gives you the mount
> >> > > namespace that it was created in -- I'm not suggesting we redesign peers
> >> > > or anything like that). This makes it basically a non-starter.
> >> > >
> >> > > But if, on top of this ground-work, we then referenced containers
> >> > > entirely via an fd to /proc/$pid then you could also avoid PID reuse
> >> > > races (as well as being able to find out implicitly whether a container
> >> > > has died thanks to the error semantics of /proc/$pid). And that's the
> >> > > way I would suggest doing it (if we had these other things in place).
> >> >
> >> > I didn't fully follow exactly what you mean. If you can explain for the
> >> > layman who doesn't know much experience with containers..
> >> >
> >> > Are you saying that keeping open a /proc/$pid directory handle is not
> >> > sufficient to prevent PID reuse while the proc entries under /proc/$pid are
> >> > being looked into? If its not sufficient, then isn't that a bug? If it is
> >> > sufficient, then can we not just keep the handle open while we do whatever we
> >> > want under /proc/$pid ?
> >>
> >> Sorry, I went on a bit of a tangent about various internals of container
> >> runtimes. My main point is that I would love to use /proc/$pid because
> >> it makes reuse handling very trivial and is always correct, but that
> >> there are things which stop us from being able to use it for everything
> >> (which is what my incoherent rambling was on about).
> >
> > Ok thanks. So I am guessing if the following sequence works, then Dan's patch is not
> > needed.
> >
> > 1. open /proc/<pid> directory
> > 2. inspect /proc/<pid> or do whatever with <pid>
> > 3. Issue the kill on <pid>
> > 4. Close the /proc/<pid> directory opened in step 1.
> >
> > So unless I missed something, the above sequence will not cause any PID reuse
> > races.
>
> Keeping a /proc/$PID directory file descriptor open does not prevent
> $PID being used to name some other process. If it could, you could
> pretty quickly fill a whole system's process table. See the program
> below, which demonstrates the PID collision.
I know. We both were not sure about that earlier, that's why I requested you
to write the program when we were privately chatting. Now I'm sure because
Aleska answered that and the you program you wrote showed that too.
I wonder if this cannot be plumbed by just making the /proc/$PID directory
opens hold a reference to task_struct (and a reference to whatever else is
supposed to prevent the PID from getting reused), instead of introducing a
brand new API.
> I think Aleksa's larger point is that it's useful to treat processes
> as other file-descriptor-named, poll-able, wait-able resources.
> Consistency is important. A process is just another system resource,
> and like any other system resource, you should be open to hold a file
> descriptor to it and do things to that process via that file
> descriptor. The precise form of this process-handle FD is up for
> debate. The existing /proc/$PID directory FD is a good candidate for a
> process handle FD, since it does almost all of what's needed. But
> regardless of what form a process handle FD takes, we need it. I don't
> see a case for continuing to treat processes in a non-unixy,
> non-file-descriptor-based manner.
So wait, how is that supposed to address what you're now saying above
"quickly fill a whole process table"? You either want this, or you don't :)
next prev parent reply other threads:[~2018-10-31 0:57 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-29 22:10 [RFC PATCH] Implement /proc/pid/kill Daniel Colascione
2018-10-30 3:21 ` Joel Fernandes
2018-10-30 8:50 ` Daniel Colascione
2018-10-30 10:39 ` Christian Brauner
2018-10-30 10:40 ` Christian Brauner
2018-10-30 10:48 ` Daniel Colascione
2018-10-30 11:04 ` Christian Brauner
2018-10-30 11:12 ` Daniel Colascione
2018-10-30 11:19 ` Christian Brauner
2018-10-31 5:00 ` Eric W. Biederman
2018-10-30 17:01 ` Joel Fernandes
2018-10-30 5:00 ` Aleksa Sarai
2018-10-30 9:05 ` Daniel Colascione
2018-10-30 20:45 ` Aleksa Sarai
2018-10-30 21:42 ` Joel Fernandes
2018-10-30 22:23 ` Aleksa Sarai
2018-10-30 22:33 ` Joel Fernandes
2018-10-30 22:49 ` Aleksa Sarai
2018-10-31 0:42 ` Joel Fernandes
2018-10-31 1:59 ` Daniel Colascione
2018-10-30 23:10 ` Daniel Colascione
2018-10-30 23:23 ` Christian Brauner
2018-10-30 23:55 ` Daniel Colascione
2018-10-31 2:56 ` Aleksa Sarai
2018-10-31 4:24 ` Joel Fernandes
2018-11-01 20:40 ` Joel Fernandes
2018-11-02 9:46 ` Christian Brauner
2018-11-02 14:34 ` Serge E. Hallyn
2018-10-31 0:57 ` Joel Fernandes [this message]
2018-10-31 1:56 ` Daniel Colascione
2018-10-31 4:47 ` Eric W. Biederman
2018-10-31 4:44 ` Eric W. Biederman
2018-10-31 12:44 ` Oleg Nesterov
2018-10-31 13:27 ` Daniel Colascione
2018-10-31 15:10 ` Oleg Nesterov
2018-10-31 15:16 ` Daniel Colascione
2018-10-31 15:49 ` Oleg Nesterov
2018-11-01 11:53 ` David Laight
2018-11-01 15:50 ` Daniel Colascione
2018-10-31 14:37 ` [PATCH v2] " Daniel Colascione
2018-10-31 15:05 ` Joel Fernandes
2018-10-31 17:33 ` Aleksa Sarai
2018-10-31 21:47 ` Joel Fernandes
2018-10-31 15:59 ` [PATCH v3] " Daniel Colascione
2018-10-31 17:54 ` Tycho Andersen
2018-10-31 18:00 ` Daniel Colascione
2018-10-31 18:17 ` Tycho Andersen
2018-10-31 19:33 ` Daniel Colascione
2018-10-31 20:06 ` Tycho Andersen
2018-11-01 11:33 ` David Laight
2018-11-12 1:19 ` Eric W. Biederman
2018-10-31 16:22 ` [RFC PATCH] " Jann Horn
2018-11-01 4:53 ` Andy Lutomirski
2018-11-12 23:13 ` Pavel Machek
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=20181031005723.GD224709@google.com \
--to=joel@joelfernandes.org \
--cc=cyphar@cyphar.com \
--cc=dancol@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=surenb@google.com \
--cc=timmurray@google.com \
/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).