Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1222454 > unrolled thread

v2 of seccomp filter c/r patches

Back to article view | Back to linux.kernel

Contents

  v2 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 02:30 +0200
    [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 02:30 +0200
      Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Daniel Borkmann <daniel@iogearbox.net> - 2015-09-11 13:50 +0200
        Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 16:30 +0200
      Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> - 2015-09-11 14:10 +0200
        Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 16:40 +0200
      Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Andy Lutomirski <luto@amacapital.net> - 2015-09-11 18:30 +0200
        Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 18:50 +0200
          Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds Andy Lutomirski <luto@amacapital.net> - 2015-09-14 20:00 +0200
    Re: v2 of seccomp filter c/r patches Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2015-09-11 05:00 +0200
    Re: v2 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-09-11 18:40 +0200
      Re: v2 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-09-11 19:10 +0200
        Re: v2 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-11 19:30 +0200
          Re: v2 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-09-14 20:00 +0200
            Re: v2 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-15 18:10 +0200
              Re: v2 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-09-15 20:20 +0200
                Re: v2 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-15 20:30 +0200
                  Re: v2 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-09-15 22:10 +0200
                    Re: v2 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-09-15 23:40 +0200

#1222454 — v2 of seccomp filter c/r patches

Hi all,

Here is v2 of the seccomp filter c/r set. The patch notes have individual
changes from the last series, but there are two points not noted:

* The series still does not allow us to correctly restore state for programs
  that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
  keep seccomp_filter's identity, I think something along the lines of another
  seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
  if this can even be done yet). In addition, we'll need a kcmp command for
  figuring out if filters are the same, although this too needs to compare
  seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
  this nicely are welcome.

* I've dropped the bpf converter bug from the set and will submit it
  separately.

Alexei mentioned that this should go via net-next to minimize cross-tree
conflicts. Does that make sense here?

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] | [next] | [standalone]

#1222455 — [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

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 f57d7fe..bfd9cab 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -162,6 +162,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);
 
@@ -180,6 +182,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 a19ddac..41b083c 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 cf1019e..041c3c3 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 58ae9f4..ac3ed1c 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 c8e0e05..a151c35 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1003,6 +1003,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 afaeddf..1856f69 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>
@@ -807,6 +809,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]

#1222731 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

On 09/11/2015 02:21 AM, Tycho Andersen 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.
>
> 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>
[...]
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 58ae9f4..ac3ed1c 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;

So in case get_prog() fails, and for example f.file is infact NULL,
you assign the bpf prog then to ERR_PTR(-EBADF)'s private_data? :(

> +}
> +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);

Any reason why these two need to be exported for modules? Which
modules are using them?

I think modules should probably not mess with this.

If you already name it generic, it would also be good if bpf_new_fd()
is used in case of maps that call anon_inode_getfd(), too.
--
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]

#1222880 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

On Fri, Sep 11, 2015 at 01:47:38PM +0200, Daniel Borkmann wrote:
> On 09/11/2015 02:21 AM, Tycho Andersen 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.
> >
> >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>
> [...]
> >diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >index 58ae9f4..ac3ed1c 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;
> 
> So in case get_prog() fails, and for example f.file is infact NULL,
> you assign the bpf prog then to ERR_PTR(-EBADF)'s private_data? :(

Thanks, I will fix for the next version.

> >+}
> >+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);
> 
> Any reason why these two need to be exported for modules? Which
> modules are using them?
> 
> I think modules should probably not mess with this.

No reason, I suppose. I was just exporting because bpf_prog_get is;
I'll drop it for the next version.

> If you already name it generic, it would also be good if bpf_new_fd()
> is used in case of maps that call anon_inode_getfd(), too.

I needed to call bpf_new_fd from kernel/seccomp.c, which it seems
shouldn't be able to reference bpf_prog_fops, which is why I added the
little "proxy". If we change the api to something like,

bpf_new_fd("bpf-map", &bpf_map_fops, map);
bpf_new_fd("bpf-prog", &bpf_prog_fops, prog);

I'd need access to bpf_prog_fops again. What about changing the name
to bpf_new_prog_fd?

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]

#1222760 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

HI Tycho

On 11 September 2015 at 02:21, 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.

Do you have a man- page patch for this change?

Cheers,

Michael

