[PATCH v2 0/5] seccomp: Make seccomp filter reusable

[PATCH v2 0/5] seccomp: Make seccomp filter reusable

[Hengqi Chen]

This patchset introduces a new operation and a new flag
to split the SECCOMP_SET_MODE_FILTER process into two steps:
load filter and attach filter. Thus we can reuse the filter.

The new SECCOMP_LOAD_FILTER loads the filter and returns a fd
which can be pinned to bpffs. This extends the lifetime of the
filter and thus can be reused by different processes.
With this new operation, we can eliminate a hot path of JITing
BPF program (the filter) where we apply the same seccomp filter
to thousands of micro VMs on a bare metal instance.

The new flag SECCOMP_FILTER_FLAG_BPF_PROG_FD is used to attach
a filter which is loaded via SECCOMP_LOAD_FILTER and represented
by a seccomp bpf prog fd. The fd is either returned from
SECCOMP_LOAD_FILTER or obtained from bpffs using bpf syscall.

Change logs:
  v1 -> v2:
    - Add a flag to attach filter from fd
    - Update selftests

  RFC -> v1:
    - Addressed comments from Kees
    - Reuse filter copy/create code (patch 1)
    - Add a selftest (patch 4)

Hengqi Chen (5):
  seccomp: Refactor filter copy/create for reuse
  seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation
  seccomp: Introduce new flag SECCOMP_FILTER_FLAG_BPF_PROG_FD
  selftests/seccomp: Test seccomp filter load and attach
  selftests/bpf: Skip BPF_PROG_TYPE_SECCOMP-related tests

 include/linux/seccomp.h                       |   3 +-
 include/uapi/linux/bpf.h                      |   1 +
 include/uapi/linux/seccomp.h                  |   3 +
 kernel/seccomp.c                              | 170 +++++++++++++++---
 tools/include/uapi/linux/bpf.h                |   1 +
 .../selftests/bpf/prog_tests/libbpf_probes.c  |   3 +-
 .../selftests/bpf/prog_tests/libbpf_str.c     |   3 +
 tools/testing/selftests/seccomp/seccomp_bpf.c |  44 +++++
 8 files changed, 197 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH v2 1/5] seccomp: Refactor filter copy/create for reuse

[Hengqi Chen]

This extracts two helpers for reuse in subsequent additions.
No functional change intended, just a prep work.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 kernel/seccomp.c | 90 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 27 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 255999ba9190..faf84fc892eb 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -640,14 +640,14 @@ static inline void seccomp_sync_threads(unsigned long flags)
 }
 
 /**
- * seccomp_prepare_filter: Prepares a seccomp filter for use.
- * @fprog: BPF program to install
+ * seccomp_prepare_prog - prepares a JITed BPF filter for use.
+ * @pfp: the unattached filter that is created
+ * @fprog: the filter program
  *
- * Returns filter on success or an ERR_PTR on failure.
+ * Returns 0 on success and non-zero otherwise.
  */
