From: Daniel Colascione <dancol@google.com>
To: Jann Horn <jannh@google.com>
Cc: "Andy Lutomirski" <luto@kernel.org>,
"Christian Brauner" <christian@brauner.io>,
"Joel Fernandes" <joel@joelfernandes.org>,
"Suren Baghdasaryan" <surenb@google.com>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Sultan Alsawaf" <sultan@kerneltoast.com>,
"Tim Murray" <timmurray@google.com>,
"Michal Hocko" <mhocko@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@android.com>,
"Martijn Coenen" <maco@android.com>,
"Ingo Molnar" <mingo@redhat.com>,
"Peter Zijlstra" <peterz@infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
"open list:ANDROID DRIVERS" <devel@driverdev.osuosl.org>,
kernel-team <kernel-team@android.com>,
"Oleg Nesterov" <oleg@redhat.com>,
"Serge E. Hallyn" <serge@hallyn.com>,
"Kees Cook" <keescook@chromium.org>,
"Jonathan Kowalski" <bl0pbl33p@gmail.com>,
"Linux API" <linux-api@vger.kernel.org>
Subject: Re: pidfd design
Date: Mon, 25 Mar 2019 13:23:21 -0700 [thread overview]
Message-ID: <CAKOZueuNGn31QYpRa5G-OP_S=G3f9TBQ-w27giABn7UVgPQ8Ng@mail.gmail.com> (raw)
In-Reply-To: <CAG48ez3VMjLJBC_F3BxC2sc2s-28NdsrUduR=jX66XH0w2O-Qg@mail.gmail.com>
On Mon, Mar 25, 2019 at 1:14 PM Jann Horn <jannh@google.com> wrote:
>
> On Mon, Mar 25, 2019 at 8:44 PM Andy Lutomirski <luto@kernel.org> wrote:
> > On Wed, Mar 20, 2019 at 12:40 PM Daniel Colascione <dancol@google.com> wrote:
> > > On Wed, Mar 20, 2019 at 12:14 PM Christian Brauner <christian@brauner.io> wrote:
> > > > On Wed, Mar 20, 2019 at 11:58:57AM -0700, Andy Lutomirski wrote:
> > > > > On Wed, Mar 20, 2019 at 11:52 AM Christian Brauner <christian@brauner.io> wrote:
> > > > > >
> > > > > > You're misunderstanding. Again, I said in my previous mails it should
> > > > > > accept pidfds optionally as arguments, yes. But I don't want it to
> > > > > > return the status fds that you previously wanted pidfd_wait() to return.
> > > > > > I really want to see Joel's pidfd_wait() patchset and have more people
> > > > > > review the actual code.
> > > > >
> > > > > Just to make sure that no one is forgetting a material security consideration:
> > > >
> > > > Andy, thanks for commenting!
> > > >
> > > > >
> > > > > $ ls /proc/self
> > > > > attr exe mountinfo projid_map status
> > > > > autogroup fd mounts root syscall
> > > > > auxv fdinfo mountstats sched task
> > > > > cgroup gid_map net schedstat timers
> > > > > clear_refs io ns sessionid timerslack_ns
> > > > > cmdline latency numa_maps setgroups uid_map
> > > > > comm limits oom_adj smaps wchan
> > > > > coredump_filter loginuid oom_score smaps_rollup
> > > > > cpuset map_files oom_score_adj stack
> > > > > cwd maps pagemap stat
> > > > > environ mem personality statm
> > > > >
> > > > > A bunch of this stuff makes sense to make accessible through a syscall
> > > > > interface that we expect to be used even in sandboxes. But a bunch of
> > > > > it does not. For example, *_map, mounts, mountstats, and net are all
> > > > > namespace-wide things that certain policies expect to be unavailable.
> > > > > stack, for example, is a potential attack surface. Etc.
> > >
> > > If you can access these files sources via open(2) on /proc/<pid>, you
> > > should be able to access them via a pidfd. If you can't, you
> > > shouldn't. Which /proc? The one you'd get by mounting procfs. I don't
> > > see how pidfd makes any material changes to anyone's security. As far
> > > as I'm concerned, if a sandbox can't mount /proc at all, it's just a
> > > broken and unsupported configuration.
> >
> > It's not "broken and unsupported". I know of an actual working,
> > deployed container-ish sandbox that does exactly this. I would also
> > guess that quite a few not-at-all-container-like sandboxes work like
> > this. (The obvious seccomp + unshare + pivot_root
> > deny-myself-access-to-lots-of-things trick results in no /proc, which
> > is by dsign.)
> >
> > >
> > > An actual threat model and real thought paid to access capabilities
> > > would help. Almost everything around the interaction of Linux kernel
> > > namespaces and security feels like a jumble of ad-hoc patches added as
> > > afterthoughts in response to random objections.
> >
> > I fully agree. But if you start thinking for real about access
> > capabilities, there's no way that you're going to conclude that a
> > capability to access some process implies a capability to access the
> > settings of its network namespace.
> >
> > >
> > > >> All these new APIs either need to
> > > > > return something more restrictive than a proc dirfd or they need to
> > > > > follow the same rules.
> > >
> >
> > ...
> >
> > > What's special about libraries? How is a library any worse-off using
> > > openat(2) on a pidfd than it would be just opening the file called
> > > "/proc/$apid"?
> >
> > Because most libraries actually work, right now, without /proc. Even
> > libraries that spawn subprocesses. If we make the new API have the
> > property that it doesn't work if you're in a non-root user namespace
> > and /proc isn't mounted, the result will be an utter mess.
> >
> > >
> > > > > Yes, this is unfortunate, but it is indeed the current situation. I
> > > > > suppose that we could return magic restricted dirfds, or we could
> > > > > return things that aren't dirfds and all and have some API that gives
> > > > > you the dirfd associated with a procfd but only if you can see
> > > > > /proc/PID.
> > > >
> > > > What would be your opinion to having a
> > > > /proc/<pid>/handle
> > > > file instead of having a dirfd. Essentially, what I initially proposed
> > > > at LPC. The change on what we currently have in master would be:
> > > > https://gist.github.com/brauner/59eec91550c5624c9999eaebd95a70df
> > >
> > > And how do you propose, given one of these handle objects, getting a
> > > process's current priority, or its current oom score, or its list of
> > > memory maps? As I mentioned in my original email, and which nobody has
> > > addressed, if you don't use a dirfd as your process handle or you
> > > don't provide an easy way to get one of these proc directory FDs, you
> > > need to duplicate a lot of metadata access interfaces.
> >
> > An API that takes a process handle object and an fd pointing at /proc
> > (the root of the proc fs) and gives you back a proc dirfd would do the
> > trick. You could do this with no new kernel features at all if you're
> > willing to read the pid, call openat(2), and handle the races in user
> > code.
>
> This seems like something that might be a good fit for two ioctls?
As an aside, we had a long discussion about why fundamental facilities
like this should be system calls, not ioctls. I think the arguments
still apply.
> One ioctl on procfs roots to translate pidfds into that procfs,
> subject to both the normal lookup permission checks and only working
> if the pidfd has a translation into the procfs:
>
> int proc_root_fd = open("/proc", O_RDONLY);
> int proc_dir_fd = ioctl(proc_root_fd, PROC_PIDFD_TO_PROCFSFD, pidfd);
>
> And one ioctl on procfs directories to translate from PGIDs and PIDs to pidfds:
>
> int proc_pgid_fd = open("/proc/self", O_RDONLY);
> int self_pg_pidfd = ioctl(proc_pgid_fd, PROC_PROCFSFD_TO_PIDFD, 0);
> int proc_pid_fd = open("/proc/thread-self", O_RDONLY);
> int self_p_pidfd = ioctl(proc_pid_fd, PROC_PROCFSFD_TO_PIDFD, 0);
>
>
> And then, as you proposed, the new sys_clone() can just return a
> pidfd, and you can convert it into a procfs fd yourself if you want.
I think that's the consensus we reached on the other thread. The
O_DIRECTORY open on /proc/self/fd/mypidfd seems like it'd work well
enough.
next prev parent reply other threads:[~2019-03-25 20:23 UTC|newest]
Thread overview: 124+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-10 20:34 [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android Sultan Alsawaf
2019-03-10 21:03 ` Greg Kroah-Hartman
2019-03-10 21:26 ` Sultan Alsawaf
2019-03-11 16:32 ` Joel Fernandes
2019-03-11 16:37 ` Joel Fernandes
2019-03-11 17:43 ` Michal Hocko
2019-03-11 17:58 ` Sultan Alsawaf
2019-03-11 20:10 ` Suren Baghdasaryan
2019-03-11 20:46 ` Sultan Alsawaf
2019-03-11 21:11 ` Joel Fernandes
2019-03-11 21:46 ` Sultan Alsawaf
2019-03-11 22:15 ` Suren Baghdasaryan
2019-03-11 22:36 ` Sultan Alsawaf
2019-03-12 8:05 ` Michal Hocko
2019-03-12 14:36 ` Suren Baghdasaryan
2019-03-12 15:25 ` Matthew Wilcox
2019-03-12 15:33 ` Michal Hocko
2019-03-12 15:39 ` Michal Hocko
2019-03-12 16:37 ` Sultan Alsawaf
2019-03-12 16:48 ` Michal Hocko
2019-03-12 16:58 ` Michal Hocko
2019-03-12 17:15 ` Suren Baghdasaryan
2019-03-12 17:17 ` Tim Murray
2019-03-12 17:45 ` Sultan Alsawaf
2019-03-12 18:43 ` Tim Murray
2019-03-12 18:50 ` Christian Brauner
2019-03-14 17:47 ` Joel Fernandes
2019-03-14 20:49 ` Sultan Alsawaf
2019-03-15 2:54 ` Joel Fernandes
2019-03-15 3:43 ` Sultan Alsawaf
2019-03-15 3:16 ` Steven Rostedt
2019-03-15 3:45 ` Sultan Alsawaf
2019-03-15 4:36 ` Daniel Colascione
2019-03-15 13:36 ` Joel Fernandes
2019-03-15 15:56 ` Suren Baghdasaryan
2019-03-15 16:12 ` Daniel Colascione
2019-03-15 16:43 ` Steven Rostedt
2019-03-15 17:17 ` Daniel Colascione
2019-03-15 18:03 ` Christian Brauner
2019-03-15 18:13 ` Joel Fernandes
2019-03-15 18:24 ` Christian Brauner
2019-03-15 18:49 ` Joel Fernandes
2019-03-16 17:31 ` Suren Baghdasaryan
2019-03-16 18:00 ` Daniel Colascione
2019-03-16 18:57 ` Christian Brauner
2019-03-16 19:37 ` Suren Baghdasaryan
2019-03-17 1:53 ` Joel Fernandes
2019-03-17 11:42 ` Christian Brauner
2019-03-17 15:40 ` Daniel Colascione
2019-03-18 0:29 ` Christian Brauner
2019-03-18 23:50 ` Joel Fernandes
2019-03-19 22:14 ` Christian Brauner
2019-03-19 22:26 ` Joel Fernandes
2019-03-19 22:48 ` Daniel Colascione
2019-03-19 23:10 ` Christian Brauner
2019-03-20 1:52 ` Joel Fernandes
2019-03-20 2:42 ` pidfd design Daniel Colascione
2019-03-20 3:59 ` Christian Brauner
2019-03-20 7:02 ` Daniel Colascione
2019-03-20 11:33 ` Joel Fernandes
2019-03-20 18:26 ` Christian Brauner
2019-03-20 18:38 ` Daniel Colascione
2019-03-20 18:51 ` Christian Brauner
2019-03-20 18:58 ` Andy Lutomirski
2019-03-20 19:14 ` Christian Brauner
2019-03-20 19:40 ` Daniel Colascione
2019-03-21 17:02 ` Andy Lutomirski
2019-03-25 20:13 ` Jann Horn
2019-03-25 20:23 ` Daniel Colascione [this message]
2019-03-25 23:42 ` Andy Lutomirski
2019-03-25 23:45 ` Christian Brauner
2019-03-26 0:00 ` Andy Lutomirski
2019-03-26 0:12 ` Christian Brauner
2019-03-26 0:24 ` Andy Lutomirski
2019-03-28 9:21 ` Christian Brauner
2019-03-20 19:19 ` Joel Fernandes
2019-03-20 19:29 ` Daniel Colascione
2019-03-24 14:44 ` Serge E. Hallyn
2019-03-24 18:48 ` Joel Fernandes
2019-03-20 19:11 ` Joel Fernandes
2019-05-07 2:16 ` [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android Sultan Alsawaf
2019-05-07 7:04 ` Greg Kroah-Hartman
2019-05-07 7:27 ` Sultan Alsawaf
2019-05-07 7:43 ` Greg Kroah-Hartman
2019-05-07 8:12 ` Sultan Alsawaf
2019-05-07 10:58 ` Christian Brauner
2019-05-07 16:28 ` Suren Baghdasaryan
2019-05-07 16:38 ` Christian Brauner
2019-05-07 16:53 ` Sultan Alsawaf
2019-05-07 20:01 ` Suren Baghdasaryan
2019-05-07 18:46 ` Joel Fernandes
2019-05-07 17:17 ` Sultan Alsawaf
2019-05-07 17:29 ` Greg Kroah-Hartman
2019-05-07 11:09 ` Greg Kroah-Hartman
2019-05-07 12:26 ` Michal Hocko
2019-05-07 15:31 ` Oleg Nesterov
2019-05-07 16:35 ` Sultan Alsawaf
2019-05-09 15:56 ` Oleg Nesterov
2019-05-09 18:33 ` Sultan Alsawaf
2019-05-10 15:10 ` Oleg Nesterov
2019-05-13 16:45 ` Sultan Alsawaf
2019-05-14 16:44 ` Steven Rostedt
2019-05-14 17:31 ` Sultan Alsawaf
2019-05-15 14:58 ` Oleg Nesterov
2019-05-15 17:27 ` Sultan Alsawaf
2019-05-15 18:32 ` Steven Rostedt
2019-05-15 18:52 ` Sultan Alsawaf
2019-05-15 20:09 ` Steven Rostedt
2019-05-16 13:54 ` Oleg Nesterov
2019-03-17 16:35 ` Serge E. Hallyn
2019-03-17 17:11 ` Daniel Colascione
2019-03-17 17:16 ` Serge E. Hallyn
2019-03-17 22:02 ` Suren Baghdasaryan
2019-03-20 20:07 pidfd design Alexey Dobriyan
2019-03-20 20:14 ` Daniel Colascione
2019-03-20 20:39 ` Alexey Dobriyan
2019-03-20 20:47 ` Christian Brauner
2019-03-20 20:50 ` Daniel Colascione
2019-03-20 21:00 ` Christian Brauner
2019-03-22 14:04 ` Michael Tirado
2019-03-25 17:45 ` Linus Torvalds
2019-03-25 16:14 ` Michael Tirado
2019-03-25 20:45 ` Christian Brauner
2019-03-25 18:50 ` Christian Brauner
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='CAKOZueuNGn31QYpRa5G-OP_S=G3f9TBQ-w27giABn7UVgPQ8Ng@mail.gmail.com' \
--to=dancol@google.com \
--cc=arve@android.com \
--cc=bl0pbl33p@gmail.com \
--cc=christian@brauner.io \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=jannh@google.com \
--cc=joel@joelfernandes.org \
--cc=keescook@chromium.org \
--cc=kernel-team@android.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=maco@android.com \
--cc=mhocko@kernel.org \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=serge@hallyn.com \
--cc=sultan@kerneltoast.com \
--cc=surenb@google.com \
--cc=timmurray@google.com \
--cc=tkjos@android.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).