> 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 f57d7fe..bfd9cab 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -162,6 +162,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);
>
> @@ -180,6 +182,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 a19ddac..41b083c 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 cf1019e..041c3c3 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 58ae9f4..ac3ed1c 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 c8e0e05..a151c35 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -1003,6 +1003,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 afaeddf..1856f69 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>
> @@ -807,6 +809,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-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
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]

#1222884 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

Hi Michael,

On Fri, Sep 11, 2015 at 02:08:50PM +0200, Michael Kerrisk (man-pages) wrote:
> HI Tycho
> 
> On 11 September 2015 at 02:21, 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.
> 
> Do you have a man- page patch for this change?

Not yet (r.e. all the man page reqs), I can draft them asap, though.
Hopefully the API is mostly stable at this point :).

Thanks,

Tycho

> Cheers,
> 
> Michael
> 
> > 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 f57d7fe..bfd9cab 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -162,6 +162,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);
> >
> > @@ -180,6 +182,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 a19ddac..41b083c 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 cf1019e..041c3c3 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 58ae9f4..ac3ed1c 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 c8e0e05..a151c35 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -1003,6 +1003,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 afaeddf..1856f69 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>
> > @@ -807,6 +809,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-api" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> -- 
> 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]

#1222955 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

On Sep 10, 2015 5:22 PM, "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.
>

> +
> +       fd = bpf_new_fd(filter->prog, O_RDONLY);
> +       if (fd > 0)
> +               atomic_inc(&filter->prog->aux->refcnt);

Why isn't this folded into bpf_new_fd?

> +
> +       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);

This lets you take an fd pointing to one prog and point it elsewhere.
I'm not sure that's a good idea.

--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]

#1222970 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

On Fri, Sep 11, 2015 at 09:20:55AM -0700, Andy Lutomirski wrote:
> On Sep 10, 2015 5:22 PM, "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.
> >
> 
> > +
> > +       fd = bpf_new_fd(filter->prog, O_RDONLY);
> > +       if (fd > 0)
> > +               atomic_inc(&filter->prog->aux->refcnt);
> 
> Why isn't this folded into bpf_new_fd?

No reason it can't be as far as I can see. I'll make the change for
the next version.

> > +
> > +       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);
> 
> This lets you take an fd pointing to one prog and point it elsewhere.
> I'm not sure that's a good idea.

That's how the interface was designed (calling ptrace(NEXT_FILTER, fd) and
then doing bpf(DUMP, fd)). I suppose we could have NEXT_FILTER return
a new fd instead if that seems better to you.

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]

#1224381 — Re: [PATCH v2 4/5] seccomp: add a way to access filters via bpf fds

On Sep 11, 2015 9:44 AM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
>
> On Fri, Sep 11, 2015 at 09:20:55AM -0700, Andy Lutomirski wrote:
> > On Sep 10, 2015 5:22 PM, "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.
> > >
> >
> > > +
> > > +       fd = bpf_new_fd(filter->prog, O_RDONLY);
> > > +       if (fd > 0)
> > > +               atomic_inc(&filter->prog->aux->refcnt);
> >
> > Why isn't this folded into bpf_new_fd?
>
> No reason it can't be as far as I can see. I'll make the change for
> the next version.
>
> > > +
> > > +       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);
> >
> > This lets you take an fd pointing to one prog and point it elsewhere.
> > I'm not sure that's a good idea.
>
> That's how the interface was designed (calling ptrace(NEXT_FILTER, fd) and
> then doing bpf(DUMP, fd)). I suppose we could have NEXT_FILTER return
> a new fd instead if that seems better to you.

It'll be slower, but it avoids a weird side effect.

>
> 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]

#1222502

On Thu, Sep 10, 2015 at 06:20:57PM -0600, Tycho Andersen wrote:
> Hi all,
> 
> Here is v2 of the seccomp filter c/r set. The patch notes have individual
> changes from the last series, but there are two points not noted:
> 
> * The series still does not allow us to correctly restore state for programs
>   that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
>   keep seccomp_filter's identity, I think something along the lines of another
>   seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
>   if this can even be done yet). In addition, we'll need a kcmp command for
>   figuring out if filters are the same, although this too needs to compare
>   seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
>   this nicely are welcome.
> 
> * I've dropped the bpf converter bug from the set and will submit it
>   separately.
> 
> Alexei mentioned that this should go via net-next to minimize cross-tree
> conflicts. Does that make sense here?