-static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
+static int seccomp_prepare_prog(struct bpf_prog **pfp, struct sock_fprog *fprog)
 {
-	struct seccomp_filter *sfilter;
 	int ret;
 	const bool save_orig =
 #if defined(CONFIG_CHECKPOINT_RESTORE) || defined(SECCOMP_ARCH_NATIVE)
@@ -657,10 +657,27 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 #endif
 
 	if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
 
+	ret = bpf_prog_create_from_user(pfp, fprog, seccomp_check_filter, save_orig);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/**
+ * seccomp_prepare_filter: Prepares a seccomp filter for use.
+ * @prog: BPF program to install
+ *
+ * Returns filter on success or an ERR_PTR on failure.
+ */
+static struct seccomp_filter *seccomp_prepare_filter(struct bpf_prog *prog)
+{
+	struct seccomp_filter *sfilter;
+
 	/*
 	 * Installing a seccomp filter requires that the task has
 	 * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
@@ -677,13 +694,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&sfilter->notify_lock);
-	ret = bpf_prog_create_from_user(&sfilter->prog, fprog,
-					seccomp_check_filter, save_orig);
-	if (ret < 0) {
-		kfree(sfilter);
-		return ERR_PTR(ret);
-	}
-
+	sfilter->prog = prog;
 	refcount_set(&sfilter->refs, 1);
 	refcount_set(&sfilter->users, 1);
 	init_waitqueue_head(&sfilter->wqh);
@@ -692,31 +703,56 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 }
 
 /**
- * seccomp_prepare_user_filter - prepares a user-supplied sock_fprog
+ * seccomp_copy_user_filter - copies a user-supplied sock_fprog
  * @user_filter: pointer to the user data containing a sock_fprog.
+ * @fprog: pointer to store the copied BPF program
  *
  * Returns 0 on success and non-zero otherwise.
  */
-static struct seccomp_filter *
-seccomp_prepare_user_filter(const char __user *user_filter)
+static int seccomp_copy_user_filter(const char __user *user_filter, struct sock_fprog *fprog)
 {
-	struct sock_fprog fprog;
-	struct seccomp_filter *filter = ERR_PTR(-EFAULT);
-
 #ifdef CONFIG_COMPAT
 	if (in_compat_syscall()) {
 		struct compat_sock_fprog fprog32;
 		if (copy_from_user(&fprog32, user_filter, sizeof(fprog32)))
-			goto out;
-		fprog.len = fprog32.len;
-		fprog.filter = compat_ptr(fprog32.filter);
+			return -EFAULT;
+		fprog->len = fprog32.len;
+		fprog->filter = compat_ptr(fprog32.filter);
 	} else /* falls through to the if below. */
 #endif
-	if (copy_from_user(&fprog, user_filter, sizeof(fprog)))
-		goto out;
-	filter = seccomp_prepare_filter(&fprog);
-out:
-	return filter;
+	if (copy_from_user(fprog, user_filter, sizeof(*fprog)))
+		return -EFAULT;
+
+	return 0;
+}
+
+/**
+ * seccomp_prepare_user_filter - prepares a user-supplied sock_fprog
+ * @user_filter: pointer to the user data containing a sock_fprog.
+ *
+ * Returns filter on success or an ERR_PTR on failure.
+ */
+static struct seccomp_filter *
+seccomp_prepare_user_filter(const char __user *user_filter)
+{
+	struct seccomp_filter *sfilter;
+	struct sock_fprog fprog;
+	struct bpf_prog *prog;
+	int ret;
+
+	ret = seccomp_copy_user_filter(user_filter, &fprog);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = seccomp_prepare_prog(&prog, &fprog);
+	if (ret)
+		return ERR_PTR(ret);
+
+	sfilter = seccomp_prepare_filter(prog);
+	if (IS_ERR(sfilter))
+		bpf_prog_destroy(prog);
+
+	return sfilter;
 }
 
 #ifdef SECCOMP_ARCH_NATIVE
-- 
2.34.1

[PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation

[Hengqi Chen]

This patch adds a new operation named SECCOMP_LOAD_FILTER.
It accepts a sock_fprog the same as SECCOMP_SET_MODE_FILTER
but only performs the loading process. If succeed, return a
new fd associated with the JITed BPF program (the filter).
The filter can then be pinned to bpffs using the returned
fd and reused for different processes. To distinguish the
filter from other BPF progs, BPF_PROG_TYPE_SECCOMP is added.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 include/uapi/linux/bpf.h       |  1 +
 include/uapi/linux/seccomp.h   |  1 +
 kernel/seccomp.c               | 43 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h |  1 +
 4 files changed, 46 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 7ba61b75bc0e..61c80ffb1724 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/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index dbfc9b37fcae..ee2c83697810 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -16,6 +16,7 @@
 #define SECCOMP_SET_MODE_FILTER		1
 #define SECCOMP_GET_ACTION_AVAIL	2
 #define SECCOMP_GET_NOTIF_SIZES		3
+#define SECCOMP_LOAD_FILTER		4
 
 /* Valid flags for SECCOMP_SET_MODE_FILTER */
 #define SECCOMP_FILTER_FLAG_TSYNC		(1UL << 0)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index faf84fc892eb..c9f6a19f7a4e 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -17,6 +17,7 @@
 
 #include <linux/refcount.h>
 #include <linux/audit.h>
+#include <linux/bpf.h>
 #include <linux/compat.h>
 #include <linux/coredump.h>
 #include <linux/kmemleak.h>
@@ -25,6 +26,7 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 #include <linux/seccomp.h>
+#include <linux/security.h>
 #include <linux/slab.h>
 #include <linux/syscalls.h>
 #include <linux/sysctl.h>
@@ -2032,12 +2034,48 @@ static long seccomp_set_mode_filter(unsigned int flags,
 	seccomp_filter_free(prepared);
 	return ret;
 }
+
+static long seccomp_load_filter(const char __user *filter)
+{
+	struct sock_fprog fprog;
+	struct bpf_prog *prog;
+	int ret;
+
+	ret = seccomp_copy_user_filter(filter, &fprog);
+	if (ret)
+		return ret;
+
+	ret = seccomp_prepare_prog(&prog, &fprog);
+	if (ret)
+		return ret;
+
+	ret = security_bpf_prog_alloc(prog->aux);
+	if (ret) {
+		bpf_prog_free(prog);
+		return ret;
+	}
+
+	prog->aux->user = get_current_user();
+	atomic64_set(&prog->aux->refcnt, 1);
+	prog->type = BPF_PROG_TYPE_SECCOMP;
+
+	ret = bpf_prog_new_fd(prog);
+	if (ret < 0)
+		bpf_prog_put(prog);
+
+	return ret;
+}
 #else
 static inline long seccomp_set_mode_filter(unsigned int flags,
 					   const char __user *filter)
 {
 	return -EINVAL;
 }
+
+static inline long seccomp_load_filter(const char __user *filter)
+{
+	return -EINVAL;
+}
 #endif
 
 static long seccomp_get_action_avail(const char __user *uaction)
@@ -2099,6 +2137,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
 			return -EINVAL;
 
 		return seccomp_get_notif_sizes(uargs);
+	case SECCOMP_LOAD_FILTER:
+		if (flags != 0)
+			return -EINVAL;
+
+		return seccomp_load_filter(uargs);
 	default:
 		return -EINVAL;
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 7ba61b75bc0e..61c80ffb1724 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/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 {
-- 
2.34.1

[PATCH v2 3/5] seccomp: Introduce new flag SECCOMP_FILTER_FLAG_BPF_PROG_FD

[Hengqi Chen]

Add a new flag SECCOMP_FILTER_FLAG_BPF_PROG_FD for
SECCOMP_SET_MODE_FILTER. This indicates the seccomp filter
is a seccomp bpf prog fd, not a sock_fprog. This allows
us to attach the seccomp filter that is previously loaded
via SECCOMP_LOAD_FILTER.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 include/linux/seccomp.h      |  3 ++-
 include/uapi/linux/seccomp.h |  2 ++
 kernel/seccomp.c             | 37 ++++++++++++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index 175079552f68..7caa53b629d9 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -9,7 +9,8 @@
 					 SECCOMP_FILTER_FLAG_SPEC_ALLOW | \
 					 SECCOMP_FILTER_FLAG_NEW_LISTENER | \
 					 SECCOMP_FILTER_FLAG_TSYNC_ESRCH | \
-					 SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV)
+					 SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV | \
+					 SECCOMP_FILTER_FLAG_BPF_PROG_FD)
 
 /* sizeof() the first published struct seccomp_notif_addfd */
 #define SECCOMP_NOTIFY_ADDFD_SIZE_VER0 24
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index ee2c83697810..d6b243d1b4d5 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -26,6 +26,8 @@
 #define SECCOMP_FILTER_FLAG_TSYNC_ESRCH		(1UL << 4)
 /* Received notifications wait in killable state (only respond to fatal signals) */
 #define SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV	(1UL << 5)
+/* Indicates that the filter is in form of bpf prog fd */
+#define SECCOMP_FILTER_FLAG_BPF_PROG_FD		(1UL << 6)
 
 /*
  * All BPF programs must return a 32-bit value.
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index c9f6a19f7a4e..3a977e5932a4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -525,7 +525,10 @@ static inline pid_t seccomp_can_sync_threads(void)
 static inline void seccomp_filter_free(struct seccomp_filter *filter)
 {
 	if (filter) {
-		bpf_prog_destroy(filter->prog);
+		if (filter->prog->type == BPF_PROG_TYPE_SECCOMP)
+			bpf_prog_put(filter->prog);
+		else
+			bpf_prog_destroy(filter->prog);
 		kfree(filter);
 	}
 }
@@ -757,6 +760,33 @@ seccomp_prepare_user_filter(const char __user *user_filter)
 	return sfilter;
 }
 
+/**
+ * seccomp_prepare_filter_from_fd - prepares filter from a user-supplied fd
+ * @ufd: pointer to fd that refers to a seccomp bpf prog.
+ *
+ * Returns filter on success or an ERR_PTR on failure.
+ */
+static struct seccomp_filter *
+seccomp_prepare_filter_from_fd(const char __user *ufd)
+{
+	struct seccomp_filter *sfilter;
+	struct bpf_prog *prog;
+	int fd;
+
+	if (copy_from_user(&fd, ufd, sizeof(fd)))
+		return ERR_PTR(-EFAULT);
+
+	prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_SECCOMP);
+	if (IS_ERR(prog))
+		return ERR_PTR(-EBADF);
+
+	sfilter = seccomp_prepare_filter(prog);
+	if (IS_ERR(sfilter))
+		bpf_prog_put(prog);
+
+	return sfilter;
+}
+
 #ifdef SECCOMP_ARCH_NATIVE
 /**
  * seccomp_is_const_allow - check if filter is constant allow with given data
@@ -1970,7 +2000,10 @@ static long seccomp_set_mode_filter(unsigned int flags,
 		return -EINVAL;
 
 	/* Prepare the new filter before holding any locks. */
-	prepared = seccomp_prepare_user_filter(filter);
+	if (flags & SECCOMP_FILTER_FLAG_BPF_PROG_FD)
+		prepared = seccomp_prepare_filter_from_fd(filter);
+	else
+		prepared = seccomp_prepare_user_filter(filter);
 	if (IS_ERR(prepared))
 		return PTR_ERR(prepared);
 
-- 
2.34.1

[PATCH v2 4/5] selftests/seccomp: Test seccomp filter load and attach

[Hengqi Chen]

Add testcases to exercise the newly added seccomp filter
load and attach functionalities.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 38f651469968..86600d40d01f 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -4735,6 +4735,50 @@ TEST(user_notification_wait_killable_fatal)
 	EXPECT_EQ(SIGTERM, WTERMSIG(status));
 }
 
