Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1219143 > unrolled thread
| Started by | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| First post | 2015-09-04 18:10 +0200 |
| Last post | 2015-09-04 22:50 +0200 |
| Articles | 20 on this page of 34 — 5 participants |
Back to article view | Back to linux.kernel
c/r of seccomp filters via underlying eBPF Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
[PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd Kees Cook <keescook@chromium.org> - 2015-09-04 22:50 +0200
Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> - 2015-09-05 09:20 +0200
Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 22:50 +0200
[PATCH 4/6] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds Kees Cook <keescook@chromium.org> - 2015-09-04 22:30 +0200
Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 22:40 +0200
Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 23:00 +0200
[PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 23:10 +0200
Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-05 00:50 +0200
Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-05 06:20 +0200
[PATCH 1/6] ebpf: add a seccomp program type Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 22:20 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 23:10 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Kees Cook <keescook@chromium.org> - 2015-09-04 22:40 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Kees Cook <keescook@chromium.org> - 2015-09-04 23:10 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 23:10 +0200
Re: [PATCH 1/6] ebpf: add a seccomp program type Andy Lutomirski <luto@amacapital.net> - 2015-09-05 00:00 +0200
[PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 18:10 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Kees Cook <keescook@chromium.org> - 2015-09-04 22:20 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 22:50 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 23:00 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 23:10 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Kees Cook <keescook@chromium.org> - 2015-09-04 23:00 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Andy Lutomirski <luto@amacapital.net> - 2015-09-04 23:50 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-05 00:30 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Andy Lutomirski <luto@amacapital.net> - 2015-09-05 01:10 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-05 02:30 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Kees Cook <keescook@chromium.org> - 2015-09-05 01:30 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Andy Lutomirski <luto@amacapital.net> - 2015-09-05 02:10 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-04 22:30 +0200
Re: [PATCH 3/6] ebpf: add a way to dump an eBPF program Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-04 22:50 +0200
Page 1 of 2 [1] 2 Next page →
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 18:10 +0200 |
| Subject | c/r of seccomp filters via underlying eBPF |
| Message-ID | <q51IS-70a-7@gated-at.bofh.it> |
Hi all, Here is a set that enables checkpoint restore of the underlying eBPF programs that power seccomp filters via the API we discussed several months ago. A few notes: * We expose prog_id in the ebpf dump as the pointer to the ebpf program in kernel memory, since this is unique. I'm not sure if this is safe. The goal of exposing prog_id is to enable userspace to figure out how the seccomp filters are inherited (i.e. is the filter a result of a fork() or an identical filter installed). This is relevant because SECCOMP_FILTER_FLAG_TSYNC depends on this ancestry. * I'm not sure what the fd argument to seccomp() should look like in the SECCOMP_FILTER_FLAG_EBPF case: it seems ugly either as &fd or fd; any thoughts on this are welcome. * Please double check the first patch. I'm not too framiliar with eBPF, so this is mostly monkey see monkey do. * The last patch is something I discovered while testing this set (I couldn't re-import some filters). I'm not sure if this is indicitave of a wider bug or if the fix is even correct but it Works For Me. Thoughts welcome, Tycho -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 18:10 +0200 |
| Subject | [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd |
| Message-ID | <q51IS-70a-19@gated-at.bofh.it> |
| In reply to | #1219143 |
This is the final bit needed to support seccomp filters created via the bpf
syscall.
One concern with this patch is exactly what the interface should look like
for users, since seccomp()'s second argument is a pointer, we could ask
people to pass a pointer to the fd, but implies we might write to it which
seems impolite. Right now we cast the pointer (and force the user to cast
it), which generates ugly warnings. I'm not sure what the right answer is
here.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
CC: Kees Cook <keescook@chromium.org>
CC: Will Drewry <wad@chromium.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/seccomp.h | 3 +-
include/uapi/linux/seccomp.h | 1 +
kernel/seccomp.c | 70 ++++++++++++++++++++++++++++++++++++--------
3 files changed, 61 insertions(+), 13 deletions(-)
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index d1a86ed..a725dd5 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -3,7 +3,8 @@
#include <uapi/linux/seccomp.h>
-#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC)
+#define SECCOMP_FILTER_FLAG_MASK (\
+ SECCOMP_FILTER_FLAG_TSYNC | SECCOMP_FILTER_FLAG_EBPF)
#ifdef CONFIG_SECCOMP
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 0f238a4..c29a423 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -16,6 +16,7 @@
/* Valid flags for SECCOMP_SET_MODE_FILTER */
#define SECCOMP_FILTER_FLAG_TSYNC 1
+#define SECCOMP_FILTER_FLAG_EBPF (1 << 1)
/*
* All BPF programs must return a 32-bit value.
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index a2c5b32..9c6bea6 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -355,17 +355,6 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
- /*
- * Installing a seccomp filter requires that the task has
- * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
- * This avoids scenarios where unprivileged tasks can affect the
- * behavior of privileged children.
- */
- if (!task_no_new_privs(current) &&
- security_capable_noaudit(current_cred(), current_user_ns(),
- CAP_SYS_ADMIN) != 0)
- return ERR_PTR(-EACCES);
-
/* Allocate a new seccomp_filter */
sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
if (!sfilter)
@@ -509,6 +498,48 @@ static void seccomp_send_sigsys(int syscall, int reason)
info.si_syscall = syscall;
force_sig_info(SIGSYS, &info, current);
}
+
+#ifdef CONFIG_BPF_SYSCALL
+static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
+{
+ /* XXX: this cast generates a warning. should we make people pass in
+ * &fd, or is there some nicer way of doing this?
+ */
+ u32 fd = (u32) filter;
+ struct seccomp_filter *ret;
+ struct bpf_prog *prog;
+
+ prog = bpf_prog_get(fd);
+ if (IS_ERR(prog))
+ return (struct seccomp_filter *) prog;
+
+ if (prog->type != BPF_PROG_TYPE_SECCOMP) {
+ bpf_prog_put(prog);
+ return ERR_PTR(-EINVAL);
+ }
+
+ ret = kzalloc(sizeof(*ret), GFP_KERNEL | __GFP_NOWARN);
+ if (!ret) {
+ bpf_prog_put(prog);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ ret->prog = prog;
+ atomic_set(&ret->usage, 1);
+
+ /* Intentionally don't bpf_prog_put() here, because the underlying prog
+ * is refcounted too and we're holding a reference from the struct
+ * seccomp_filter object.
+ */
+
+ return ret;
+}
+#else
+static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
#endif /* CONFIG_SECCOMP_FILTER */
/*
@@ -775,8 +806,23 @@ static long seccomp_set_mode_filter(unsigned int flags,
if (flags & ~SECCOMP_FILTER_FLAG_MASK)
return -EINVAL;
+ /*
+ * Installing a seccomp filter requires that the task has
+ * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
+ * This avoids scenarios where unprivileged tasks can affect the
+ * behavior of privileged children.
+ */
+ if (!task_no_new_privs(current) &&
+ security_capable_noaudit(current_cred(), current_user_ns(),
+ CAP_SYS_ADMIN) != 0)
+ return -EACCES;
+
/* Prepare the new filter before holding any locks. */
- prepared = seccomp_prepare_user_filter(filter);
+ if (flags & SECCOMP_FILTER_FLAG_EBPF)
+ prepared = seccomp_prepare_ebpf(filter);
+ else
+ prepared = seccomp_prepare_user_filter(filter);
+
if (IS_ERR(prepared))
return PTR_ERR(prepared);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-09-04 22:50 +0200 |
| Subject | Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd |
| Message-ID | <q565Q-4De-9@gated-at.bofh.it> |
| In reply to | #1219144 |
On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> This is the final bit needed to support seccomp filters created via the bpf
> syscall.
>
> One concern with this patch is exactly what the interface should look like
> for users, since seccomp()'s second argument is a pointer, we could ask
> people to pass a pointer to the fd, but implies we might write to it which
> seems impolite. Right now we cast the pointer (and force the user to cast
> it), which generates ugly warnings. I'm not sure what the right answer is
> here.
>
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Will Drewry <wad@chromium.org>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/linux/seccomp.h | 3 +-
> include/uapi/linux/seccomp.h | 1 +
> kernel/seccomp.c | 70 ++++++++++++++++++++++++++++++++++++--------
> 3 files changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index d1a86ed..a725dd5 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -3,7 +3,8 @@
>
> #include <uapi/linux/seccomp.h>
>
> -#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC)
> +#define SECCOMP_FILTER_FLAG_MASK (\
> + SECCOMP_FILTER_FLAG_TSYNC | SECCOMP_FILTER_FLAG_EBPF)
>
> #ifdef CONFIG_SECCOMP
>
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 0f238a4..c29a423 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -16,6 +16,7 @@
>
> /* Valid flags for SECCOMP_SET_MODE_FILTER */
> #define SECCOMP_FILTER_FLAG_TSYNC 1
> +#define SECCOMP_FILTER_FLAG_EBPF (1 << 1)
>
> /*
> * All BPF programs must return a 32-bit value.
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index a2c5b32..9c6bea6 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -355,17 +355,6 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
>
> BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
>
> - /*
> - * Installing a seccomp filter requires that the task has
> - * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
> - * This avoids scenarios where unprivileged tasks can affect the
> - * behavior of privileged children.
> - */
> - if (!task_no_new_privs(current) &&
> - security_capable_noaudit(current_cred(), current_user_ns(),
> - CAP_SYS_ADMIN) != 0)
> - return ERR_PTR(-EACCES);
> -
> /* Allocate a new seccomp_filter */
> sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
> if (!sfilter)
> @@ -509,6 +498,48 @@ static void seccomp_send_sigsys(int syscall, int reason)
> info.si_syscall = syscall;
> force_sig_info(SIGSYS, &info, current);
> }
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
> +{
> + /* XXX: this cast generates a warning. should we make people pass in
> + * &fd, or is there some nicer way of doing this?
> + */
> + u32 fd = (u32) filter;
I think this is probably the right way to do it, modulo getting the
warning fixed. Let me invoke the great linux-api subscribers to get
some more opinions.
tl;dr: adding SECCOMP_FILTER_FLAG_EBPF to the flags changes the
pointer argument into an fd argument. Is this sane, should it be a
pointer to an fd, or should it not be a flag at all, creating a new
seccomp command instead (SECCOMP_MODE_FILTER_EBPF)?
-Kees
> + struct seccomp_filter *ret;
> + struct bpf_prog *prog;
> +
> + prog = bpf_prog_get(fd);
> + if (IS_ERR(prog))
> + return (struct seccomp_filter *) prog;
> +
> + if (prog->type != BPF_PROG_TYPE_SECCOMP) {
> + bpf_prog_put(prog);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + ret = kzalloc(sizeof(*ret), GFP_KERNEL | __GFP_NOWARN);
> + if (!ret) {
> + bpf_prog_put(prog);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + ret->prog = prog;
> + atomic_set(&ret->usage, 1);
> +
> + /* Intentionally don't bpf_prog_put() here, because the underlying prog
> + * is refcounted too and we're holding a reference from the struct
> + * seccomp_filter object.
> + */
> +
> + return ret;
> +}
> +#else
> +static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
> +{
> + return ERR_PTR(-EINVAL);
> +}
> +#endif
> #endif /* CONFIG_SECCOMP_FILTER */
>
> /*
> @@ -775,8 +806,23 @@ static long seccomp_set_mode_filter(unsigned int flags,
> if (flags & ~SECCOMP_FILTER_FLAG_MASK)
> return -EINVAL;
>
> + /*
> + * Installing a seccomp filter requires that the task has
> + * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
> + * This avoids scenarios where unprivileged tasks can affect the
> + * behavior of privileged children.
> + */
> + if (!task_no_new_privs(current) &&
> + security_capable_noaudit(current_cred(), current_user_ns(),
> + CAP_SYS_ADMIN) != 0)
> + return -EACCES;
> +
> /* Prepare the new filter before holding any locks. */
> - prepared = seccomp_prepare_user_filter(filter);
> + if (flags & SECCOMP_FILTER_FLAG_EBPF)
> + prepared = seccomp_prepare_ebpf(filter);
> + else
> + prepared = seccomp_prepare_user_filter(filter);
> +
> if (IS_ERR(prepared))
> return PTR_ERR(prepared);
>
> --
> 2.1.4
>
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> |
|---|---|
| Date | 2015-09-05 09:20 +0200 |
| Subject | Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd |
| Message-ID | <q5fVv-1Ug-1@gated-at.bofh.it> |
| In reply to | #1219308 |
On 09/04/2015 10:41 PM, Kees Cook wrote:
> On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
>> This is the final bit needed to support seccomp filters created via the bpf
>> syscall.
Hmm. Thanks Kees, for CCinf linux-api@. That really should have been done at
the outset.
Tycho, where's the man-pages patch describing this new kernel-userspace
API feature? :-)
>> One concern with this patch is exactly what the interface should look like
>> for users, since seccomp()'s second argument is a pointer, we could ask
>> people to pass a pointer to the fd, but implies we might write to it which
>> seems impolite. Right now we cast the pointer (and force the user to cast
>> it), which generates ugly warnings. I'm not sure what the right answer is
>> here.
>>
>> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
>> CC: Kees Cook <keescook@chromium.org>
>> CC: Will Drewry <wad@chromium.org>
>> CC: Oleg Nesterov <oleg@redhat.com>
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Pavel Emelyanov <xemul@parallels.com>
>> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
>> CC: Alexei Starovoitov <ast@kernel.org>
>> CC: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>> include/linux/seccomp.h | 3 +-
>> include/uapi/linux/seccomp.h | 1 +
>> kernel/seccomp.c | 70 ++++++++++++++++++++++++++++++++++++--------
>> 3 files changed, 61 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
>> index d1a86ed..a725dd5 100644
>> --- a/include/linux/seccomp.h
>> +++ b/include/linux/seccomp.h
>> @@ -3,7 +3,8 @@
>>
>> #include <uapi/linux/seccomp.h>
>>
>> -#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC)
>> +#define SECCOMP_FILTER_FLAG_MASK (\
>> + SECCOMP_FILTER_FLAG_TSYNC | SECCOMP_FILTER_FLAG_EBPF)
>>
>> #ifdef CONFIG_SECCOMP
>>
>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>> index 0f238a4..c29a423 100644
>> --- a/include/uapi/linux/seccomp.h
>> +++ b/include/uapi/linux/seccomp.h
>> @@ -16,6 +16,7 @@
>>
>> /* Valid flags for SECCOMP_SET_MODE_FILTER */
>> #define SECCOMP_FILTER_FLAG_TSYNC 1
>> +#define SECCOMP_FILTER_FLAG_EBPF (1 << 1)
>>
>> /*
>> * All BPF programs must return a 32-bit value.
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index a2c5b32..9c6bea6 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -355,17 +355,6 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
>>
>> BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
>>
>> - /*
>> - * Installing a seccomp filter requires that the task has
>> - * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
>> - * This avoids scenarios where unprivileged tasks can affect the
>> - * behavior of privileged children.
>> - */
>> - if (!task_no_new_privs(current) &&
>> - security_capable_noaudit(current_cred(), current_user_ns(),
>> - CAP_SYS_ADMIN) != 0)
>> - return ERR_PTR(-EACCES);
>> -
>> /* Allocate a new seccomp_filter */
>> sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
>> if (!sfilter)
>> @@ -509,6 +498,48 @@ static void seccomp_send_sigsys(int syscall, int reason)
>> info.si_syscall = syscall;
>> force_sig_info(SIGSYS, &info, current);
>> }
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
>> +{
>> + /* XXX: this cast generates a warning. should we make people pass in
>> + * &fd, or is there some nicer way of doing this?
>> + */
>> + u32 fd = (u32) filter;
>
> I think this is probably the right way to do it, modulo getting the
> warning fixed. Let me invoke the great linux-api subscribers to get
> some more opinions.
Sigh. It's sad, but the using a cast does seem the simplest option.
But, how about another idea...
> tl;dr: adding SECCOMP_FILTER_FLAG_EBPF to the flags changes the
> pointer argument into an fd argument. Is this sane, should it be a
> pointer to an fd, or should it not be a flag at all, creating a new
> seccomp command instead (SECCOMP_MODE_FILTER_EBPF)?
What about
seccomp(SECCOMP_MODE_FILTER_EBPF, flags, structp)
Where structp is a pointer to something like
struct seccomp_ebpf {
int size; /* Size of this whole struct */
int fd;
}
'size' allows for future expansion of the struct (in case we want to
expand it later), and placing 'fd' inside a struct avoids unpleasant
implication that would be made by passing a pointer to an fd as the
third argument.
Cheers,
Michael
> -Kees
>
>> + struct seccomp_filter *ret;
>> + struct bpf_prog *prog;
>> +
>> + prog = bpf_prog_get(fd);
>> + if (IS_ERR(prog))
>> + return (struct seccomp_filter *) prog;
>> +
>> + if (prog->type != BPF_PROG_TYPE_SECCOMP) {
>> + bpf_prog_put(prog);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + ret = kzalloc(sizeof(*ret), GFP_KERNEL | __GFP_NOWARN);
>> + if (!ret) {
>> + bpf_prog_put(prog);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + ret->prog = prog;
>> + atomic_set(&ret->usage, 1);
>> +
>> + /* Intentionally don't bpf_prog_put() here, because the underlying prog
>> + * is refcounted too and we're holding a reference from the struct
>> + * seccomp_filter object.
>> + */
>> +
>> + return ret;
>> +}
>> +#else
>> +static struct seccomp_filter *seccomp_prepare_ebpf(const char __user *filter)
>> +{
>> + return ERR_PTR(-EINVAL);
>> +}
>> +#endif
>> #endif /* CONFIG_SECCOMP_FILTER */
>>
>> /*
>> @@ -775,8 +806,23 @@ static long seccomp_set_mode_filter(unsigned int flags,
>> if (flags & ~SECCOMP_FILTER_FLAG_MASK)
>> return -EINVAL;
>>
>> + /*
>> + * Installing a seccomp filter requires that the task has
>> + * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
>> + * This avoids scenarios where unprivileged tasks can affect the
>> + * behavior of privileged children.
>> + */
>> + if (!task_no_new_privs(current) &&
>> + security_capable_noaudit(current_cred(), current_user_ns(),
>> + CAP_SYS_ADMIN) != 0)
>> + return -EACCES;
>> +
>> /* Prepare the new filter before holding any locks. */
>> - prepared = seccomp_prepare_user_filter(filter);
>> + if (flags & SECCOMP_FILTER_FLAG_EBPF)
>> + prepared = seccomp_prepare_ebpf(filter);
>> + else
>> + prepared = seccomp_prepare_user_filter(filter);
>> +
>> if (IS_ERR(prepared))
>> return PTR_ERR(prepared);
>>
>> --
>> 2.1.4
>>
>
>
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexei Starovoitov <alexei.starovoitov@gmail.com> |
|---|---|
| Date | 2015-09-04 22:50 +0200 |
| Subject | Re: [PATCH 5/6] seccomp: add a way to attach a filter via eBPF fd |
| Message-ID | <q565R-4De-17@gated-at.bofh.it> |
| In reply to | #1219144 |
On Fri, Sep 04, 2015 at 10:04:23AM -0600, Tycho Andersen wrote: > This is the final bit needed to support seccomp filters created via the bpf > syscall. > > One concern with this patch is exactly what the interface should look like > for users, since seccomp()'s second argument is a pointer, we could ask > people to pass a pointer to the fd, but implies we might write to it which > seems impolite. Right now we cast the pointer (and force the user to cast > it), which generates ugly warnings. I'm not sure what the right answer is > here. I think passing &fd is fine. setsockopt does similar things. > -#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC) > +#define SECCOMP_FILTER_FLAG_MASK (\ > + SECCOMP_FILTER_FLAG_TSYNC | SECCOMP_FILTER_FLAG_EBPF) > > #ifdef CONFIG_SECCOMP > > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h > index 0f238a4..c29a423 100644 > --- a/include/uapi/linux/seccomp.h > +++ b/include/uapi/linux/seccomp.h > @@ -16,6 +16,7 @@ > > /* Valid flags for SECCOMP_SET_MODE_FILTER */ > #define SECCOMP_FILTER_FLAG_TSYNC 1 > +#define SECCOMP_FILTER_FLAG_EBPF (1 << 1) ... > - prepared = seccomp_prepare_user_filter(filter); > + if (flags & SECCOMP_FILTER_FLAG_EBPF) > + prepared = seccomp_prepare_ebpf(filter); > + else > + prepared = seccomp_prepare_user_filter(filter); > + I think instead of flag for existing SECCOMP_SET_MODE_FILTER command, it would have been cleaner to add new command SECCOMP_SET_MODE_BPF and pass &fd to it. Both kernel implementation and user side would look better ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 18:10 +0200 |
| Subject | [PATCH 4/6] seccomp: add a way to access filters via bpf fds |
| Message-ID | <q51IS-70a-21@gated-at.bofh.it> |
| In reply to | #1219143 |
This patch adds a way for a process that is "real root" to access the
seccomp filters of another process. The process first does a
PTRACE_SECCOMP_GET_FILTER_FD to get an fd with that process' seccomp filter
attached, and then iterates on this with PTRACE_SECCOMP_NEXT_FILTER using
bpf(BPF_PROG_DUMP) to dump the actual program at each step.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
CC: Kees Cook <keescook@chromium.org>
CC: Will Drewry <wad@chromium.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 12 ++++++++++
include/linux/seccomp.h | 14 +++++++++++
include/uapi/linux/ptrace.h | 3 +++
kernel/bpf/syscall.c | 26 ++++++++++++++++++++-
kernel/ptrace.c | 7 ++++++
kernel/seccomp.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4383476..30682dc 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -157,6 +157,8 @@ void bpf_register_prog_type(struct bpf_prog_type_list *tl);
void bpf_register_map_type(struct bpf_map_type_list *tl);
struct bpf_prog *bpf_prog_get(u32 ufd);
+int bpf_prog_set(u32 ufd, struct bpf_prog *new);
+int bpf_new_fd(struct bpf_prog *prog, int flags);
void bpf_prog_put(struct bpf_prog *prog);
void bpf_prog_put_rcu(struct bpf_prog *prog);
@@ -175,6 +177,16 @@ static inline struct bpf_prog *bpf_prog_get(u32 ufd)
return ERR_PTR(-EOPNOTSUPP);
}
+static inline int bpf_prog_set(u32 ufd, struct bpf_prog *new)
+{
+ return -EINVAL;
+}
+
+static inline int bpf_new_fd(struct bpf_prog *prog, int flags)
+{
+ return -EINVAL;
+}
+
static inline void bpf_prog_put(struct bpf_prog *prog)
{
}
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index f426503..d1a86ed 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -95,4 +95,18 @@ static inline void get_seccomp_filter(struct task_struct *tsk)
return;
}
#endif /* CONFIG_SECCOMP_FILTER */
+
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
+extern long seccomp_get_filter_fd(struct task_struct *child);
+extern long seccomp_next_filter(struct task_struct *child, u32 fd);
+#else
+static inline long seccomp_get_filter_fd(struct task_struct *child)
+{
+ return -EINVAL;
+}
+static inline long seccomp_next_filter(struct task_struct *child, u32 fd)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_CHECKPOINT_RESTORE */
#endif /* _LINUX_SECCOMP_H */
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index a7a6979..dfd7d2e 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -23,6 +23,9 @@
#define PTRACE_SYSCALL 24
+#define PTRACE_SECCOMP_GET_FILTER_FD 40
+#define PTRACE_SECCOMP_NEXT_FILTER 41
+
/* 0x4200-0x4300 are reserved for architecture-independent additions. */
#define PTRACE_SETOPTIONS 0x4200
#define PTRACE_GETEVENTMSG 0x4201
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ee580d0..58e7421 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -506,6 +506,30 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
}
EXPORT_SYMBOL_GPL(bpf_prog_get);
+int bpf_prog_set(u32 ufd, struct bpf_prog *new)
+{
+ struct fd f;
+ struct bpf_prog *prog;
+
+ f = fdget(ufd);
+
+ prog = get_prog(f);
+ if (!IS_ERR(prog) && prog)
+ bpf_prog_put(prog);
+
+ atomic_inc(&new->aux->refcnt);
+ f.file->private_data = new;
+ fdput(f);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(bpf_prog_set);
+
+int bpf_new_fd(struct bpf_prog *prog, int flags)
+{
+ return anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, flags);
+}
+EXPORT_SYMBOL_GPL(bpf_new_fd);
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD kern_version
@@ -572,7 +596,7 @@ static int bpf_prog_load(union bpf_attr *attr)
if (err < 0)
goto free_used_maps;
- err = anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, O_RDWR | O_CLOEXEC);
+ err = bpf_new_fd(prog, O_RDWR | O_CLOEXEC);
if (err < 0)
/* failed to allocate fd */
goto free_used_maps;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 787320d..4e4b534 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1016,6 +1016,13 @@ int ptrace_request(struct task_struct *child, long request,
break;
}
#endif
+
+ case PTRACE_SECCOMP_GET_FILTER_FD:
+ return seccomp_get_filter_fd(child);
+
+ case PTRACE_SECCOMP_NEXT_FILTER:
+ return seccomp_next_filter(child, data);
+
default:
break;
}
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index acfe1fb..a2c5b32 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -26,6 +26,8 @@
#endif
#ifdef CONFIG_SECCOMP_FILTER
+#include <linux/bpf.h>
+#include <uapi/linux/bpf.h>
#include <linux/filter.h>
#include <linux/pid.h>
#include <linux/ptrace.h>
@@ -814,6 +816,61 @@ static inline long seccomp_set_mode_filter(unsigned int flags,
}
#endif
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
+long seccomp_get_filter_fd(struct task_struct *child)
+{
+ long fd;
+ struct seccomp_filter *filter;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ if (child->seccomp.mode != SECCOMP_MODE_FILTER)
+ return -EINVAL;
+
+ filter = child->seccomp.filter;
+
+ fd = bpf_new_fd(filter->prog, O_RDONLY);
+ if (fd > 0)
+ atomic_inc(&filter->prog->aux->refcnt);
+
+ return fd;
+}
+
+long seccomp_next_filter(struct task_struct *child, u32 fd)
+{
+ struct seccomp_filter *cur;
+ struct bpf_prog *prog;
+ long ret = -ESRCH;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ if (child->seccomp.mode != SECCOMP_MODE_FILTER)
+ return -EINVAL;
+
+ prog = bpf_prog_get(fd);
+ if (IS_ERR(prog)) {
+ ret = PTR_ERR(prog);
+ goto out;
+ }
+
+ for (cur = child->seccomp.filter; cur; cur = cur->prev) {
+ if (cur->prog == prog) {
+ if (!cur->prev)
+ ret = -ENOENT;
+ else
+ ret = bpf_prog_set(fd, cur->prev->prog);
+ break;
+ }
+ }
+
+out:
+ bpf_prog_put(prog);
+ return ret;
+}
+#endif
+
/* Common entry point for both prctl and syscall. */
static long do_seccomp(unsigned int op, unsigned int flags,
const char __user *uargs)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-09-04 22:30 +0200 |
| Subject | Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds |
| Message-ID | <q55Mu-4gS-9@gated-at.bofh.it> |
| In reply to | #1219145 |
On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> This patch adds a way for a process that is "real root" to access the
> seccomp filters of another process. The process first does a
> PTRACE_SECCOMP_GET_FILTER_FD to get an fd with that process' seccomp filter
> attached, and then iterates on this with PTRACE_SECCOMP_NEXT_FILTER using
> bpf(BPF_PROG_DUMP) to dump the actual program at each step.
Why is this a new ptrace interface instead of a new seccomp interface?
I would expect this to only be valid for "current", otherwise we could
run into races as the ptracee adds filters. i.e. it is not safe to
examine seccomp filters from tasks other than current.
-Kees
>
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Will Drewry <wad@chromium.org>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/linux/bpf.h | 12 ++++++++++
> include/linux/seccomp.h | 14 +++++++++++
> include/uapi/linux/ptrace.h | 3 +++
> kernel/bpf/syscall.c | 26 ++++++++++++++++++++-
> kernel/ptrace.c | 7 ++++++
> kernel/seccomp.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 118 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4383476..30682dc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -157,6 +157,8 @@ void bpf_register_prog_type(struct bpf_prog_type_list *tl);
> void bpf_register_map_type(struct bpf_map_type_list *tl);
>
> struct bpf_prog *bpf_prog_get(u32 ufd);
> +int bpf_prog_set(u32 ufd, struct bpf_prog *new);
> +int bpf_new_fd(struct bpf_prog *prog, int flags);
> void bpf_prog_put(struct bpf_prog *prog);
> void bpf_prog_put_rcu(struct bpf_prog *prog);
>
> @@ -175,6 +177,16 @@ static inline struct bpf_prog *bpf_prog_get(u32 ufd)
> return ERR_PTR(-EOPNOTSUPP);
> }
>
> +static inline int bpf_prog_set(u32 ufd, struct bpf_prog *new)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int bpf_new_fd(struct bpf_prog *prog, int flags)
> +{
> + return -EINVAL;
> +}
> +
> static inline void bpf_prog_put(struct bpf_prog *prog)
> {
> }
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index f426503..d1a86ed 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -95,4 +95,18 @@ static inline void get_seccomp_filter(struct task_struct *tsk)
> return;
> }
> #endif /* CONFIG_SECCOMP_FILTER */
> +
> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
> +extern long seccomp_get_filter_fd(struct task_struct *child);
> +extern long seccomp_next_filter(struct task_struct *child, u32 fd);
> +#else
> +static inline long seccomp_get_filter_fd(struct task_struct *child)
> +{
> + return -EINVAL;
> +}
> +static inline long seccomp_next_filter(struct task_struct *child, u32 fd)
> +{
> + return -EINVAL;
> +}
> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_CHECKPOINT_RESTORE */
> #endif /* _LINUX_SECCOMP_H */
> diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
> index a7a6979..dfd7d2e 100644
> --- a/include/uapi/linux/ptrace.h
> +++ b/include/uapi/linux/ptrace.h
> @@ -23,6 +23,9 @@
>
> #define PTRACE_SYSCALL 24
>
> +#define PTRACE_SECCOMP_GET_FILTER_FD 40
> +#define PTRACE_SECCOMP_NEXT_FILTER 41
> +
> /* 0x4200-0x4300 are reserved for architecture-independent additions. */
> #define PTRACE_SETOPTIONS 0x4200
> #define PTRACE_GETEVENTMSG 0x4201
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ee580d0..58e7421 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -506,6 +506,30 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
> }
> EXPORT_SYMBOL_GPL(bpf_prog_get);
>
> +int bpf_prog_set(u32 ufd, struct bpf_prog *new)
> +{
> + struct fd f;
> + struct bpf_prog *prog;
> +
> + f = fdget(ufd);
> +
> + prog = get_prog(f);
> + if (!IS_ERR(prog) && prog)
> + bpf_prog_put(prog);
> +
> + atomic_inc(&new->aux->refcnt);
> + f.file->private_data = new;
> + fdput(f);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(bpf_prog_set);
> +
> +int bpf_new_fd(struct bpf_prog *prog, int flags)
> +{
> + return anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, flags);
> +}
> +EXPORT_SYMBOL_GPL(bpf_new_fd);
> +
> /* last field in 'union bpf_attr' used by this command */
> #define BPF_PROG_LOAD_LAST_FIELD kern_version
>
> @@ -572,7 +596,7 @@ static int bpf_prog_load(union bpf_attr *attr)
> if (err < 0)
> goto free_used_maps;
>
> - err = anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, O_RDWR | O_CLOEXEC);
> + err = bpf_new_fd(prog, O_RDWR | O_CLOEXEC);
> if (err < 0)
> /* failed to allocate fd */
> goto free_used_maps;
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 787320d..4e4b534 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -1016,6 +1016,13 @@ int ptrace_request(struct task_struct *child, long request,
> break;
> }
> #endif
> +
> + case PTRACE_SECCOMP_GET_FILTER_FD:
> + return seccomp_get_filter_fd(child);
> +
> + case PTRACE_SECCOMP_NEXT_FILTER:
> + return seccomp_next_filter(child, data);
> +
> default:
> break;
> }
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index acfe1fb..a2c5b32 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -26,6 +26,8 @@
> #endif
>
> #ifdef CONFIG_SECCOMP_FILTER
> +#include <linux/bpf.h>
> +#include <uapi/linux/bpf.h>
> #include <linux/filter.h>
> #include <linux/pid.h>
> #include <linux/ptrace.h>
> @@ -814,6 +816,61 @@ static inline long seccomp_set_mode_filter(unsigned int flags,
> }
> #endif
>
> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
> +long seccomp_get_filter_fd(struct task_struct *child)
> +{
> + long fd;
> + struct seccomp_filter *filter;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (child->seccomp.mode != SECCOMP_MODE_FILTER)
> + return -EINVAL;
> +
> + filter = child->seccomp.filter;
> +
> + fd = bpf_new_fd(filter->prog, O_RDONLY);
> + if (fd > 0)
> + atomic_inc(&filter->prog->aux->refcnt);
> +
> + return fd;
> +}
> +
> +long seccomp_next_filter(struct task_struct *child, u32 fd)
> +{
> + struct seccomp_filter *cur;
> + struct bpf_prog *prog;
> + long ret = -ESRCH;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (child->seccomp.mode != SECCOMP_MODE_FILTER)
> + return -EINVAL;
> +
> + prog = bpf_prog_get(fd);
> + if (IS_ERR(prog)) {
> + ret = PTR_ERR(prog);
> + goto out;
> + }
> +
> + for (cur = child->seccomp.filter; cur; cur = cur->prev) {
> + if (cur->prog == prog) {
> + if (!cur->prev)
> + ret = -ENOENT;
> + else
> + ret = bpf_prog_set(fd, cur->prev->prog);
> + break;
> + }
> + }
> +
> +out:
> + bpf_prog_put(prog);
> + return ret;
> +}
> +#endif
> +
> /* Common entry point for both prctl and syscall. */
> static long do_seccomp(unsigned int op, unsigned int flags,
> const char __user *uargs)
> --
> 2.1.4
>
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexei Starovoitov <alexei.starovoitov@gmail.com> |
|---|---|
| Date | 2015-09-04 22:40 +0200 |
| Subject | Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds |
| Message-ID | <q55Wa-4s0-3@gated-at.bofh.it> |
| In reply to | #1219292 |
On Fri, Sep 04, 2015 at 01:26:42PM -0700, Kees Cook wrote: > On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen > <tycho.andersen@canonical.com> wrote: > > This patch adds a way for a process that is "real root" to access the > > seccomp filters of another process. The process first does a > > PTRACE_SECCOMP_GET_FILTER_FD to get an fd with that process' seccomp filter > > attached, and then iterates on this with PTRACE_SECCOMP_NEXT_FILTER using > > bpf(BPF_PROG_DUMP) to dump the actual program at each step. > > Why is this a new ptrace interface instead of a new seccomp interface? > I would expect this to only be valid for "current", otherwise we could > run into races as the ptracee adds filters. i.e. it is not safe to > examine seccomp filters from tasks other than current. same question. I thought we discussed to add a command to seccomp() syscall for that? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 23:00 +0200 |
| Subject | Re: [PATCH 4/6] seccomp: add a way to access filters via bpf fds |
| Message-ID | <q56fw-4On-9@gated-at.bofh.it> |
| In reply to | #1219297 |
On Fri, Sep 04, 2015 at 01:29:49PM -0700, Alexei Starovoitov wrote: > On Fri, Sep 04, 2015 at 01:26:42PM -0700, Kees Cook wrote: > > On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen > > <tycho.andersen@canonical.com> wrote: > > > This patch adds a way for a process that is "real root" to access the > > > seccomp filters of another process. The process first does a > > > PTRACE_SECCOMP_GET_FILTER_FD to get an fd with that process' seccomp filter > > > attached, and then iterates on this with PTRACE_SECCOMP_NEXT_FILTER using > > > bpf(BPF_PROG_DUMP) to dump the actual program at each step. > > > > Why is this a new ptrace interface instead of a new seccomp interface? > > I would expect this to only be valid for "current", otherwise we could > > run into races as the ptracee adds filters. The task has to be in the stopped state in order for the filters to be inspected, so I think this isn't a problem. > > i.e. it is not safe to > > examine seccomp filters from tasks other than current. > > same question. > I thought we discussed to add a command to seccomp() syscall for that? We did initially, although at the end Andy posted about not allowing tasks to see some filters that had been installed: On Mon, 10 Aug 2015 14:36:09 -0700 Andy Lutomirski <luto@amacapital.net> wrote: > On Mon, Aug 10, 2015 at 2:31 PM, Tycho Andersen > <tycho.andersen@canonical.com> wrote: > > On Mon, Aug 10, 2015 at 02:18:29PM -0700, Kees Cook wrote: > >> On Sun, Aug 9, 2015 at 7:20 PM, Andy Lutomirski > >> <luto@amacapital.net> wrote: > >> > On Fri, Aug 7, 2015 at 10:59 AM, Tycho Andersen > >> > <tycho.andersen@canonical.com> wrote: > >> >> On Fri, Aug 07, 2015 at 10:36:02AM -0700, Andy Lutomirski wrote: > >> >>> On Fri, Aug 7, 2015 at 8:45 AM, Tycho Andersen > >> >>> <tycho.andersen@canonical.com> wrote: > >> >>> > > >> >>> > fd = seccomp(SECCOMP_GET_FILTER_FD); > >> >>> > >> >>> > >> > > >> > Let me propose something crazy. First, some scenarios: > >> > > >> > Scenario A: A program runs in a sandbox. It tries to interrogate > >> > its > >> > seccomp filter. I think it would be fine, and maybe even > >> > beneficial, > >> > if this didn't work (or pretended there was no filter). > >> > > >> > Scenario B: A shell runs in a sandbox. The user fires up some > >> > program > >> > in that shell and then, *still in that shell*, checkpoints it > >> > with > >> > CRIU. I think CRIU shouldn't see the seccomp filter. > >> > > >> > Scenario C: A program runs in a sandbox. CRIU, *in a different > >> > sandbox*, tries to checkpoint that program. IMO this is doomed > >> > to > >> > eventual failure no matter what the kernel does: restoring the > >> > program > >> > isn't going to work as expected. > >> > > >> > Scenario D: A shell runs in a sandbox. Someone fires up a > >> > seccomp-using program (with in inner filter) under that shell and > >> > then > >> > separately checkpoints it from inside the shell. I think CRIU > >> > should > >> > see the inner filter but not the outer filter. > >> > > >> > So here's my crazy proposal: If process A tries to read process > >> > B's > >> > seccomp state, the kernel could check whether B's seccomp state > >> > is a > >> > descendent of A's. If so, then the attempt to read B's seccomp > >> > state > >> > should see only the part that stricly descends from A's state. > >> > (If > >> > B's and A's states are the same under a referential equality > >> > test, > >> > then this means A should see no seccomp state at all). If, on > >> > the > >> > other hand, B's state is not a descendent of A's state, then the > >> > read > >> > call should fail. > >> > > >> > Thoughts? > >> > > >> > At the very least, this means that no one ever needs to worry > >> > about > >> > preventing seccomp state reads in their seccomp filter program, > >> > since > >> > the filter is invisible from inside the sandbox using that > >> > filter. > >> > >> I don't oppose this idea, but I think our first pass at CRIU can > >> just > >> be to work from the init namespace. I don't mind rejecting > >> filter-reading requests when CAP_SYS_ADMIN is missing or something > >> like that. > > > > The problem is that we can't do a CAP_SYS_ADMIN check, at least with > > the proposed interface, since the seccomp() syscall will only tell > > the > > process about it's own filters. I was going to just have the > > PT_SUSPEND_SECCOMP flag we use also affect whether seccomp() would > > allow a task to inspect its own filters. > > > > If we do that, the result is that without PT_SUSPEND_SECCOMP, a task > > always gets EACCES (or an empty list, whichever is better), and with > > PT_SUSPEND_SECCOMP it always gets the real filters. > > Hmm. IMO this interface has the downside that we can't really > implement my crazy proposal on top of it. Why do we need the task to > be able to read its own filters at all? Wouldn't it be easier if the > ptracer could read the tracee's filters directly? (Even if it > required that the tracee be stopped?) Which is how we ended up with the proposed ptrace() interface. We could proxy this through seccomp instead, but give that it is an external process inspecting the filters, it fits better in ptrace IMO. If we go back to a process asking for its own filters, then we could do it via seccomp(). Tycho -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 18:10 +0200 |
| Subject | [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps |
| Message-ID | <q51IT-70a-35@gated-at.bofh.it> |
| In reply to | #1219143 |
The classic converter generates conditional jumps with:
if (BPF_SRC(fp->code) == BPF_K && (int) fp->k < 0) {
...
} else {
insn->dst_reg = BPF_REG_A;
insn->src_reg = BPF_REG_X;
insn->imm = fp->k;
bpf_src = BPF_SRC(fp->code);
}
but here, we enforce that the src_reg == BPF_REG_0. We should also allow
BPF_REG_X since that's what the converter generates; this enables us to
load eBPF programs that were generated by the converter.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
CC: Kees Cook <keescook@chromium.org>
CC: Will Drewry <wad@chromium.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/verifier.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 039d866..2fff8cd 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1080,7 +1080,12 @@ static int check_cond_jmp_op(struct verifier_env *env,
if (err)
return err;
} else {
- if (insn->src_reg != BPF_REG_0) {
+ switch (insn->src_reg) {
+ case BPF_REG_0:
+ /* the classic converter generates BPF_JMP with src_reg of X */
+ case BPF_REG_X:
+ break;
+ default:
verbose("BPF_JMP uses reserved fields\n");
return -EINVAL;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexei Starovoitov <alexei.starovoitov@gmail.com> |
|---|---|
| Date | 2015-09-04 23:10 +0200 |
| Subject | Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps |
| Message-ID | <q56pc-5eU-35@gated-at.bofh.it> |
| In reply to | #1219146 |
On Fri, Sep 04, 2015 at 10:04:24AM -0600, Tycho Andersen wrote:
> The classic converter generates conditional jumps with:
>
> if (BPF_SRC(fp->code) == BPF_K && (int) fp->k < 0) {
> ...
> } else {
> insn->dst_reg = BPF_REG_A;
> insn->src_reg = BPF_REG_X;
> insn->imm = fp->k;
> bpf_src = BPF_SRC(fp->code);
> }
>
> but here, we enforce that the src_reg == BPF_REG_0. We should also allow
> BPF_REG_X since that's what the converter generates; this enables us to
> load eBPF programs that were generated by the converter.
good catch. classic->extended converter is just being untidy.
It shouldn't be populating unused 'src_reg' field when BPF_SRC == BPF_K
verifier is doing the right thing. It's rejecting instructions that
have junk in unused fields to make sure that someday we can extend it
with something useful.
The fix should be something like this:
diff --git a/net/core/filter.c b/net/core/filter.c
index 13079f03902e..05a04ea87172 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -478,9 +478,9 @@ do_pass:
bpf_src = BPF_X;
} else {
insn->dst_reg = BPF_REG_A;
- insn->src_reg = BPF_REG_X;
insn->imm = fp->k;
bpf_src = BPF_SRC(fp->code);
+ insn->src_reg = bpf_src == BPF_X ? BPF_REG_X : 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-05 00:50 +0200 |
| Subject | Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps |
| Message-ID | <q57XZ-7hG-29@gated-at.bofh.it> |
| In reply to | #1219324 |
Hi Alexei,
On Fri, Sep 04, 2015 at 02:06:19PM -0700, Alexei Starovoitov wrote:
> On Fri, Sep 04, 2015 at 10:04:24AM -0600, Tycho Andersen wrote:
> > The classic converter generates conditional jumps with:
> >
> > if (BPF_SRC(fp->code) == BPF_K && (int) fp->k < 0) {
> > ...
> > } else {
> > insn->dst_reg = BPF_REG_A;
> > insn->src_reg = BPF_REG_X;
> > insn->imm = fp->k;
> > bpf_src = BPF_SRC(fp->code);
> > }
> >
> > but here, we enforce that the src_reg == BPF_REG_0. We should also allow
> > BPF_REG_X since that's what the converter generates; this enables us to
> > load eBPF programs that were generated by the converter.
>
> good catch. classic->extended converter is just being untidy.
> It shouldn't be populating unused 'src_reg' field when BPF_SRC == BPF_K
> verifier is doing the right thing. It's rejecting instructions that
> have junk in unused fields to make sure that someday we can extend it
> with something useful.
> The fix should be something like this:
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 13079f03902e..05a04ea87172 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -478,9 +478,9 @@ do_pass:
> bpf_src = BPF_X;
> } else {
> insn->dst_reg = BPF_REG_A;
> - insn->src_reg = BPF_REG_X;
> insn->imm = fp->k;
> bpf_src = BPF_SRC(fp->code);
> + insn->src_reg = bpf_src == BPF_X ? BPF_REG_X : 0;
> }
Yep, I just tested this and it works for me. Do you want to manage it
or should I carry it as part of this set?
Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexei Starovoitov <alexei.starovoitov@gmail.com> |
|---|---|
| Date | 2015-09-05 06:20 +0200 |
| Subject | Re: [PATCH 6/6] ebpf: allow BPF_REG_X in src_reg conditional jumps |
| Message-ID | <q5d7k-6jK-1@gated-at.bofh.it> |
| In reply to | #1219361 |
On Fri, Sep 04, 2015 at 04:43:50PM -0600, Tycho Andersen wrote:
> > The fix should be something like this:
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 13079f03902e..05a04ea87172 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -478,9 +478,9 @@ do_pass:
> > bpf_src = BPF_X;
> > } else {
> > insn->dst_reg = BPF_REG_A;
> > - insn->src_reg = BPF_REG_X;
> > insn->imm = fp->k;
> > bpf_src = BPF_SRC(fp->code);
> > + insn->src_reg = bpf_src == BPF_X ? BPF_REG_X : 0;
> > }
>
> Yep, I just tested this and it works for me. Do you want to manage it
> or should I carry it as part of this set?
Though it's a bug, it doesn't affect anything at the moment
and not worth fixing in net, so please submit it as separate bug
fix when net-next reopens. imo the rest of the patches should
also go via net-next to minimize cross-tree conflicts.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 18:10 +0200 |
| Subject | [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q51IT-70a-29@gated-at.bofh.it> |
| In reply to | #1219143 |
seccomp uses eBPF as its underlying storage and execution format, and eBPF
has features that seccomp would like to make use of in the future. This
patch adds a formal seccomp type to the eBPF verifier.
The current implementation of the seccomp eBPF type is very limited, and
doesn't support some interesting features (notably, maps) of eBPF. However,
the primary motivation for this patchset is to enable checkpoint/restore
for seccomp filters later in the series, to this limited feature set is ok
for now.
Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
CC: Kees Cook <keescook@chromium.org>
CC: Will Drewry <wad@chromium.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Pavel Emelyanov <xemul@parallels.com>
CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Daniel Borkmann <daniel@iogearbox.net>
---
include/uapi/linux/bpf.h | 1 +
net/core/filter.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 29ef6f9..79b825a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -122,6 +122,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_KPROBE,
BPF_PROG_TYPE_SCHED_CLS,
BPF_PROG_TYPE_SCHED_ACT,
+ BPF_PROG_TYPE_SECCOMP,
};
#define BPF_PSEUDO_MAP_FD 1
diff --git a/net/core/filter.c b/net/core/filter.c
index be3098f..ed339fa 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1466,6 +1466,39 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
}
}
+static const struct bpf_func_proto *
+seccomp_func_proto(enum bpf_func_id func_id)
+{
+ /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
+ * are considered to be read-only after they're installed, so map fds
+ * probably need to be invalidated when a seccomp filter with maps is
+ * installed.
+ *
+ * The rest of these might be reasonable to call from seccomp, so we
+ * export them.
+ */
+ switch (func_id) {
+ case BPF_FUNC_ktime_get_ns:
+ return &bpf_ktime_get_ns_proto;
+ case BPF_FUNC_trace_printk:
+ return bpf_get_trace_printk_proto();
+ case BPF_FUNC_get_prandom_u32:
+ return &bpf_get_prandom_u32_proto;
+ case BPF_FUNC_get_smp_processor_id:
+ return &bpf_get_smp_processor_id_proto;
+ case BPF_FUNC_tail_call:
+ return &bpf_tail_call_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ case BPF_FUNC_get_current_uid_gid:
+ return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_comm:
+ return &bpf_get_current_comm_proto;
+ default:
+ return NULL;
+ }
+}
+
static bool __is_valid_access(int off, int size, enum bpf_access_type type)
{
/* check bounds */
@@ -1516,6 +1549,17 @@ static bool tc_cls_act_is_valid_access(int off, int size,
return __is_valid_access(off, size, type);
}
+static bool seccomp_is_valid_access(int off, int size,
+ enum bpf_access_type type)
+{
+ if (type == BPF_WRITE)
+ return false;
+
+ if (off < 0 || off >= sizeof(struct seccomp_data) || off & 3)
+ return false;
+
+ return true;
+}
static u32 bpf_net_convert_ctx_access(enum bpf_access_type type, int dst_reg,
int src_reg, int ctx_off,
struct bpf_insn *insn_buf)
@@ -1630,6 +1674,45 @@ static u32 bpf_net_convert_ctx_access(enum bpf_access_type type, int dst_reg,
return insn - insn_buf;
}
+static u32 seccomp_convert_ctx_access(enum bpf_access_type type, int dst_reg,
+ int src_reg, int ctx_off,
+ struct bpf_insn *insn_buf)
+{
+ struct bpf_insn *insn = insn_buf;
+
+ switch (ctx_off) {
+ case offsetof(struct seccomp_data, nr):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, nr) != 4);
+
+ *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg, ctx_off);
+ break;
+
+ case offsetof(struct seccomp_data, arch):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, arch) != 4);
+
+ *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg, ctx_off);
+ break;
+
+ case offsetof(struct seccomp_data, instruction_pointer):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data,
+ instruction_pointer) != 8);
+
+ *insn++ = BPF_LDX_MEM(BPF_DW, dst_reg, src_reg, ctx_off);
+ break;
+
+ default:
+ if (ctx_off & 7 ||
+ ctx_off < offsetof(struct seccomp_data, args))
+ return -EINVAL;
+
+ BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, args[0]) != 8);
+
+ *insn++ = BPF_LDX_MEM(BPF_DW, dst_reg, src_reg, ctx_off);
+ }
+
+ return insn - insn_buf;
+}
+
static const struct bpf_verifier_ops sk_filter_ops = {
.get_func_proto = sk_filter_func_proto,
.is_valid_access = sk_filter_is_valid_access,
@@ -1642,6 +1725,12 @@ static const struct bpf_verifier_ops tc_cls_act_ops = {
.convert_ctx_access = bpf_net_convert_ctx_access,
};
+static const struct bpf_verifier_ops seccomp_ops = {
+ .get_func_proto = seccomp_func_proto,
+ .is_valid_access = seccomp_is_valid_access,
+ .convert_ctx_access = seccomp_convert_ctx_access,
+};
+
static struct bpf_prog_type_list sk_filter_type __read_mostly = {
.ops = &sk_filter_ops,
.type = BPF_PROG_TYPE_SOCKET_FILTER,
@@ -1657,11 +1746,17 @@ static struct bpf_prog_type_list sched_act_type __read_mostly = {
.type = BPF_PROG_TYPE_SCHED_ACT,
};
+static struct bpf_prog_type_list seccomp_type __read_mostly = {
+ .ops = &seccomp_ops,
+ .type = BPF_PROG_TYPE_SECCOMP,
+};
+
static int __init register_sk_filter_ops(void)
{
bpf_register_prog_type(&sk_filter_type);
bpf_register_prog_type(&sched_cls_type);
bpf_register_prog_type(&sched_act_type);
+ bpf_register_prog_type(&seccomp_type);
return 0;
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexei Starovoitov <alexei.starovoitov@gmail.com> |
|---|---|
| Date | 2015-09-04 22:20 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q55CO-44B-5@gated-at.bofh.it> |
| In reply to | #1219147 |
On Fri, Sep 04, 2015 at 10:04:19AM -0600, Tycho Andersen wrote:
> seccomp uses eBPF as its underlying storage and execution format, and eBPF
> has features that seccomp would like to make use of in the future. This
> patch adds a formal seccomp type to the eBPF verifier.
>
> The current implementation of the seccomp eBPF type is very limited, and
> doesn't support some interesting features (notably, maps) of eBPF. However,
> the primary motivation for this patchset is to enable checkpoint/restore
> for seccomp filters later in the series, to this limited feature set is ok
> for now.
yes. good compromise to start.
> +static const struct bpf_func_proto *
> +seccomp_func_proto(enum bpf_func_id func_id)
> +{
> + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
> + * are considered to be read-only after they're installed, so map fds
> + * probably need to be invalidated when a seccomp filter with maps is
> + * installed.
Just disabling bpf_map_lookup/update() helpers (the way you did here)
is enough. The prorgram can still have references to maps, but since they
won't be accessed it's safe.
> + *
> + * The rest of these might be reasonable to call from seccomp, so we
> + * export them.
> + */
> + switch (func_id) {
> + case BPF_FUNC_ktime_get_ns:
> + return &bpf_ktime_get_ns_proto;
> + case BPF_FUNC_trace_printk:
> + return bpf_get_trace_printk_proto();
> + case BPF_FUNC_get_prandom_u32:
> + return &bpf_get_prandom_u32_proto;
> + case BPF_FUNC_get_smp_processor_id:
> + return &bpf_get_smp_processor_id_proto;
> + case BPF_FUNC_tail_call:
> + return &bpf_tail_call_proto;
> + case BPF_FUNC_get_current_pid_tgid:
> + return &bpf_get_current_pid_tgid_proto;
> + case BPF_FUNC_get_current_uid_gid:
> + return &bpf_get_current_uid_gid_proto;
> + case BPF_FUNC_get_current_comm:
> + return &bpf_get_current_comm_proto;
the list looks good to start with.
>
> +static u32 seccomp_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> + int src_reg, int ctx_off,
> + struct bpf_insn *insn_buf)
> +{
> + struct bpf_insn *insn = insn_buf;
> +
> + switch (ctx_off) {
> + case offsetof(struct seccomp_data, nr):
the conversion of seccomp_data fields is unnecessary.
We're doing conversion for sk_buff, because sk_buff and __sk_buff aree two
different structures. __sk_buff is user ABI with its own fields that losely
correspond to in-kernel struct sk_buff.
seccomp_data is already part of user ABI, so it's ok to access as-is.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 23:10 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q56pc-5eU-23@gated-at.bofh.it> |
| In reply to | #1219286 |
On Fri, Sep 04, 2015 at 01:17:47PM -0700, Alexei Starovoitov wrote:
> On Fri, Sep 04, 2015 at 10:04:19AM -0600, Tycho Andersen wrote:
> > seccomp uses eBPF as its underlying storage and execution format, and eBPF
> > has features that seccomp would like to make use of in the future. This
> > patch adds a formal seccomp type to the eBPF verifier.
> >
> > The current implementation of the seccomp eBPF type is very limited, and
> > doesn't support some interesting features (notably, maps) of eBPF. However,
> > the primary motivation for this patchset is to enable checkpoint/restore
> > for seccomp filters later in the series, to this limited feature set is ok
> > for now.
>
> yes. good compromise to start.
>
> > +static const struct bpf_func_proto *
> > +seccomp_func_proto(enum bpf_func_id func_id)
> > +{
> > + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
> > + * are considered to be read-only after they're installed, so map fds
> > + * probably need to be invalidated when a seccomp filter with maps is
> > + * installed.
>
> Just disabling bpf_map_lookup/update() helpers (the way you did here)
> is enough. The prorgram can still have references to maps, but since they
> won't be accessed it's safe.
>
> > + *
> > + * The rest of these might be reasonable to call from seccomp, so we
> > + * export them.
> > + */
> > + switch (func_id) {
> > + case BPF_FUNC_ktime_get_ns:
> > + return &bpf_ktime_get_ns_proto;
> > + case BPF_FUNC_trace_printk:
> > + return bpf_get_trace_printk_proto();
> > + case BPF_FUNC_get_prandom_u32:
> > + return &bpf_get_prandom_u32_proto;
> > + case BPF_FUNC_get_smp_processor_id:
> > + return &bpf_get_smp_processor_id_proto;
> > + case BPF_FUNC_tail_call:
> > + return &bpf_tail_call_proto;
> > + case BPF_FUNC_get_current_pid_tgid:
> > + return &bpf_get_current_pid_tgid_proto;
> > + case BPF_FUNC_get_current_uid_gid:
> > + return &bpf_get_current_uid_gid_proto;
> > + case BPF_FUNC_get_current_comm:
> > + return &bpf_get_current_comm_proto;
>
> the list looks good to start with.
>
> >
> > +static u32 seccomp_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> > + int src_reg, int ctx_off,
> > + struct bpf_insn *insn_buf)
> > +{
> > + struct bpf_insn *insn = insn_buf;
> > +
> > + switch (ctx_off) {
> > + case offsetof(struct seccomp_data, nr):
>
> the conversion of seccomp_data fields is unnecessary.
> We're doing conversion for sk_buff, because sk_buff and __sk_buff aree two
> different structures. __sk_buff is user ABI with its own fields that losely
> correspond to in-kernel struct sk_buff.
> seccomp_data is already part of user ABI, so it's ok to access as-is.
Ok, I noticed this but somehow didn't put it all together. I'll axe
this for the next version, thanks.
Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-09-04 22:40 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q55Wb-4s0-21@gated-at.bofh.it> |
| In reply to | #1219147 |
On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> seccomp uses eBPF as its underlying storage and execution format, and eBPF
> has features that seccomp would like to make use of in the future. This
> patch adds a formal seccomp type to the eBPF verifier.
>
> The current implementation of the seccomp eBPF type is very limited, and
> doesn't support some interesting features (notably, maps) of eBPF. However,
> the primary motivation for this patchset is to enable checkpoint/restore
> for seccomp filters later in the series, to this limited feature set is ok
> for now.
>
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Will Drewry <wad@chromium.org>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/uapi/linux/bpf.h | 1 +
> net/core/filter.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 29ef6f9..79b825a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -122,6 +122,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_KPROBE,
> BPF_PROG_TYPE_SCHED_CLS,
> BPF_PROG_TYPE_SCHED_ACT,
> + BPF_PROG_TYPE_SECCOMP,
> };
>
> #define BPF_PSEUDO_MAP_FD 1
> diff --git a/net/core/filter.c b/net/core/filter.c
> index be3098f..ed339fa 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1466,6 +1466,39 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
> }
> }
>
> +static const struct bpf_func_proto *
> +seccomp_func_proto(enum bpf_func_id func_id)
> +{
> + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
> + * are considered to be read-only after they're installed, so map fds
> + * probably need to be invalidated when a seccomp filter with maps is
> + * installed.
> + *
> + * The rest of these might be reasonable to call from seccomp, so we
> + * export them.
> + */
> + switch (func_id) {
> + case BPF_FUNC_ktime_get_ns:
> + return &bpf_ktime_get_ns_proto;
> + case BPF_FUNC_trace_printk:
> + return bpf_get_trace_printk_proto();
> + case BPF_FUNC_get_prandom_u32:
> + return &bpf_get_prandom_u32_proto;
> + case BPF_FUNC_get_smp_processor_id:
> + return &bpf_get_smp_processor_id_proto;
> + case BPF_FUNC_tail_call:
> + return &bpf_tail_call_proto;
> + case BPF_FUNC_get_current_pid_tgid:
> + return &bpf_get_current_pid_tgid_proto;
> + case BPF_FUNC_get_current_uid_gid:
> + return &bpf_get_current_uid_gid_proto;
> + case BPF_FUNC_get_current_comm:
> + return &bpf_get_current_comm_proto;
> + default:
> + return NULL;
> + }
> +}
While this list is probably fine, I don't want to mix the addition of
eBPF functions to the seccomp ABI with the CRIU changes. No function
calls are currently possible and it should stay that way.
I was expecting to see a validator, similar to the existing BPF
validator that is called when creating seccomp filters currently. Can
we add a similar validator for new BPF_PROG_TYPE_SECCOMP?
-Kees
> +
> static bool __is_valid_access(int off, int size, enum bpf_access_type type)
> {
> /* check bounds */
> @@ -1516,6 +1549,17 @@ static bool tc_cls_act_is_valid_access(int off, int size,
> return __is_valid_access(off, size, type);
> }
>
> +static bool seccomp_is_valid_access(int off, int size,
> + enum bpf_access_type type)
> +{
> + if (type == BPF_WRITE)
> + return false;
> +
> + if (off < 0 || off >= sizeof(struct seccomp_data) || off & 3)
> + return false;
> +
> + return true;
> +}
> static u32 bpf_net_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> int src_reg, int ctx_off,
> struct bpf_insn *insn_buf)
> @@ -1630,6 +1674,45 @@ static u32 bpf_net_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> return insn - insn_buf;
> }
>
> +static u32 seccomp_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> + int src_reg, int ctx_off,
> + struct bpf_insn *insn_buf)
> +{
> + struct bpf_insn *insn = insn_buf;
> +
> + switch (ctx_off) {
> + case offsetof(struct seccomp_data, nr):
> + BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, nr) != 4);
> +
> + *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg, ctx_off);
> + break;
> +
> + case offsetof(struct seccomp_data, arch):
> + BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, arch) != 4);
> +
> + *insn++ = BPF_LDX_MEM(BPF_W, dst_reg, src_reg, ctx_off);
> + break;
> +
> + case offsetof(struct seccomp_data, instruction_pointer):
> + BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data,
> + instruction_pointer) != 8);
> +
> + *insn++ = BPF_LDX_MEM(BPF_DW, dst_reg, src_reg, ctx_off);
> + break;
> +
> + default:
> + if (ctx_off & 7 ||
> + ctx_off < offsetof(struct seccomp_data, args))
> + return -EINVAL;
> +
> + BUILD_BUG_ON(FIELD_SIZEOF(struct seccomp_data, args[0]) != 8);
> +
> + *insn++ = BPF_LDX_MEM(BPF_DW, dst_reg, src_reg, ctx_off);
> + }
> +
> + return insn - insn_buf;
> +}
> +
> static const struct bpf_verifier_ops sk_filter_ops = {
> .get_func_proto = sk_filter_func_proto,
> .is_valid_access = sk_filter_is_valid_access,
> @@ -1642,6 +1725,12 @@ static const struct bpf_verifier_ops tc_cls_act_ops = {
> .convert_ctx_access = bpf_net_convert_ctx_access,
> };
>
> +static const struct bpf_verifier_ops seccomp_ops = {
> + .get_func_proto = seccomp_func_proto,
> + .is_valid_access = seccomp_is_valid_access,
> + .convert_ctx_access = seccomp_convert_ctx_access,
> +};
> +
> static struct bpf_prog_type_list sk_filter_type __read_mostly = {
> .ops = &sk_filter_ops,
> .type = BPF_PROG_TYPE_SOCKET_FILTER,
> @@ -1657,11 +1746,17 @@ static struct bpf_prog_type_list sched_act_type __read_mostly = {
> .type = BPF_PROG_TYPE_SCHED_ACT,
> };
>
> +static struct bpf_prog_type_list seccomp_type __read_mostly = {
> + .ops = &seccomp_ops,
> + .type = BPF_PROG_TYPE_SECCOMP,
> +};
> +
> static int __init register_sk_filter_ops(void)
> {
> bpf_register_prog_type(&sk_filter_type);
> bpf_register_prog_type(&sched_cls_type);
> bpf_register_prog_type(&sched_act_type);
> + bpf_register_prog_type(&seccomp_type);
>
> return 0;
> }
> --
> 2.1.4
>
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-09-04 23:10 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q56pc-5eU-25@gated-at.bofh.it> |
| In reply to | #1219303 |
On Fri, Sep 4, 2015 at 2:06 PM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> On Fri, Sep 04, 2015 at 01:34:12PM -0700, Kees Cook wrote:
>> On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
>> <tycho.andersen@canonical.com> wrote:
>> > +static const struct bpf_func_proto *
>> > +seccomp_func_proto(enum bpf_func_id func_id)
>> > +{
>> > + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
>> > + * are considered to be read-only after they're installed, so map fds
>> > + * probably need to be invalidated when a seccomp filter with maps is
>> > + * installed.
>> > + *
>> > + * The rest of these might be reasonable to call from seccomp, so we
>> > + * export them.
>> > + */
>> > + switch (func_id) {
>> > + case BPF_FUNC_ktime_get_ns:
>> > + return &bpf_ktime_get_ns_proto;
>> > + case BPF_FUNC_trace_printk:
>> > + return bpf_get_trace_printk_proto();
>> > + case BPF_FUNC_get_prandom_u32:
>> > + return &bpf_get_prandom_u32_proto;
>> > + case BPF_FUNC_get_smp_processor_id:
>> > + return &bpf_get_smp_processor_id_proto;
>> > + case BPF_FUNC_tail_call:
>> > + return &bpf_tail_call_proto;
>> > + case BPF_FUNC_get_current_pid_tgid:
>> > + return &bpf_get_current_pid_tgid_proto;
>> > + case BPF_FUNC_get_current_uid_gid:
>> > + return &bpf_get_current_uid_gid_proto;
>> > + case BPF_FUNC_get_current_comm:
>> > + return &bpf_get_current_comm_proto;
>> > + default:
>> > + return NULL;
>> > + }
>> > +}
>>
>> While this list is probably fine, I don't want to mix the addition of
>> eBPF functions to the seccomp ABI with the CRIU changes. No function
>> calls are currently possible and it should stay that way.
>
> Ok, I can remove them.
>
>> I was expecting to see a validator, similar to the existing BPF
>> validator that is called when creating seccomp filters currently. Can
>> we add a similar validator for new BPF_PROG_TYPE_SECCOMP?
>
> That's effectively what this patch does; when the eBPF is loaded via
> bpf(), you tell bpf() you want a BPF_PROG_TYPE_SECCOMP, and it invokes
> this validation/translation code, i.e. it uses
> seccomp_is_valid_access() to check and make sure access are aligned
> and inside struct seccomp_data.
What about limiting the possible instructions?
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tycho Andersen <tycho.andersen@canonical.com> |
|---|---|
| Date | 2015-09-04 23:10 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q56pc-5eU-27@gated-at.bofh.it> |
| In reply to | #1219303 |
On Fri, Sep 04, 2015 at 01:34:12PM -0700, Kees Cook wrote:
> On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > +static const struct bpf_func_proto *
> > +seccomp_func_proto(enum bpf_func_id func_id)
> > +{
> > + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
> > + * are considered to be read-only after they're installed, so map fds
> > + * probably need to be invalidated when a seccomp filter with maps is
> > + * installed.
> > + *
> > + * The rest of these might be reasonable to call from seccomp, so we
> > + * export them.
> > + */
> > + switch (func_id) {
> > + case BPF_FUNC_ktime_get_ns:
> > + return &bpf_ktime_get_ns_proto;
> > + case BPF_FUNC_trace_printk:
> > + return bpf_get_trace_printk_proto();
> > + case BPF_FUNC_get_prandom_u32:
> > + return &bpf_get_prandom_u32_proto;
> > + case BPF_FUNC_get_smp_processor_id:
> > + return &bpf_get_smp_processor_id_proto;
> > + case BPF_FUNC_tail_call:
> > + return &bpf_tail_call_proto;
> > + case BPF_FUNC_get_current_pid_tgid:
> > + return &bpf_get_current_pid_tgid_proto;
> > + case BPF_FUNC_get_current_uid_gid:
> > + return &bpf_get_current_uid_gid_proto;
> > + case BPF_FUNC_get_current_comm:
> > + return &bpf_get_current_comm_proto;
> > + default:
> > + return NULL;
> > + }
> > +}
>
> While this list is probably fine, I don't want to mix the addition of
> eBPF functions to the seccomp ABI with the CRIU changes. No function
> calls are currently possible and it should stay that way.
Ok, I can remove them.
> I was expecting to see a validator, similar to the existing BPF
> validator that is called when creating seccomp filters currently. Can
> we add a similar validator for new BPF_PROG_TYPE_SECCOMP?
That's effectively what this patch does; when the eBPF is loaded via
bpf(), you tell bpf() you want a BPF_PROG_TYPE_SECCOMP, and it invokes
this validation/translation code, i.e. it uses
seccomp_is_valid_access() to check and make sure access are aligned
and inside struct seccomp_data.
Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2015-09-05 00:00 +0200 |
| Subject | Re: [PATCH 1/6] ebpf: add a seccomp program type |
| Message-ID | <q57bA-68F-5@gated-at.bofh.it> |
| In reply to | #1219147 |
On Fri, Sep 4, 2015 at 9:04 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> seccomp uses eBPF as its underlying storage and execution format, and eBPF
> has features that seccomp would like to make use of in the future. This
> patch adds a formal seccomp type to the eBPF verifier.
>
> The current implementation of the seccomp eBPF type is very limited, and
> doesn't support some interesting features (notably, maps) of eBPF. However,
> the primary motivation for this patchset is to enable checkpoint/restore
> for seccomp filters later in the series, to this limited feature set is ok
> for now.
>
> Signed-off-by: Tycho Andersen <tycho.andersen@canonical.com>
> CC: Kees Cook <keescook@chromium.org>
> CC: Will Drewry <wad@chromium.org>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Pavel Emelyanov <xemul@parallels.com>
> CC: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> CC: Alexei Starovoitov <ast@kernel.org>
> CC: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/uapi/linux/bpf.h | 1 +
> net/core/filter.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 29ef6f9..79b825a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -122,6 +122,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_KPROBE,
> BPF_PROG_TYPE_SCHED_CLS,
> BPF_PROG_TYPE_SCHED_ACT,
> + BPF_PROG_TYPE_SECCOMP,
> };
>
> #define BPF_PSEUDO_MAP_FD 1
> diff --git a/net/core/filter.c b/net/core/filter.c
> index be3098f..ed339fa 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1466,6 +1466,39 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
> }
> }
>
> +static const struct bpf_func_proto *
> +seccomp_func_proto(enum bpf_func_id func_id)
> +{
> + /* Right now seccomp eBPF loading doesn't support maps; seccomp filters
> + * are considered to be read-only after they're installed, so map fds
> + * probably need to be invalidated when a seccomp filter with maps is
> + * installed.
> + *
> + * The rest of these might be reasonable to call from seccomp, so we
> + * export them.
> + */
> + switch (func_id) {
> + case BPF_FUNC_ktime_get_ns:
> + return &bpf_ktime_get_ns_proto;
> + case BPF_FUNC_trace_printk:
> + return bpf_get_trace_printk_proto();
> + case BPF_FUNC_get_prandom_u32:
> + return &bpf_get_prandom_u32_proto;
I don't think we should expose prandom to unprivileged userspace.
This may be an attack vector.
Also, before we allow anything with side effects, we should think
carefully about whether seccomp filters with side effects (other than
through their return value) are okay.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web