Having looked at the set again I already see conflicts in net/core/filter.c
and in linux/bpf.h with things myself and others are working on for net-next.
So I think it makes the most sense to get the whole set via net-next,
since seccomp bits look limited comparing to bpf changes.
Otherwise the merge window will be unpleasant.

--
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]

#1222965

On Sep 10, 2015 5:22 PM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
>
> Hi all,
>
> Here is v2 of the seccomp filter c/r set. The patch notes have individual
> changes from the last series, but there are two points not noted:
>
> * The series still does not allow us to correctly restore state for programs
>   that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
>   keep seccomp_filter's identity, I think something along the lines of another
>   seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
>   if this can even be done yet). In addition, we'll need a kcmp command for
>   figuring out if filters are the same, although this too needs to compare
>   seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
>   this nicely are welcome.

Let's add a concept of a seccompfd.

For background of what I want to add: I want to be able to create a
seccomp monitor.  A seccomp monitor will be, logically, a pair of a
struct file that represents the monitor and a seccomp_filter that is
controlled by the monitor.  Depending on flags, whoever holds the
monitor fd could change the active filter, intercept syscalls, and
issue syscalls on behalf of a process that is trapped in an
intercepted syscall.

Seccomp filters would nest properly.

The interface would probably be (extremely pseudocoded):

monitor_fd, filter_fd = seccomp(CREATE_MONITOR, flags, ...);

Then, later:

seccomp(ATTACH_TO_FILTER, filter_fd);  /* now filtered */

read(monitor_fd, buf, size); /* returns an intercepted syscall */
write(monitor_fd, buf, size); /* issues a syscall or releases the
trapped task */

This can't be implemented on x86 without either going insane or
finishing the massive set of pending cleanups to the x86 entry code.
I favor the latter.

We could, however, add part of it right now: we could have a way to
create a filterfd, we could add kcmp support for it, and we could add
the ATTACH_TO_FILTER thing.  I think that would solve your problem.

One major open question: does a filter_fd know what its parent is and,
if so, will it just refuse to attach if the caller's parent is wrong?
Or will a filter_fd attach anywhere.

--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]

#1222985

On Fri, Sep 11, 2015 at 9:30 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Sep 10, 2015 5:22 PM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
>>
>> Hi all,
>>
>> Here is v2 of the seccomp filter c/r set. The patch notes have individual
>> changes from the last series, but there are two points not noted:
>>
>> * The series still does not allow us to correctly restore state for programs
>>   that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
>>   keep seccomp_filter's identity, I think something along the lines of another
>>   seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
>>   if this can even be done yet). In addition, we'll need a kcmp command for
>>   figuring out if filters are the same, although this too needs to compare
>>   seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
>>   this nicely are welcome.
>
> Let's add a concept of a seccompfd.
>
> For background of what I want to add: I want to be able to create a
> seccomp monitor.  A seccomp monitor will be, logically, a pair of a
> struct file that represents the monitor and a seccomp_filter that is
> controlled by the monitor.  Depending on flags, whoever holds the
> monitor fd could change the active filter, intercept syscalls, and
> issue syscalls on behalf of a process that is trapped in an
> intercepted syscall.
>
> Seccomp filters would nest properly.
>
> The interface would probably be (extremely pseudocoded):
>
> monitor_fd, filter_fd = seccomp(CREATE_MONITOR, flags, ...);
>
> Then, later:
>
> seccomp(ATTACH_TO_FILTER, filter_fd);  /* now filtered */
>
> read(monitor_fd, buf, size); /* returns an intercepted syscall */
> write(monitor_fd, buf, size); /* issues a syscall or releases the
> trapped task */
>
> This can't be implemented on x86 without either going insane or
> finishing the massive set of pending cleanups to the x86 entry code.
> I favor the latter.
>
> We could, however, add part of it right now: we could have a way to
> create a filterfd, we could add kcmp support for it, and we could add
> the ATTACH_TO_FILTER thing.  I think that would solve your problem.
>
> One major open question: does a filter_fd know what its parent is and,
> if so, will it just refuse to attach if the caller's parent is wrong?
> Or will a filter_fd attach anywhere.
>

Let me add one more thought:

Currently, struct seccomp_filter encodes a strict tree hierarchy: it
knows what its parent is.  This only matters as an implementation
detail and because TSYNC checks for seccomp_filter equality.

