From: Jann Horn <jannh@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
Tycho Andersen <tycho@tycho.ws>,
kernel list <linux-kernel@vger.kernel.org>,
containers@lists.linux-foundation.org,
Linux API <linux-api@vger.kernel.org>,
Oleg Nesterov <oleg@redhat.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
"Serge E. Hallyn" <serge@hallyn.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Tyler Hicks <tyhicks@canonical.com>,
suda.akihiro@lab.ntt.co.jp, "Tobin C. Harding" <me@tobin.cc>
Subject: Re: [PATCH v4 1/4] seccomp: add a return code to trap to userspace
Date: Sat, 23 Jun 2018 00:27:43 +0200 [thread overview]
Message-ID: <CAG48ez3gobTL5mUnZKjhLedotZx49nGrxYKKud5_7+512PaOFw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jKWQCDzXPdTz=+bxT9+4enaBvgYfMseUiGti+t1KwZT8g@mail.gmail.com>
On Fri, Jun 22, 2018 at 11:51 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jun 22, 2018 at 11:09 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > One possible extra issue: IIRC /proc/.../mem uses FOLL_FORCE, which is not what we want here.
Uuugh, I forgot about that.
> > How about just adding an explicit “read/write the seccomp-trapped task’s memory” primitive? That should be easier than a “open mem fd” primitive.
>
> Uuugh. Can we avoid adding another "read/write remote process memory"
> interface? The point of this series was to provide a lightweight
> approach to what should normally be possible via the existing
> seccomp+ptrace interface. I do like Jann's context idea, but I agree
> with Andy: it can't be a handle to /proc/$pid/mem, since it's
> FOLL_FORCE. Is there any other kind of process context id we can use
> for this instead of pid? There was once an idea of pid-fd but it never
> landed... This would let us get rid of the "id" in the structure too.
> And if that existed, we could make process_vm_*v() safer too (taking a
> pid-fd instead of a pid).
Or make a duplicate of /proc/$pid/mem that only differs in whether it
sets FOLL_FORCE? The code is basically already there... something like
this:
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 80aa42506b8b..e8a6a63046da 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -762,6 +762,8 @@ static int mem_open(struct inode *inode, struct file *file)
return ret;
}
+static const struct file_operations proc_mem_operations;
+
static ssize_t mem_rw(struct file *file, char __user *buf,
size_t count, loff_t *ppos, int write)
{
@@ -782,7 +784,10 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
if (!mmget_not_zero(mm))
goto free;
- flags = FOLL_FORCE | (write ? FOLL_WRITE : 0);
+ flags = (write ? FOLL_WRITE : 0);
+ if (file->f_op == &proc_mem_operations) {
+ flags |= FOLL_FORCE;
+ }
while (count > 0) {
int this_len = min_t(int, count, PAGE_SIZE);
@@ -861,6 +866,14 @@ static const struct file_operations proc_mem_operations = {
.release = mem_release,
};
+static const struct file_operations proc_mem_noforce_operations = {
+ .llseek = mem_lseek,
+ .read = mem_read,
+ .write = mem_write,
+ .open = mem_open,
+ .release = mem_release,
+};
+
static int environ_open(struct inode *inode, struct file *file)
{
return __mem_open(inode, file, PTRACE_MODE_READ);
@@ -2916,6 +2929,7 @@ static const struct pid_entry tgid_base_stuff[] = {
REG("numa_maps", S_IRUGO, proc_pid_numa_maps_operations),
#endif
REG("mem", S_IRUSR|S_IWUSR, proc_mem_operations),
+ REG("mem_noforce", S_IRUSR|S_IWUSR, proc_mem_noforce_operations),
LNK("cwd", proc_cwd_link),
LNK("root", proc_root_link),
LNK("exe", proc_exe_link),
@@ -3302,6 +3316,7 @@ static const struct pid_entry tid_base_stuff[] = {
REG("numa_maps", S_IRUGO, proc_tid_numa_maps_operations),
#endif
REG("mem", S_IRUSR|S_IWUSR, proc_mem_operations),
+ REG("mem_noforce",S_IRUSR|S_IWUSR, proc_mem_noforce_operations),
LNK("cwd", proc_cwd_link),
LNK("root", proc_root_link),
LNK("exe", proc_exe_link),
next prev parent reply other threads:[~2018-06-22 22:28 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-21 22:04 [PATCH v4 0/4] seccomp trap to userspace Tycho Andersen
2018-06-21 22:04 ` [PATCH v4 1/4] seccomp: add a return code to " Tycho Andersen
2018-06-21 23:21 ` Jann Horn
2018-06-22 0:58 ` Tycho Andersen
2018-06-22 1:28 ` Jann Horn
2018-06-22 1:39 ` Tycho Andersen
2018-06-22 14:40 ` Jann Horn
2018-06-22 15:15 ` Tycho Andersen
2018-06-22 16:24 ` Jann Horn
2018-06-22 18:09 ` Andy Lutomirski
2018-06-22 21:51 ` Kees Cook
2018-06-22 22:27 ` Jann Horn [this message]
2018-06-26 1:32 ` Tycho Andersen
2018-06-26 2:00 ` Andy Lutomirski
2018-06-21 22:04 ` [PATCH v4 2/4] seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE Tycho Andersen
2018-06-21 22:04 ` [PATCH v4 3/4] seccomp: add a way to get a listener fd from ptrace Tycho Andersen
2018-06-21 22:48 ` Jann Horn
2018-06-21 23:07 ` Tycho Andersen
2018-06-21 22:04 ` [PATCH v4 4/4] seccomp: add support for passing fds via USER_NOTIF Tycho Andersen
2018-06-21 23:34 ` Jann Horn
2018-06-22 0:51 ` Tycho Andersen
2018-06-22 16:23 ` Jann Horn
2018-06-22 18:21 ` Andy Lutomirski
2018-08-07 2:44 ` [PATCH v4 0/4] seccomp trap to userspace Tycho Andersen
2018-08-07 2:57 ` Andy Lutomirski
2018-08-07 3:30 ` Christian Brauner
2018-08-07 4:19 ` Andy Lutomirski
2018-08-07 12:23 ` Christian Brauner
2018-08-07 14:34 ` James Bottomley
2018-08-10 0:31 ` Dinesh Subhraveti
[not found] ` <CAP4sa4+rODVahad2hW-L3h7k6fkfGBsoCfDfBVuMwp3Aaie2KA@mail.gmail.com>
2018-08-11 2:32 ` Tycho Andersen
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=CAG48ez3gobTL5mUnZKjhLedotZx49nGrxYKKud5_7+512PaOFw@mail.gmail.com \
--to=jannh@google.com \
--cc=christian.brauner@ubuntu.com \
--cc=containers@lists.linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=me@tobin.cc \
--cc=oleg@redhat.com \
--cc=serge@hallyn.com \
--cc=suda.akihiro@lab.ntt.co.jp \
--cc=tycho@tycho.ws \
--cc=tyhicks@canonical.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).