+TEST(seccomp_filter_load_and_attach)
+{
+	struct sock_filter filter[] = {
+		BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+	};
+	struct sock_fprog prog = {
+		.len = (unsigned short)ARRAY_SIZE(filter),
+		.filter = filter,
+	};
+	int fd, ret, flags;
+
+	flags = 0;
+	fd = seccomp(SECCOMP_LOAD_FILTER, flags, &prog);
+	ASSERT_GT(fd, -1);
+
+	flags = SECCOMP_FILTER_FLAG_BPF_PROG_FD;
+	ret = seccomp(SECCOMP_SET_MODE_FILTER, flags, &fd);
+	ASSERT_EQ(ret, 0);
+
+	close(fd);
+}
+
+TEST(seccomp_attach_fd_failed)
+{
+	int fd, ret, flags;
+
+	fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_GT(fd, -1);
+
+	/* copy a sock_fprog from a fd */
+	flags = 0;
+	ret = seccomp(SECCOMP_SET_MODE_FILTER, flags, &fd);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EFAULT);
+
+	/* pass a non seccomp bpf prog fd */
+	flags = SECCOMP_FILTER_FLAG_BPF_PROG_FD;
+	ret = seccomp(SECCOMP_SET_MODE_FILTER, flags, &fd);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EBADF);
+
+	close(fd);
+}
+
 /*
  * TODO:
  * - expand NNP testing
-- 
2.34.1

[PATCH v2 5/5] selftests/bpf: Skip BPF_PROG_TYPE_SECCOMP-related tests

[Hengqi Chen]

We only allow BPF_PROG_TYPE_SECCOMP progs to be loaded via
seccomp syscall. Skip related test on BPF side.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/libbpf_probes.c | 3 ++-
 tools/testing/selftests/bpf/prog_tests/libbpf_str.c    | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_probes.c b/tools/testing/selftests/bpf/prog_tests/libbpf_probes.c
index 9f766ddd946a..134ae042c4da 100644
--- a/tools/testing/selftests/bpf/prog_tests/libbpf_probes.c
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_probes.c
@@ -28,7 +28,8 @@ void test_libbpf_probe_prog_types(void)
 		enum bpf_prog_type prog_type = (enum bpf_prog_type)e->val;
 		int res;
 
-		if (prog_type == BPF_PROG_TYPE_UNSPEC)
+		if (prog_type == BPF_PROG_TYPE_UNSPEC ||
+		    prog_type == BPF_PROG_TYPE_SECCOMP)
 			continue;
 
 		if (!test__start_subtest(prog_type_name))
diff --git a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
index c440ea3311ed..35365500c326 100644
--- a/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
+++ b/tools/testing/selftests/bpf/prog_tests/libbpf_str.c
@@ -186,6 +186,9 @@ static void test_libbpf_bpf_prog_type_str(void)
 		const char *prog_type_str;
 		char buf[256];
 
+		if (prog_type == BPF_PROG_TYPE_SECCOMP)
+			continue;
+
 		prog_type_name = btf__str_by_offset(btf, e->name_off);
 		prog_type_str = libbpf_bpf_prog_type_str(prog_type);
 		ASSERT_OK_PTR(prog_type_str, prog_type_name);
-- 
2.34.1

Re: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation

[Daniel Borkmann]

On 10/16/23 1:29 AM, Hengqi Chen wrote:
> This patch adds a new operation named SECCOMP_LOAD_FILTER.
> It accepts a sock_fprog the same as SECCOMP_SET_MODE_FILTER
> but only performs the loading process. If succeed, return a
> new fd associated with the JITed BPF program (the filter).
> The filter can then be pinned to bpffs using the returned
> fd and reused for different processes. To distinguish the
> filter from other BPF progs, BPF_PROG_TYPE_SECCOMP is added.
> 
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>   include/uapi/linux/bpf.h       |  1 +
>   include/uapi/linux/seccomp.h   |  1 +
>   kernel/seccomp.c               | 43 ++++++++++++++++++++++++++++++++++
>   tools/include/uapi/linux/bpf.h |  1 +
>   4 files changed, 46 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 7ba61b75bc0e..61c80ffb1724 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,

Please don't extend UAPI surface if this is not reachable/usable from user
space anyway.

>   enum bpf_attach_type {
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index dbfc9b37fcae..ee2c83697810 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -16,6 +16,7 @@
>   #define SECCOMP_SET_MODE_FILTER		1
>   #define SECCOMP_GET_ACTION_AVAIL	2
>   #define SECCOMP_GET_NOTIF_SIZES		3
> +#define SECCOMP_LOAD_FILTER		4
>   
>   /* Valid flags for SECCOMP_SET_MODE_FILTER */
>   #define SECCOMP_FILTER_FLAG_TSYNC		(1UL << 0)
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index faf84fc892eb..c9f6a19f7a4e 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -17,6 +17,7 @@
>   
>   #include <linux/refcount.h>
>   #include <linux/audit.h>
> +#include <linux/bpf.h>
>   #include <linux/compat.h>
>   #include <linux/coredump.h>
>   #include <linux/kmemleak.h>
> @@ -25,6 +26,7 @@
>   #include <linux/sched.h>
>   #include <linux/sched/task_stack.h>
>   #include <linux/seccomp.h>
> +#include <linux/security.h>
>   #include <linux/slab.h>
>   #include <linux/syscalls.h>
>   #include <linux/sysctl.h>
> @@ -2032,12 +2034,48 @@ static long seccomp_set_mode_filter(unsigned int flags,
>   	seccomp_filter_free(prepared);
>   	return ret;
>   }
> +
> +static long seccomp_load_filter(const char __user *filter)
> +{
> +	struct sock_fprog fprog;
> +	struct bpf_prog *prog;
> +	int ret;
> +
> +	ret = seccomp_copy_user_filter(filter, &fprog);
> +	if (ret)
> +		return ret;
> +
> +	ret = seccomp_prepare_prog(&prog, &fprog);
> +	if (ret)
> +		return ret;
> +
> +	ret = security_bpf_prog_alloc(prog->aux);
> +	if (ret) {
> +		bpf_prog_free(prog);
> +		return ret;
> +	}
> +
> +	prog->aux->user = get_current_user();
> +	atomic64_set(&prog->aux->refcnt, 1);
> +	prog->type = BPF_PROG_TYPE_SECCOMP;
> +
> +	ret = bpf_prog_new_fd(prog);
> +	if (ret < 0)
> +		bpf_prog_put(prog);

My bigger concern here is that bpf_prog_new_fd() is only used by eBPF (not cBPF).

Then you get an 'eBPF'-like fd back to user space which you can pass to various
other bpf(2) commands like BPF_OBJ_GET_INFO_BY_FD etc which all have the assumption
that this is a proper looking eBPF prog fd.

There may be breakage/undefined behavior in subtle ways.

I would suggest two potential alternatives :

1) Build a seccomp-specific fd via anon_inode_getfd() so that BPF side does not
    confuse it with bpf_prog_fops and therefore does not recognize it in bpf(2)
    as a prog fd.