We could change this without user-visible effects.  We could say that,
for TSYNC purposes, two filter states match if they contain exactly
the same layers in the same order where a layer does *not* encode a
concept of parent.  We could then say that attaching a classic bpf
filter creates a branch new layer that is not equal to any other layer
that's been created.

This has no effect whatsoever.  The difference would be that we could
declare that attaching the same ebpf program twice creates the *same*
layer so that, if you fork and both children attach the same ebpf
program, then they match for TSYNC purposes.  Similarly, attaching the
same hypothetical filterfd would create the same layer.

Thoughts?

--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]

#1222999

On Fri, Sep 11, 2015 at 10:00:22AM -0700, Andy Lutomirski wrote:
> On Fri, Sep 11, 2015 at 9:30 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > On Sep 10, 2015 5:22 PM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
> >>
> >> Hi all,
> >>
> >> Here is v2 of the seccomp filter c/r set. The patch notes have individual
> >> changes from the last series, but there are two points not noted:
> >>
> >> * The series still does not allow us to correctly restore state for programs
> >>   that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
> >>   keep seccomp_filter's identity, I think something along the lines of another
> >>   seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
> >>   if this can even be done yet). In addition, we'll need a kcmp command for
> >>   figuring out if filters are the same, although this too needs to compare
> >>   seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
> >>   this nicely are welcome.
> >
> > Let's add a concept of a seccompfd.
> >
> > For background of what I want to add: I want to be able to create a
> > seccomp monitor.  A seccomp monitor will be, logically, a pair of a
> > struct file that represents the monitor and a seccomp_filter that is
> > controlled by the monitor.  Depending on flags, whoever holds the
> > monitor fd could change the active filter, intercept syscalls, and
> > issue syscalls on behalf of a process that is trapped in an
> > intercepted syscall.
> >
> > Seccomp filters would nest properly.
> >
> > The interface would probably be (extremely pseudocoded):
> >
> > monitor_fd, filter_fd = seccomp(CREATE_MONITOR, flags, ...);
> >
> > Then, later:
> >
> > seccomp(ATTACH_TO_FILTER, filter_fd);  /* now filtered */
> >
> > read(monitor_fd, buf, size); /* returns an intercepted syscall */
> > write(monitor_fd, buf, size); /* issues a syscall or releases the
> > trapped task */
> >
> > This can't be implemented on x86 without either going insane or
> > finishing the massive set of pending cleanups to the x86 entry code.
> > I favor the latter.
> >
> > We could, however, add part of it right now: we could have a way to
> > create a filterfd, we could add kcmp support for it, and we could add
> > the ATTACH_TO_FILTER thing.  I think that would solve your problem.
> >
> > One major open question: does a filter_fd know what its parent is and,
> > if so, will it just refuse to attach if the caller's parent is wrong?
> > Or will a filter_fd attach anywhere.
> >
> 
> Let me add one more thought:
> 
> Currently, struct seccomp_filter encodes a strict tree hierarchy: it
> knows what its parent is.  This only matters as an implementation
> detail and because TSYNC checks for seccomp_filter equality.
> 
> We could change this without user-visible effects.  We could say that,
> for TSYNC purposes, two filter states match if they contain exactly
> the same layers in the same order where a layer does *not* encode a
> concept of parent.  We could then say that attaching a classic bpf
> filter creates a branch new layer that is not equal to any other layer
> that's been created.
> 
> This has no effect whatsoever.  The difference would be that we could
> declare that attaching the same ebpf program twice creates the *same*
> layer so that, if you fork and both children attach the same ebpf
> program, then they match for TSYNC purposes.

Would you keep struct seccomp_filter identity here (meaning that you'd
reach over and grab the seccomp_filter from a sibling thread if it
existed)? Would it only work for the last filter attached to siblings,
or for all the filters? This does make my life easier, but I like the
idea of just using seccompfd directly below as it seems somewhat
easier (for me at least) to understand,

> Similarly, attaching the
> same hypothetical filterfd would create the same layer.

If we change the api of my current set to have the ptrace commands
iterate over seccomp fds, it looks something like:

seccompfd = ptrace(GET_FILTER_FD, pid);
while (ptrace(NEXT_FD, pid, seccompfd) == 0) {
        if (seccomp(CHECK_INHERITED, seccompfd))
                break;

        bpffd = seccomp(GET_BPF_FD, seccompfd);
        err = buf(BPF_PROG_DUMP, bpffd, &attr);
        /* save the bpf prog */
}

