[bpf-next,1/6] bpf: Introduce BPF_PROG_TYPE_SECCOMP

Commit Message

Hengqi Chen Oct. 31, 2023, 1:24 a.m. UTC

This adds minimal support for seccomp eBPF programs
which can be hooked into the existing seccomp framework.
This allows users to write seccomp filter in eBPF language
and enables seccomp filter reuse through bpf prog fd and
bpffs. Currently, no helper calls are allowed just like
its cBPF version.

Signed-off-by: Hengqi Chen <[e-mail]>
---
 include/linux/bpf_types.h     |  4 +++
 include/uapi/linux/bpf.h      |  1 +
 kernel/seccomp.c              | 54 +++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.c        |  2 ++
 tools/lib/bpf/libbpf_probes.c |  1 +
 5 files changed, 62 insertions(+)

Comments

Andrii Nakryiko Nov. 2, 2023, 5:30 p.m. UTC | #1

On Mon, Oct 30, 2023 at 11:00 PM Hengqi Chen <[e-mail]> wrote:
>
> This adds minimal support for seccomp eBPF programs
> which can be hooked into the existing seccomp framework.
> This allows users to write seccomp filter in eBPF language
> and enables seccomp filter reuse through bpf prog fd and
> bpffs. Currently, no helper calls are allowed just like
> its cBPF version.
>
> Signed-off-by: Hengqi Chen <[e-mail]>
> ---
>  include/linux/bpf_types.h     |  4 +++
>  include/uapi/linux/bpf.h      |  1 +
>  kernel/seccomp.c              | 54 +++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.c        |  2 ++
>  tools/lib/bpf/libbpf_probes.c |  1 +
>  5 files changed, 62 insertions(+)
>

Let's make sure that bpf_prog_load_check_attach() errors out on
non-zero attach type for this new program type?

Ideally, if you can, let's refactor bpf_prog_load_check_attach() in
such a way as to default to failing on non-zero attach type for any
new program type. You'll need to explicitly list program types for
which we don't enforce attach type.

Thanks!

> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index fc0d6f32c687..7c0a9fc0b150 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -83,6 +83,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
>  BPF_PROG_TYPE(BPF_PROG_TYPE_NETFILTER, netfilter,
>               struct bpf_nf_ctx, struct bpf_nf_ctx)
>  #endif
> +#ifdef CONFIG_SECCOMP_FILTER
> +BPF_PROG_TYPE(BPF_PROG_TYPE_SECCOMP, seccomp,
> +             struct seccomp_data, struct seccomp_data)
> +#endif
>
>  BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
>  BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0f6cdf52b1da..f0fcfe0ccb2e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -995,6 +995,7 @@ enum bpf_prog_type {
>         BPF_PROG_TYPE_SK_LOOKUP,
>         BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
>         BPF_PROG_TYPE_NETFILTER,
> +       BPF_PROG_TYPE_SECCOMP,
>  };
>
>  enum bpf_attach_type {
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 255999ba9190..5a6ed8630566 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -15,6 +15,7 @@
>   */
>  #define pr_fmt(fmt) "seccomp: " fmt
>
> +#include <linux/bpf.h>
>  #include <linux/refcount.h>
>  #include <linux/audit.h>
>  #include <linux/compat.h>
> @@ -2513,3 +2514,56 @@ int proc_pid_seccomp_cache(struct seq_file *m, struct pid_namespace *ns,
>         return 0;
>  }
>  #endif /* CONFIG_SECCOMP_CACHE_DEBUG */
> +
> +#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_BPF_SYSCALL)
> +const struct bpf_prog_ops seccomp_prog_ops = {
> +};
> +
> +static bool seccomp_is_valid_access(int off, int size, enum bpf_access_type type,
> +                                   const struct bpf_prog *prog,
> +                                   struct bpf_insn_access_aux *info)
> +{
> +       if (off < 0 || off >= sizeof(struct seccomp_data))
> +               return false;
> +
> +       if (off % size != 0)
> +               return false;
> +
> +       if (type == BPF_WRITE)
> +               return false;
> +
> +       switch (off) {
> +       case bpf_ctx_range(struct seccomp_data, nr):
> +               if (size != sizeof_field(struct seccomp_data, nr))
> +                       return false;
> +               return true;
> +       case bpf_ctx_range(struct seccomp_data, arch):
> +               if (size != sizeof_field(struct seccomp_data, arch))
> +                       return false;
> +               return true;
> +       case bpf_ctx_range(struct seccomp_data, instruction_pointer):
> +               if (size != sizeof_field(struct seccomp_data, instruction_pointer))
> +                       return false;
> +               return true;
> +       case bpf_ctx_range(struct seccomp_data, args):
> +               if (size != sizeof(__u64))
> +                       return false;
> +               return true;
> +       default:
> +               return false;
> +       }
> +
> +       return false;
> +}
> +
> +static const struct bpf_func_proto *
> +bpf_seccomp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> +{
> +       return NULL;
> +}
> +
> +const struct bpf_verifier_ops seccomp_verifier_ops = {
> +       .is_valid_access = seccomp_is_valid_access,
> +       .get_func_proto  = bpf_seccomp_func_proto,
> +};
> +#endif /* CONFIG_SECCOMP_FILTER && CONFIG_BPF_SYSCALL */
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e067be95da3c..455d733f7315 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -217,6 +217,7 @@ static const char * const prog_type_name[] = {
>         [BPF_PROG_TYPE_SK_LOOKUP]               = "sk_lookup",
>         [BPF_PROG_TYPE_SYSCALL]                 = "syscall",
>         [BPF_PROG_TYPE_NETFILTER]               = "netfilter",
> +       [BPF_PROG_TYPE_SECCOMP]                 = "seccomp",
>  };
>
>  static int __base_pr(enum libbpf_print_level level, const char *format,
> @@ -8991,6 +8992,7 @@ static const struct bpf_sec_def section_defs[] = {
>         SEC_DEF("struct_ops.s+",        STRUCT_OPS, 0, SEC_SLEEPABLE),
>         SEC_DEF("sk_lookup",            SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE),
>         SEC_DEF("netfilter",            NETFILTER, BPF_NETFILTER, SEC_NONE),
> +       SEC_DEF("seccomp",              SECCOMP, 0, SEC_NONE),
>  };
>
>  int libbpf_register_prog_handler(const char *sec,
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 9c4db90b92b6..b3ef3c0747be 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -180,6 +180,7 @@ static int probe_prog_load(enum bpf_prog_type prog_type,
>         case BPF_PROG_TYPE_SK_REUSEPORT:
>         case BPF_PROG_TYPE_FLOW_DISSECTOR:
>         case BPF_PROG_TYPE_CGROUP_SYSCTL:
> +       case BPF_PROG_TYPE_SECCOMP:
>                 break;
>         case BPF_PROG_TYPE_NETFILTER:
>                 opts.expected_attach_type = BPF_NETFILTER;
> --
> 2.34.1
>

Kees Cook Nov. 2, 2023, 7:49 p.m. UTC | #2

On October 30, 2023 6:24:02 PM PDT, Hengqi Chen <[e-mail]> wrote:
>This adds minimal support for seccomp eBPF programs
>which can be hooked into the existing seccomp framework.
>This allows users to write seccomp filter in eBPF language
>and enables seccomp filter reuse through bpf prog fd and
>bpffs. Currently, no helper calls are allowed just like
>its cBPF version.

I think this is bypassing the seccomp bitmap generation pass, so this will break (at least) performance.

I continue to prefer sticking to only cBPF for seccomp, so let's just use the seccomp syscall to generate the fds.

-Kees

Alexei Starovoitov Nov. 2, 2023, 7:53 p.m. UTC | #3

On Thu, Nov 2, 2023 at 12:49 PM Kees Cook <[e-mail]> wrote:
>
>
>
> On October 30, 2023 6:24:02 PM PDT, Hengqi Chen <[e-mail]> wrote:
> >This adds minimal support for seccomp eBPF programs
> >which can be hooked into the existing seccomp framework.
> >This allows users to write seccomp filter in eBPF language
> >and enables seccomp filter reuse through bpf prog fd and
> >bpffs. Currently, no helper calls are allowed just like
> >its cBPF version.
>
> I think this is bypassing the seccomp bitmap generation pass, so this will break (at least) performance.
>
> I continue to prefer sticking to only cBPF for seccomp, so let's just use the seccomp syscall to generate the fds.

That's fine, but let's not mix old things with bpffs, bpftool, etc.
If you want an anon_fd then go ahead and allocate it standalone.
It shouldn't be confused with eBPF fd-s.
No bpffs treatment and no bpftool visibility.

Hengqi Chen Nov. 3, 2023, 5:46 a.m. UTC | #4

Hi, Kees:

On Fri, Nov 3, 2023 at 3:49 AM Kees Cook <[e-mail]> wrote:
>
>
>
> On October 30, 2023 6:24:02 PM PDT, Hengqi Chen <[e-mail]> wrote:
> >This adds minimal support for seccomp eBPF programs
> >which can be hooked into the existing seccomp framework.
> >This allows users to write seccomp filter in eBPF language
> >and enables seccomp filter reuse through bpf prog fd and
> >bpffs. Currently, no helper calls are allowed just like
> >its cBPF version.
>
> I think this is bypassing the seccomp bitmap generation pass, so this will break (at least) performance.
>

What if we did the same for eBPF, a bit harder though, does that
address your concerns ?

> I continue to prefer sticking to only cBPF for seccomp, so let's just use the seccomp syscall to generate the fds.
>

That's an alternative. But as Alexei said, there would be no more bpffs things.
AFAIK, we could only share the filter via UDS.

> -Kees
>
> --
> Kees Cook

Hengqi Chen Nov. 3, 2023, 8:47 a.m. UTC | #5

On Fri, Nov 3, 2023 at 1:46 PM Hengqi Chen <[e-mail]> wrote:
>
> Hi, Kees:
>
> On Fri, Nov 3, 2023 at 3:49 AM Kees Cook <[e-mail]> wrote:
> >
> >
> >
> > On October 30, 2023 6:24:02 PM PDT, Hengqi Chen <[e-mail]> wrote:
> > >This adds minimal support for seccomp eBPF programs
> > >which can be hooked into the existing seccomp framework.
> > >This allows users to write seccomp filter in eBPF language
> > >and enables seccomp filter reuse through bpf prog fd and
> > >bpffs. Currently, no helper calls are allowed just like
> > >its cBPF version.
> >
> > I think this is bypassing the seccomp bitmap generation pass, so this will break (at least) performance.
> >
>
> What if we did the same for eBPF, a bit harder though, does that
> address your concerns ?
>
> > I continue to prefer sticking to only cBPF for seccomp, so let's just use the seccomp syscall to generate the fds.
> >
>
> That's an alternative. But as Alexei said, there would be no more bpffs things.
> AFAIK, we could only share the filter via UDS.
>

Just take a deeper look, there are too many
registers/instructions/states in eBPF,
stick to cBPF would be easier for now.

> > -Kees
> >
> > --
> > Kees Cook

Kees Cook Nov. 3, 2023, 8:44 p.m. UTC | #6

On November 2, 2023 12:53:56 PM PDT, Alexei Starovoitov <[e-mail]> wrote:
>On Thu, Nov 2, 2023 at 12:49 PM Kees Cook <[e-mail]> wrote:
>>
>>
>>
>> On October 30, 2023 6:24:02 PM PDT, Hengqi Chen <[e-mail]> wrote:
>> >This adds minimal support for seccomp eBPF programs
>> >which can be hooked into the existing seccomp framework.
>> >This allows users to write seccomp filter in eBPF language
>> >and enables seccomp filter reuse through bpf prog fd and
>> >bpffs. Currently, no helper calls are allowed just like
>> >its cBPF version.
>>
>> I think this is bypassing the seccomp bitmap generation pass, so this will break (at least) performance.
>>
>> I continue to prefer sticking to only cBPF for seccomp, so let's just use the seccomp syscall to generate the fds.
>
>That's fine, but let's not mix old things with bpffs, bpftool, etc.
>If you want an anon_fd then go ahead and allocate it standalone.
>It shouldn't be confused with eBPF fd-s.
>No bpffs treatment and no bpftool visibility.

Agreed. Let's just emit an anon_fd from the seccomp syscall.

Patch

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index fc0d6f32c687..7c0a9fc0b150 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -83,6 +83,10 @@  BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
 BPF_PROG_TYPE(BPF_PROG_TYPE_NETFILTER, netfilter,
 	      struct bpf_nf_ctx, struct bpf_nf_ctx)
 #endif
+#ifdef CONFIG_SECCOMP_FILTER
+BPF_PROG_TYPE(BPF_PROG_TYPE_SECCOMP, seccomp,
+	      struct seccomp_data, struct seccomp_data)
+#endif
 
 BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0f6cdf52b1da..f0fcfe0ccb2e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -995,6 +995,7 @@  enum bpf_prog_type {
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
 	BPF_PROG_TYPE_NETFILTER,
+	BPF_PROG_TYPE_SECCOMP,
 };
 
 enum bpf_attach_type {
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 255999ba9190..5a6ed8630566 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -15,6 +15,7 @@ 
  */
 #define pr_fmt(fmt) "seccomp: " fmt
 
+#include <linux/bpf.h>
 #include <linux/refcount.h>
 #include <linux/audit.h>
 #include <linux/compat.h>
@@ -2513,3 +2514,56 @@  int proc_pid_seccomp_cache(struct seq_file *m, struct pid_namespace *ns,
 	return 0;
 }
 #endif /* CONFIG_SECCOMP_CACHE_DEBUG */
+
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_BPF_SYSCALL)
+const struct bpf_prog_ops seccomp_prog_ops = {
+};
+
+static bool seccomp_is_valid_access(int off, int size, enum bpf_access_type type,
+				    const struct bpf_prog *prog,
+				    struct bpf_insn_access_aux *info)
+{
+	if (off < 0 || off >= sizeof(struct seccomp_data))
+		return false;
+
+	if (off % size != 0)
+		return false;
+
+	if (type == BPF_WRITE)
+		return false;
+
+	switch (off) {
+	case bpf_ctx_range(struct seccomp_data, nr):
+		if (size != sizeof_field(struct seccomp_data, nr))
+			return false;
+		return true;
+	case bpf_ctx_range(struct seccomp_data, arch):
+		if (size != sizeof_field(struct seccomp_data, arch))
+			return false;
+		return true;
+	case bpf_ctx_range(struct seccomp_data, instruction_pointer):
+		if (size != sizeof_field(struct seccomp_data, instruction_pointer))
+			return false;
+		return true;
+	case bpf_ctx_range(struct seccomp_data, args):
+		if (size != sizeof(__u64))
+			return false;
+		return true;
+	default:
+		return false;
+	}
+
+	return false;
+}
+
+static const struct bpf_func_proto *
+bpf_seccomp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	return NULL;
+}
+
+const struct bpf_verifier_ops seccomp_verifier_ops = {
+	.is_valid_access = seccomp_is_valid_access,
+	.get_func_proto  = bpf_seccomp_func_proto,
+};
+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_BPF_SYSCALL */
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e067be95da3c..455d733f7315 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -217,6 +217,7 @@  static const char * const prog_type_name[] = {
 	[BPF_PROG_TYPE_SK_LOOKUP]		= "sk_lookup",
 	[BPF_PROG_TYPE_SYSCALL]			= "syscall",
 	[BPF_PROG_TYPE_NETFILTER]		= "netfilter",
+	[BPF_PROG_TYPE_SECCOMP]			= "seccomp",
 };
 
 static int __base_pr(enum libbpf_print_level level, const char *format,
@@ -8991,6 +8992,7 @@  static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("struct_ops.s+",	STRUCT_OPS, 0, SEC_SLEEPABLE),
 	SEC_DEF("sk_lookup",		SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE),
 	SEC_DEF("netfilter",		NETFILTER, BPF_NETFILTER, SEC_NONE),
+	SEC_DEF("seccomp",		SECCOMP, 0, SEC_NONE),
 };
 
 int libbpf_register_prog_handler(const char *sec,
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 9c4db90b92b6..b3ef3c0747be 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -180,6 +180,7 @@  static int probe_prog_load(enum bpf_prog_type prog_type,
 	case BPF_PROG_TYPE_SK_REUSEPORT:
 	case BPF_PROG_TYPE_FLOW_DISSECTOR:
 	case BPF_PROG_TYPE_CGROUP_SYSCTL:
+	case BPF_PROG_TYPE_SECCOMP:
 		break;
 	case BPF_PROG_TYPE_NETFILTER:
 		opts.expected_attach_type = BPF_NETFILTER;