2) Extend seccomp where proper eBPF could be supported.

If option 2) is not realistic (where you would get this out of the box), then I
think 1) could be however.

> +	return ret;
> +}
>   #else
>   static inline long seccomp_set_mode_filter(unsigned int flags,
>   					   const char __user *filter)
>   {
>   	return -EINVAL;
>   }
> +
> +static inline long seccomp_load_filter(const char __user *filter)
> +{
> +	return -EINVAL;
> +}
>   #endif
>   
>   static long seccomp_get_action_avail(const char __user *uaction)
> @@ -2099,6 +2137,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
>   			return -EINVAL;
>   
>   		return seccomp_get_notif_sizes(uargs);
> +	case SECCOMP_LOAD_FILTER:
> +		if (flags != 0)
> +			return -EINVAL;
> +
> +		return seccomp_load_filter(uargs);
>   	default:
>   		return -EINVAL;
>   	}
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 7ba61b75bc0e..61c80ffb1724 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/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 {
> 

Re: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation

[kernel test robot]

Hi Hengqi,

kernel test robot noticed the following build errors:

[auto build test ERROR on kees/for-next/seccomp]
[also build test ERROR on bpf-next/master bpf/master linus/master v6.6-rc6 next-20231020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Hengqi-Chen/seccomp-Refactor-filter-copy-create-for-reuse/20231017-134654
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/seccomp
patch link:    https://lore.kernel.org/r/20231015232953.84836-3-hengqi.chen%40gmail.com
patch subject: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation
config: sh-shx3_defconfig (https://download.01.org/0day-ci/archive/20231023/202310230704.Uif0R7cz-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231023/202310230704.Uif0R7cz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310230704.Uif0R7cz-lkp@intel.com/

All errors (new ones prefixed by >>):

   kernel/seccomp.c: In function 'seccomp_load_filter':
>> kernel/seccomp.c:2052:15: error: implicit declaration of function 'security_bpf_prog_alloc'; did you mean 'security_msg_msg_alloc'? [-Werror=implicit-function-declaration]
    2052 |         ret = security_bpf_prog_alloc(prog->aux);
         |               ^~~~~~~~~~~~~~~~~~~~~~~
         |               security_msg_msg_alloc
>> kernel/seccomp.c:2062:15: error: implicit declaration of function 'bpf_prog_new_fd'; did you mean 'bpf_prog_get_ok'? [-Werror=implicit-function-declaration]
    2062 |         ret = bpf_prog_new_fd(prog);
         |               ^~~~~~~~~~~~~~~
         |               bpf_prog_get_ok
   cc1: some warnings being treated as errors


vim +2052 kernel/seccomp.c

  2037	
  2038	static long seccomp_load_filter(const char __user *filter)
  2039	{
  2040		struct sock_fprog fprog;
  2041		struct bpf_prog *prog;
  2042		int ret;
  2043	
  2044		ret = seccomp_copy_user_filter(filter, &fprog);
  2045		if (ret)
  2046			return ret;
  2047	
  2048		ret = seccomp_prepare_prog(&prog, &fprog);
  2049		if (ret)
  2050			return ret;
  2051	
> 2052		ret = security_bpf_prog_alloc(prog->aux);
  2053		if (ret) {
  2054			bpf_prog_free(prog);
  2055			return ret;
  2056		}
  2057	
  2058		prog->aux->user = get_current_user();
  2059		atomic64_set(&prog->aux->refcnt, 1);
  2060		prog->type = BPF_PROG_TYPE_SECCOMP;
  2061	
> 2062		ret = bpf_prog_new_fd(prog);
  2063		if (ret < 0)
  2064			bpf_prog_put(prog);
  2065	
  2066		return ret;
  2067	}
  2068	#else
  2069	static inline long seccomp_set_mode_filter(unsigned int flags,
  2070						   const char __user *filter)
  2071	{
  2072		return -EINVAL;
  2073	}
  2074	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/5] seccomp, bpf: Introduce SECCOMP_LOAD_FILTER operation

[Hengqi Chen]

On Mon, Oct 16, 2023 at 8:44 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 10/16/23 1:29 AM, Hengqi Chen wrote:
> > This patch adds a new operation named SECCOMP_LOAD_FILTER.
> > It accepts a sock_fprog the same as SECCOMP_SET_MODE_FILTER
> > but only performs the loading process. If succeed, return a
> > new fd associated with the JITed BPF program (the filter).
> > The filter can then be pinned to bpffs using the returned
> > fd and reused for different processes. To distinguish the
> > filter from other BPF progs, BPF_PROG_TYPE_SECCOMP is added.
> >
> > Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> > ---
> >   include/uapi/linux/bpf.h       |  1 +
> >   include/uapi/linux/seccomp.h   |  1 +
> >   kernel/seccomp.c               | 43 ++++++++++++++++++++++++++++++++++
> >   tools/include/uapi/linux/bpf.h |  1 +
> >   4 files changed, 46 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 7ba61b75bc0e..61c80ffb1724 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,
>
> Please don't extend UAPI surface if this is not reachable/usable from user
> space anyway.
>
> >   enum bpf_attach_type {
> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index dbfc9b37fcae..ee2c83697810 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -16,6 +16,7 @@
> >   #define SECCOMP_SET_MODE_FILTER             1
> >   #define SECCOMP_GET_ACTION_AVAIL    2
> >   #define SECCOMP_GET_NOTIF_SIZES             3
> > +#define SECCOMP_LOAD_FILTER          4
> >
> >   /* Valid flags for SECCOMP_SET_MODE_FILTER */
> >   #define SECCOMP_FILTER_FLAG_TSYNC           (1UL << 0)
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index faf84fc892eb..c9f6a19f7a4e 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -17,6 +17,7 @@
> >
> >   #include <linux/refcount.h>
> >   #include <linux/audit.h>
> > +#include <linux/bpf.h>
> >   #include <linux/compat.h>
> >   #include <linux/coredump.h>
> >   #include <linux/kmemleak.h>
> > @@ -25,6 +26,7 @@
> >   #include <linux/sched.h>
> >   #include <linux/sched/task_stack.h>
> >   #include <linux/seccomp.h>
> > +#include <linux/security.h>
> >   #include <linux/slab.h>
> >   #include <linux/syscalls.h>
> >   #include <linux/sysctl.h>
> > @@ -2032,12 +2034,48 @@ static long seccomp_set_mode_filter(unsigned int flags,
> >       seccomp_filter_free(prepared);
> >       return ret;
> >   }
> > +
> > +static long seccomp_load_filter(const char __user *filter)
> > +{
> > +     struct sock_fprog fprog;
> > +     struct bpf_prog *prog;
> > +     int ret;
> > +
> > +     ret = seccomp_copy_user_filter(filter, &fprog);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = seccomp_prepare_prog(&prog, &fprog);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = security_bpf_prog_alloc(prog->aux);
> > +     if (ret) {
> > +             bpf_prog_free(prog);
> > +             return ret;
> > +     }
> > +
> > +     prog->aux->user = get_current_user();
> > +     atomic64_set(&prog->aux->refcnt, 1);
> > +     prog->type = BPF_PROG_TYPE_SECCOMP;
> > +
> > +     ret = bpf_prog_new_fd(prog);
> > +     if (ret < 0)
> > +             bpf_prog_put(prog);
>
> My bigger concern here is that bpf_prog_new_fd() is only used by eBPF (not cBPF).
>
> Then you get an 'eBPF'-like fd back to user space which you can pass to various
> other bpf(2) commands like BPF_OBJ_GET_INFO_BY_FD etc which all have the assumption
> that this is a proper looking eBPF prog fd.
>
> There may be breakage/undefined behavior in subtle ways.
>
> I would suggest two potential alternatives :
>
> 1) Build a seccomp-specific fd via anon_inode_getfd() so that BPF side does not
>     confuse it with bpf_prog_fops and therefore does not recognize it in bpf(2)
>     as a prog fd.
>
> 2) Extend seccomp where proper eBPF could be supported.
>
> If option 2) is not realistic (where you would get this out of the box), then I
> think 1) could be however.
>

The intention is to use bpffs, so we need a bpf prog fd.
I prefer option 2, though it requires a bit of work.
That way, we could also write seccomp filter in eBPF language.

Kees, could you share your opinions ? If you have no objection,
I will continue this work.

> > +     return ret;
> > +}
> >   #else
> >   static inline long seccomp_set_mode_filter(unsigned int flags,
> >                                          const char __user *filter)
> >   {
> >       return -EINVAL;
> >   }
> > +
> > +static inline long seccomp_load_filter(const char __user *filter)
> > +{
> > +     return -EINVAL;
> > +}
> >   #endif
> >
> >   static long seccomp_get_action_avail(const char __user *uaction)
> > @@ -2099,6 +2137,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
> >                       return -EINVAL;
> >
> >               return seccomp_get_notif_sizes(uargs);
> > +     case SECCOMP_LOAD_FILTER:
> > +             if (flags != 0)
> > +                     return -EINVAL;
> > +
> > +             return seccomp_load_filter(uargs);
> >       default:
> >               return -EINVAL;
> >       }
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index 7ba61b75bc0e..61c80ffb1724 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/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 {
> >
>