then restore can look like:

while (have_noninherited_filters()) {
        filter = load_filter();
        bpffd = bpf(BPF_PROG_LOAD, filter);
        seccompfd = seccomp(SECCOMP_FD_CREATE, bpffd);

        filters[n_filters++] = seccompfd;
}

/* fork any children as necessary and do the rest of the restore */

for (i = 0; i < n_filters; i++) {
        seccomp(SECCOMP_FD_INSTALL, filters[i]);
}

then the only question is how to implement the CHECK_INHERITED command
on dump.

If we support the above API, we don't need to think about the concept
of layers at all, or do any extra work on filter install to preserve
struct seccomp_filter identity, it just comes naturally.

Tycho

> Thoughts?
> 
> --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]

#1224376

On Sep 11, 2015 10:28 AM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
>
> On Fri, Sep 11, 2015 at 10:00:22AM -0700, Andy Lutomirski wrote:
> > On Fri, Sep 11, 2015 at 9:30 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > > On Sep 10, 2015 5:22 PM, "Tycho Andersen" <tycho.andersen@canonical.com> wrote:
> > >>
> > >> Hi all,
> > >>
> > >> Here is v2 of the seccomp filter c/r set. The patch notes have individual
> > >> changes from the last series, but there are two points not noted:
> > >>
> > >> * The series still does not allow us to correctly restore state for programs
> > >>   that will use SECCOMP_FILTER_FLAG_TSYNC in the future. Given that we want to
> > >>   keep seccomp_filter's identity, I think something along the lines of another
> > >>   seccomp command like SECCOMP_INHERIT_PARENT is needed (although I'm not sure
> > >>   if this can even be done yet). In addition, we'll need a kcmp command for
> > >>   figuring out if filters are the same, although this too needs to compare
> > >>   seccomp_filter objects, so it's a little screwy. Any thoughts on how to do
> > >>   this nicely are welcome.
> > >
> > > Let's add a concept of a seccompfd.
> > >
> > > For background of what I want to add: I want to be able to create a
> > > seccomp monitor.  A seccomp monitor will be, logically, a pair of a
> > > struct file that represents the monitor and a seccomp_filter that is
> > > controlled by the monitor.  Depending on flags, whoever holds the
> > > monitor fd could change the active filter, intercept syscalls, and
> > > issue syscalls on behalf of a process that is trapped in an
> > > intercepted syscall.
> > >
> > > Seccomp filters would nest properly.
> > >
> > > The interface would probably be (extremely pseudocoded):
> > >
> > > monitor_fd, filter_fd = seccomp(CREATE_MONITOR, flags, ...);
> > >
> > > Then, later:
> > >
> > > seccomp(ATTACH_TO_FILTER, filter_fd);  /* now filtered */
> > >
> > > read(monitor_fd, buf, size); /* returns an intercepted syscall */
> > > write(monitor_fd, buf, size); /* issues a syscall or releases the
> > > trapped task */
> > >
> > > This can't be implemented on x86 without either going insane or
> > > finishing the massive set of pending cleanups to the x86 entry code.
> > > I favor the latter.
> > >
> > > We could, however, add part of it right now: we could have a way to
> > > create a filterfd, we could add kcmp support for it, and we could add
> > > the ATTACH_TO_FILTER thing.  I think that would solve your problem.
> > >
> > > One major open question: does a filter_fd know what its parent is and,
> > > if so, will it just refuse to attach if the caller's parent is wrong?
> > > Or will a filter_fd attach anywhere.
> > >
> >
> > Let me add one more thought:
> >
> > Currently, struct seccomp_filter encodes a strict tree hierarchy: it
> > knows what its parent is.  This only matters as an implementation
> > detail and because TSYNC checks for seccomp_filter equality.
> >
> > We could change this without user-visible effects.  We could say that,
> > for TSYNC purposes, two filter states match if they contain exactly
> > the same layers in the same order where a layer does *not* encode a
> > concept of parent.  We could then say that attaching a classic bpf
> > filter creates a branch new layer that is not equal to any other layer
> > that's been created.
> >
> > This has no effect whatsoever.  The difference would be that we could
> > declare that attaching the same ebpf program twice creates the *same*
> > layer so that, if you fork and both children attach the same ebpf
> > program, then they match for TSYNC purposes.
>
> Would you keep struct seccomp_filter identity here (meaning that you'd
> reach over and grab the seccomp_filter from a sibling thread if it
> existed)? Would it only work for the last filter attached to siblings,
> or for all the filters? This does make my life easier, but I like the
> idea of just using seccompfd directly below as it seems somewhat
> easier (for me at least) to understand,
>

If we did that, it would just be an internal optimization.

> > Similarly, attaching the
> > same hypothetical filterfd would create the same layer.
>
> If we change the api of my current set to have the ptrace commands
> iterate over seccomp fds, it looks something like:
>
> seccompfd = ptrace(GET_FILTER_FD, pid);
> while (ptrace(NEXT_FD, pid, seccompfd) == 0) {
>         if (seccomp(CHECK_INHERITED, seccompfd))
>                 break;
>
>         bpffd = seccomp(GET_BPF_FD, seccompfd);
>         err = buf(BPF_PROG_DUMP, bpffd, &attr);
>         /* save the bpf prog */
> }
>
> then restore can look like:
>
> while (have_noninherited_filters()) {
>         filter = load_filter();
>         bpffd = bpf(BPF_PROG_LOAD, filter);
>         seccompfd = seccomp(SECCOMP_FD_CREATE, bpffd);
>
>         filters[n_filters++] = seccompfd;
> }
>
> /* fork any children as necessary and do the rest of the restore */
>
> for (i = 0; i < n_filters; i++) {
>         seccomp(SECCOMP_FD_INSTALL, filters[i]);
> }
>
> then the only question is how to implement the CHECK_INHERITED command
> on dump.

I don't think it would be a well defined operation.  I think you'd
have to ask "for this pid, give me the nth thing in the stack", since
an fd identifying a layer without reference to its parent would no
longer even be guaranteed to be unique in the filter stack for a given
task.

I'm not sure I entirely like this solution...

>
> If we support the above API, we don't need to think about the concept
> of layers at all, or do any extra work on filter install to preserve
> struct seccomp_filter identity, it just comes naturally.
>
> Tycho
>
> > Thoughts?
> >
> > --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]

#1225382

Hi Andy,

On Mon, Sep 14, 2015 at 10:52:46AM -0700, Andy Lutomirski wrote:
>
> I'm not sure I entirely like this solution...

Ok. Since we also aren't going to do all the eBPF stuff now, how about
something that looks like this:

struct seccomp_layer {
  unsigned int size;
  unsigned int type; /* SECCOMP_BPF_CLASSIC or SECCOMP_EBPF or ... */
  bool inherited;
  union {
    unsigned int insn_cnt;
    struct bpf_insn *insns;
  };
};

with a ptrace command:

ptrace(PTRACE_SECCOMP_DUMP_LAYER, pid, i, &layer);

If we save a pointer to the current seccomp filter on fork (if there
is one), then I think the inherited flag is just,

inherited = is_ancestor(child->seccomp.filter, child->seccomp.inherited_filter)

In order to restore this (so it can be checkpointed again), we need a
command that looks like:

seccomp(SECCOMP_INHERIT_FILTER);

which sets the current and inherited filter to that of the parent
process. (Optionally we could have seccomp(SECCOMP_INHERIT_FILTER, i)
to inherit the ith filter from the parent, but we can coordinate this
via userpace so it's not strictly necessary.) So the whole c/r process
looks something like:

--- dump ---

for (i = 0; true; i++) {
  ret = ptrace(PTRACE_SECCOMP_DUMP_FILTER, pid, i, &layer);
  if (ret == -ESRCH)
    break;
  if (ret < 0)
    /* real error */

  /* save the filter if it's not inherited, if it is, mark the filter
   * to be inherited from ppid; note that this index is walking up the
   * tree following filter->prev, and the index we want to reason
   * about on restore is walking down, so we should reverse the whole
   * array.
   */
}

--- restore ---

if (have_inherited_filters) {
  wait_for_ppid_seccomp_restore(n_inherited);
  seccomp(SECCOMP_INHERIT_FILTER);
  signal_done_inheriting();
}

for (i = 0; i < n_filters; i++) {
  seccomp(SECCOMP_SET_MODE_FILTER, ...);
  if (child_inherited_filter(i))
    signal_children_filter(i);
}

I played around with an implementation of SECCOMP_INHERIT_FILTER last
night and I think I have one that might work. Thoughts?

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]

#1225484

On Tue, Sep 15, 2015 at 9:07 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> Hi Andy,
>
> On Mon, Sep 14, 2015 at 10:52:46AM -0700, Andy Lutomirski wrote:
>>
>> I'm not sure I entirely like this solution...
>
> Ok. Since we also aren't going to do all the eBPF stuff now, how about
> something that looks like this:
>
> struct seccomp_layer {
>   unsigned int size;
>   unsigned int type; /* SECCOMP_BPF_CLASSIC or SECCOMP_EBPF or ... */
>   bool inherited;
>   union {
>     unsigned int insn_cnt;
>     struct bpf_insn *insns;
>   };
> };
>
> with a ptrace command:
>
> ptrace(PTRACE_SECCOMP_DUMP_LAYER, pid, i, &layer);
>
> If we save a pointer to the current seccomp filter on fork (if there
> is one), then I think the inherited flag is just,
>
> inherited = is_ancestor(child->seccomp.filter, child->seccomp.inherited_filter)
>

I'm lost.  What is the inherited flag for?

--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]

#1225493

Hi Andy,

On Tue, Sep 15, 2015 at 11:13:51AM -0700, Andy Lutomirski wrote:
> On Tue, Sep 15, 2015 at 9:07 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > Hi Andy,
> >
> > On Mon, Sep 14, 2015 at 10:52:46AM -0700, Andy Lutomirski wrote:
> >>
> >> I'm not sure I entirely like this solution...
> >
> > Ok. Since we also aren't going to do all the eBPF stuff now, how about
> > something that looks like this:
> >
> > struct seccomp_layer {
> >   unsigned int size;
> >   unsigned int type; /* SECCOMP_BPF_CLASSIC or SECCOMP_EBPF or ... */
> >   bool inherited;
> >   union {
> >     unsigned int insn_cnt;
> >     struct bpf_insn *insns;
> >   };
> > };
> >
> > with a ptrace command:
> >
> > ptrace(PTRACE_SECCOMP_DUMP_LAYER, pid, i, &layer);
> >
> > If we save a pointer to the current seccomp filter on fork (if there
> > is one), then I think the inherited flag is just,
> >
> > inherited = is_ancestor(child->seccomp.filter, child->seccomp.inherited_filter)
> >
> 
> I'm lost.  What is the inherited flag for?

We need some way to expose the seccomp hierarchy, specifically which
filters are inherited, so that we can correctly restore the filter
tree for tasks that may use TSYNC in the future. You've mentioned that
you don't like kcmp, so this is an alternative to that.

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]

#1225531

On Tue, Sep 15, 2015 at 11:26 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> Hi Andy,
>
> On Tue, Sep 15, 2015 at 11:13:51AM -0700, Andy Lutomirski wrote:
>> On Tue, Sep 15, 2015 at 9:07 AM, Tycho Andersen
>> <tycho.andersen@canonical.com> wrote:
>> > Hi Andy,
>> >
>> > On Mon, Sep 14, 2015 at 10:52:46AM -0700, Andy Lutomirski wrote:
>> >>
>> >> I'm not sure I entirely like this solution...
>> >
>> > Ok. Since we also aren't going to do all the eBPF stuff now, how about
>> > something that looks like this:
>> >
>> > struct seccomp_layer {
>> >   unsigned int size;
>> >   unsigned int type; /* SECCOMP_BPF_CLASSIC or SECCOMP_EBPF or ... */
>> >   bool inherited;
>> >   union {
>> >     unsigned int insn_cnt;
>> >     struct bpf_insn *insns;
>> >   };
>> > };
>> >
>> > with a ptrace command:
>> >
>> > ptrace(PTRACE_SECCOMP_DUMP_LAYER, pid, i, &layer);
>> >
>> > If we save a pointer to the current seccomp filter on fork (if there
>> > is one), then I think the inherited flag is just,
>> >
>> > inherited = is_ancestor(child->seccomp.filter, child->seccomp.inherited_filter)
>> >
>>
>> I'm lost.  What is the inherited flag for?
>
> We need some way to expose the seccomp hierarchy, specifically which
> filters are inherited, so that we can correctly restore the filter
> tree for tasks that may use TSYNC in the future. You've mentioned that
> you don't like kcmp, so this is an alternative to that.
>

My only objection to kcmp is that IMO it's a suboptimal interface and
could be better.  I have no problem with the general principle of
asking to compare two objects.

The thing I really don't have a good handle on is whether the seccomp
filter hierarchy should look more like A:

struct seccomp_filter {
    ...;
    struct seccomp_filter *prev;
};

with the seccomp_filter being the user-visible object

Or B:

struct seccomp_layer {
   ...;  /* BPF program, etc. */
}

struct seccomp_filter {
   struct seccomp_layer *layer;
   struct seccomp_filter *prev;
};  /* or equivalent */

with seccomp_layer being the user-visible object.

A is simpler to implement in a memory-efficient way, but it's less
flexible.  I haven't come up with a compelling use case for B where A
doesn't work, with the caveat that, if an fd points to a
seccomp_filter in model A, you can't attach it unless your current
state matches its "prev" state (or an ancestor thereof), which might
be a little bit awkward.

Am I making more sense now?

--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]

#1225559

Hi Andy,

On Tue, Sep 15, 2015 at 01:01:23PM -0700, Andy Lutomirski wrote:
> On Tue, Sep 15, 2015 at 11:26 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > Hi Andy,
> >
> > On Tue, Sep 15, 2015 at 11:13:51AM -0700, Andy Lutomirski wrote:
> >> On Tue, Sep 15, 2015 at 9:07 AM, Tycho Andersen
> >> <tycho.andersen@canonical.com> wrote:
> >> > Hi Andy,
> >> >
> >> > On Mon, Sep 14, 2015 at 10:52:46AM -0700, Andy Lutomirski wrote:
> >> >>
> >> >> I'm not sure I entirely like this solution...
> >> >
> >> > Ok. Since we also aren't going to do all the eBPF stuff now, how about
> >> > something that looks like this:
> >> >
> >> > struct seccomp_layer {
> >> >   unsigned int size;
> >> >   unsigned int type; /* SECCOMP_BPF_CLASSIC or SECCOMP_EBPF or ... */
> >> >   bool inherited;
> >> >   union {
> >> >     unsigned int insn_cnt;
> >> >     struct bpf_insn *insns;
> >> >   };
> >> > };
> >> >
> >> > with a ptrace command:
> >> >
> >> > ptrace(PTRACE_SECCOMP_DUMP_LAYER, pid, i, &layer);
> >> >
> >> > If we save a pointer to the current seccomp filter on fork (if there
> >> > is one), then I think the inherited flag is just,
> >> >
> >> > inherited = is_ancestor(child->seccomp.filter, child->seccomp.inherited_filter)
> >> >
> >>
> >> I'm lost.  What is the inherited flag for?
> >
> > We need some way to expose the seccomp hierarchy, specifically which
> > filters are inherited, so that we can correctly restore the filter
> > tree for tasks that may use TSYNC in the future. You've mentioned that
> > you don't like kcmp, so this is an alternative to that.
> >
> 
> My only objection to kcmp is that IMO it's a suboptimal interface and
> could be better.  I have no problem with the general principle of
> asking to compare two objects.

Ok, in that case I think we can get rid of all the inherited stuff,
and use kcmp to figure it out.

> The thing I really don't have a good handle on is whether the seccomp
> filter hierarchy should look more like A:
> 
> struct seccomp_filter {
>     ...;
>     struct seccomp_filter *prev;
> };
> 
> with the seccomp_filter being the user-visible object
> 
> Or B:
> 
> struct seccomp_layer {
>    ...;  /* BPF program, etc. */
> }
> 
> struct seccomp_filter {
>    struct seccomp_layer *layer;
>    struct seccomp_filter *prev;
> };  /* or equivalent */
> 
> with seccomp_layer being the user-visible object.
> 
> A is simpler to implement in a memory-efficient way, but it's less
> flexible.  I haven't come up with a compelling use case for B where A
> doesn't work, with the caveat that, if an fd points to a
> seccomp_filter in model A, you can't attach it unless your current
> state matches its "prev" state (or an ancestor thereof), which might
> be a little bit awkward.

Perhaps, although I don't think it would be an issue for c/r.

> Am I making more sense now?

Yes, thanks for the clarifications. I guess personally I'd probably
choose option A. If this (using kcmp and one of A/B) sounds good to
you, I'll start working on a set to do c/r that way.

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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web