[RFC PATCH 0/2] seccomp: Split set filter into two steps

[RFC PATCH 0/2] seccomp: Split set filter into two steps

[Hengqi Chen]

This patchset introduces two new operations which essentially
splits the SECCOMP_SET_MODE_FILTER process into two steps:
SECCOMP_LOAD_FILTER and SECCOMP_ATTACH_FILTER.

The 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 SECCOMP_ATTACH_FILTER is used to attach a loaded filter.
The filter is represented by a fd which is either returned
from SECCOMP_LOAD_FILTER or obtained from bpffs using bpf syscall.

Hengqi Chen (2):
  seccomp: Introduce SECCOMP_LOAD_FILTER operation
  seccomp: Introduce SECCOMP_ATTACH_FILTER operation

 include/uapi/linux/seccomp.h |   2 +
 kernel/seccomp.c             | 138 ++++++++++++++++++++++++++++++++++-
 2 files changed, 136 insertions(+), 4 deletions(-)

-- 
2.34.1

[RFC PATCH 1/2] seccomp: Introduce SECCOMP_LOAD_FILTER operation

[Hengqi Chen]

This patch adds a new operation named SECCOMP_LOAD_FILTER.
It accepts the same arguments 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.

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

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 255999ba9190..7aff22f56a91 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1996,12 +1996,71 @@ 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 = -EFAULT;
+	const bool save_orig =
+#if defined(CONFIG_CHECKPOINT_RESTORE) || defined(SECCOMP_ARCH_NATIVE)
+		true;
+#else
+		false;
+#endif
+
+#ifdef CONFIG_COMPAT
+	if (in_compat_syscall()) {
+		struct compat_sock_fprog fprog32;
+		if (copy_from_user(&fprog32, filter, sizeof(fprog32)))
+			goto out;
+		fprog.len = fprog32.len;
+		fprog.filter = compat_ptr(fprog32.filter);
+	} else /* falls through to the if below. */
+#endif
+	if (copy_from_user(&fprog, filter, sizeof(fprog)))
+		goto out;
+
+	ret = -EINVAL;
+	if (fprog.len == 0 || fprog.len > BPF_MAXINSNS)
+		goto out;
+
+	BUG_ON(INT_MAX / fprog.len < sizeof(struct sock_filter));
+
+	ret = bpf_prog_create_from_user(&prog, &fprog, seccomp_check_filter, save_orig);
+	if (ret < 0)
+		goto out;
+
+	ret = security_bpf_prog_alloc(prog->aux);
+	if (ret) {
+		ret = -EINVAL;
+		goto prog_free;
+	}
+
+	prog->aux->user = get_current_user();
+	atomic64_set(&prog->aux->refcnt, 1);
+
+	ret = bpf_prog_new_fd(prog);
+	if (ret < 0)
+		bpf_prog_put(prog);
+	return ret;
+
+prog_free:
+	bpf_prog_free(prog);
+out:
+	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)
@@ -2063,6 +2122,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;
 	}
-- 
2.34.1

[RFC PATCH 2/2] seccomp: Introduce SECCOMP_ATTACH_FILTER operation

[Hengqi Chen]

The SECCOMP_ATTACH_FILTER operation is used to attach
a loaded filter to the current process. The loaded filter
is represented by a fd which is either returned by the
SECCOMP_LOAD_FILTER operation or obtained from bpffs using
bpf syscall.

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

diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index ee2c83697810..fbe30262fdfc 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -17,6 +17,7 @@
 #define SECCOMP_GET_ACTION_AVAIL	2
 #define SECCOMP_GET_NOTIF_SIZES		3
 #define SECCOMP_LOAD_FILTER		4
+#define SECCOMP_ATTACH_FILTER		5
 
 /* 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 7aff22f56a91..adfafee4c3da 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -862,7 +862,7 @@ static void seccomp_cache_prepare(struct seccomp_filter *sfilter)
 #endif /* SECCOMP_ARCH_NATIVE */
 
 /**
- * seccomp_attach_filter: validate and attach filter
+ * seccomp_do_attach_filter: validate and attach filter
  * @flags:  flags to change filter behavior
  * @filter: seccomp filter to add to the current process
  *
@@ -873,8 +873,8 @@ static void seccomp_cache_prepare(struct seccomp_filter *sfilter)
  *     seccomp mode or did not have an ancestral seccomp filter
  *   - in NEW_LISTENER mode: the fd of the new listener
  */
-static long seccomp_attach_filter(unsigned int flags,
-				  struct seccomp_filter *filter)
+static long seccomp_do_attach_filter(unsigned int flags,
+				     struct seccomp_filter *filter)
 {
 	unsigned long total_insns;
 	struct seccomp_filter *walker;
@@ -1969,7 +1969,7 @@ static long seccomp_set_mode_filter(unsigned int flags,
 		goto out;
 	}
 
-	ret = seccomp_attach_filter(flags, prepared);
+	ret = seccomp_do_attach_filter(flags, prepared);
 	if (ret)
 		goto out;
 	/* Do not free the successfully attached filter. */
@@ -2050,6 +2050,62 @@ static long seccomp_load_filter(const char __user *filter)
 out:
 	return ret;
 }
+
+static long seccomp_attach_filter(const char __user *ufd)
+{
+	const unsigned long seccomp_mode = SECCOMP_MODE_FILTER;
+	struct seccomp_filter *sfilter;
+	struct bpf_prog *prog;
+	struct file *filp;
+	int flags = 0;
+	int fd, ret;
+
+	if (copy_from_user(&fd, ufd, sizeof(fd)))
+		return -EFAULT;
+
+	filp = fget(fd);
+	if (!filp)
+		return -EBADF;
+
+	if (filp->f_op != &bpf_prog_fops) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	prog = filp->private_data;
+
+	sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
+	if (!sfilter) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	sfilter->prog = prog;
+	refcount_set(&sfilter->refs, 1);
+	refcount_set(&sfilter->users, 1);
+	mutex_init(&sfilter->notify_lock);
+	init_waitqueue_head(&sfilter->wqh);
+
+	spin_lock_irq(&current->sighand->siglock);
+
+	ret = -EINVAL;
+	if (!seccomp_may_assign_mode(seccomp_mode))
+		goto out_unlock;
+
+	ret = seccomp_do_attach_filter(flags, sfilter);
+	if (ret)
+		goto out_unlock;
+
+	sfilter = NULL;
+	seccomp_assign_mode(current, seccomp_mode, flags);
+
+out_unlock:
+	spin_unlock_irq(&current->sighand->siglock);
+	seccomp_filter_free(sfilter);
+out:
+	fput(filp);
+	return ret;
+}
 #else
 static inline long seccomp_set_mode_filter(unsigned int flags,
 					   const char __user *filter)
@@ -2061,6 +2117,11 @@ static inline long seccomp_load_filter(const char __user *filter)
 {
 	return -EINVAL;
 }
+
+static inline long seccomp_attach_filter(const char __user *ufd)
+{
+	return -EINVAL;
+}
 #endif
 
 static long seccomp_get_action_avail(const char __user *uaction)
@@ -2127,6 +2188,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
 			return -EINVAL;
 
 		return seccomp_load_filter(uargs);
+	case SECCOMP_ATTACH_FILTER:
+		if (flags != 0)
+			return -EINVAL;
+
+		return seccomp_attach_filter(uargs);
 	default:
 		return -EINVAL;
 	}
-- 
2.34.1

Re: [RFC PATCH 1/2] seccomp: Introduce SECCOMP_LOAD_FILTER operation

[kernel test robot]

Hi Hengqi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on kees/for-next/seccomp]
[also build test ERROR on kees/for-next/pstore kees/for-next/kspp linus/master v6.6-rc4 next-20231003]
[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-Introduce-SECCOMP_LOAD_FILTER-operation/20231003-164556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/seccomp
patch link:    https://lore.kernel.org/r/20231003083836.100706-2-hengqi.chen%40gmail.com
patch subject: [RFC PATCH 1/2] seccomp: Introduce SECCOMP_LOAD_FILTER operation
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20231004/202310040027.1dNPiBPq-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310040027.1dNPiBPq-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/202310040027.1dNPiBPq-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> kernel/seccomp.c:2034:8: error: implicit declaration of function 'security_bpf_prog_alloc' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ret = security_bpf_prog_alloc(prog->aux);
                 ^
   kernel/seccomp.c:2034:8: note: did you mean 'security_msg_msg_alloc'?
   include/linux/security.h:1245:19: note: 'security_msg_msg_alloc' declared here
   static inline int security_msg_msg_alloc(struct msg_msg *msg)
                     ^
>> kernel/seccomp.c:2043:8: error: implicit declaration of function 'bpf_prog_new_fd' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           ret = bpf_prog_new_fd(prog);
                 ^
   12 warnings and 2 errors generated.


vim +/security_bpf_prog_alloc +2034 kernel/seccomp.c

  2011	
  2012	#ifdef CONFIG_COMPAT
  2013		if (in_compat_syscall()) {
  2014			struct compat_sock_fprog fprog32;
  2015			if (copy_from_user(&fprog32, filter, sizeof(fprog32)))
  2016				goto out;
  2017			fprog.len = fprog32.len;
  2018			fprog.filter = compat_ptr(fprog32.filter);
  2019		} else /* falls through to the if below. */
  2020	#endif
  2021		if (copy_from_user(&fprog, filter, sizeof(fprog)))
  2022			goto out;
  2023	
  2024		ret = -EINVAL;
  2025		if (fprog.len == 0 || fprog.len > BPF_MAXINSNS)
  2026			goto out;
  2027	
  2028		BUG_ON(INT_MAX / fprog.len < sizeof(struct sock_filter));
  2029	
  2030		ret = bpf_prog_create_from_user(&prog, &fprog, seccomp_check_filter, save_orig);
  2031		if (ret < 0)
  2032			goto out;
  2033	
> 2034		ret = security_bpf_prog_alloc(prog->aux);
  2035		if (ret) {
  2036			ret = -EINVAL;
  2037			goto prog_free;
  2038		}
  2039	
  2040		prog->aux->user = get_current_user();
  2041		atomic64_set(&prog->aux->refcnt, 1);
  2042	
> 2043		ret = bpf_prog_new_fd(prog);
  2044		if (ret < 0)
  2045			bpf_prog_put(prog);
  2046		return ret;
  2047	
  2048	prog_free:
  2049		bpf_prog_free(prog);
  2050	out:
  2051		return ret;
  2052	}
  2053	#else
  2054	static inline long seccomp_set_mode_filter(unsigned int flags,
  2055						   const char __user *filter)
  2056	{
  2057		return -EINVAL;
  2058	}
  2059	

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

Re: [RFC PATCH 1/2] seccomp: Introduce SECCOMP_LOAD_FILTER operation

[kernel test robot]

Hi Hengqi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on kees/for-next/seccomp]
[also build test ERROR on kees/for-next/pstore kees/for-next/kspp linus/master v6.6-rc4 next-20231003]
[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-Introduce-SECCOMP_LOAD_FILTER-operation/20231003-164556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/seccomp
patch link:    https://lore.kernel.org/r/20231003083836.100706-2-hengqi.chen%40gmail.com
patch subject: [RFC PATCH 1/2] seccomp: Introduce SECCOMP_LOAD_FILTER operation
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20231004/202310040012.kV4xilAy-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310040012.kV4xilAy-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/202310040012.kV4xilAy-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> kernel/seccomp.c:2034:8: error: call to undeclared function 'security_bpf_prog_alloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2034 |         ret = security_bpf_prog_alloc(prog->aux);
         |               ^
   kernel/seccomp.c:2034:8: note: did you mean 'security_msg_msg_alloc'?
   include/linux/security.h:1245:19: note: 'security_msg_msg_alloc' declared here
    1245 | static inline int security_msg_msg_alloc(struct msg_msg *msg)
         |                   ^
>> kernel/seccomp.c:2043:8: error: call to undeclared function 'bpf_prog_new_fd'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2043 |         ret = bpf_prog_new_fd(prog);
         |               ^
   12 warnings and 2 errors generated.


vim +/security_bpf_prog_alloc +2034 kernel/seccomp.c

  2011	
  2012	#ifdef CONFIG_COMPAT
  2013		if (in_compat_syscall()) {
  2014			struct compat_sock_fprog fprog32;
  2015			if (copy_from_user(&fprog32, filter, sizeof(fprog32)))
  2016				goto out;
  2017			fprog.len = fprog32.len;
  2018			fprog.filter = compat_ptr(fprog32.filter);
  2019		} else /* falls through to the if below. */
  2020	#endif
  2021		if (copy_from_user(&fprog, filter, sizeof(fprog)))
  2022			goto out;
  2023	
  2024		ret = -EINVAL;
  2025		if (fprog.len == 0 || fprog.len > BPF_MAXINSNS)
  2026			goto out;
  2027	
  2028		BUG_ON(INT_MAX / fprog.len < sizeof(struct sock_filter));
  2029	
  2030		ret = bpf_prog_create_from_user(&prog, &fprog, seccomp_check_filter, save_orig);
  2031		if (ret < 0)
  2032			goto out;
  2033	
> 2034		ret = security_bpf_prog_alloc(prog->aux);
  2035		if (ret) {
  2036			ret = -EINVAL;
  2037			goto prog_free;
  2038		}
  2039	
  2040		prog->aux->user = get_current_user();
  2041		atomic64_set(&prog->aux->refcnt, 1);
  2042	
> 2043		ret = bpf_prog_new_fd(prog);
  2044		if (ret < 0)
  2045			bpf_prog_put(prog);
  2046		return ret;
  2047	
  2048	prog_free:
  2049		bpf_prog_free(prog);
  2050	out:
  2051		return ret;
  2052	}
  2053	#else
  2054	static inline long seccomp_set_mode_filter(unsigned int flags,
  2055						   const char __user *filter)
  2056	{
  2057		return -EINVAL;
  2058	}
  2059	

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

Re: [RFC PATCH 0/2] seccomp: Split set filter into two steps

[Kees Cook]

On Tue, Oct 03, 2023 at 08:38:34AM +0000, Hengqi Chen wrote:
> This patchset introduces two new operations which essentially
> splits the SECCOMP_SET_MODE_FILTER process into two steps:
> SECCOMP_LOAD_FILTER and SECCOMP_ATTACH_FILTER.
> 
> The 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 SECCOMP_ATTACH_FILTER is used to attach a loaded filter.
> The filter is represented by a fd which is either returned
> from SECCOMP_LOAD_FILTER or obtained from bpffs using bpf syscall.

Interesting! I like this idea, thanks for writing it up.

Two design notes:

- Can you reuse/refactor seccomp_prepare_filter() instead of duplicating
  the logic into two new functions?

- Is there a way to make sure the BPF program coming from the fd is one
  that was built via SECCOMP_LOAD_FILTER? (I want to make sure we can
  never confuse a non-seccomp program into getting loaded into seccomp.)

-Kees

-- 
Kees Cook

Re: [RFC PATCH 2/2] seccomp: Introduce SECCOMP_ATTACH_FILTER operation

[kernel test robot]

Hi Hengqi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on kees/for-next/seccomp]
[also build test ERROR on kees/for-next/pstore kees/for-next/kspp linus/master v6.6-rc4 next-20231003]
[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-Introduce-SECCOMP_LOAD_FILTER-operation/20231003-164556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/seccomp
patch link:    https://lore.kernel.org/r/20231003083836.100706-3-hengqi.chen%40gmail.com
patch subject: [RFC PATCH 2/2] seccomp: Introduce SECCOMP_ATTACH_FILTER operation
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20231004/202310040409.sFAAtW2o-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310040409.sFAAtW2o-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/202310040409.sFAAtW2o-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from kernel/seccomp.c:29:
   In file included from include/linux/syscalls.h:90:
   In file included from include/trace/syscall.h:7:
   In file included from include/linux/trace_events.h:9:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:5:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   kernel/seccomp.c:2034:8: error: call to undeclared function 'security_bpf_prog_alloc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2034 |         ret = security_bpf_prog_alloc(prog->aux);
         |               ^
   kernel/seccomp.c:2034:8: note: did you mean 'security_msg_msg_alloc'?
   include/linux/security.h:1245:19: note: 'security_msg_msg_alloc' declared here
    1245 | static inline int security_msg_msg_alloc(struct msg_msg *msg)
         |                   ^
   kernel/seccomp.c:2043:8: error: call to undeclared function 'bpf_prog_new_fd'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2043 |         ret = bpf_prog_new_fd(prog);
         |               ^
>> kernel/seccomp.c:2070:21: error: use of undeclared identifier 'bpf_prog_fops'; did you mean 'bpf_prog_free'?
    2070 |         if (filp->f_op != &bpf_prog_fops) {
         |                            ^~~~~~~~~~~~~
         |                            bpf_prog_free
   include/linux/filter.h:865:6: note: 'bpf_prog_free' declared here
     865 | void bpf_prog_free(struct bpf_prog *fp);
         |      ^
   12 warnings and 3 errors generated.


vim +2070 kernel/seccomp.c

  2053	
  2054	static long seccomp_attach_filter(const char __user *ufd)
  2055	{
  2056		const unsigned long seccomp_mode = SECCOMP_MODE_FILTER;
  2057		struct seccomp_filter *sfilter;
  2058		struct bpf_prog *prog;
  2059		struct file *filp;
  2060		int flags = 0;
  2061		int fd, ret;
  2062	
  2063		if (copy_from_user(&fd, ufd, sizeof(fd)))
  2064			return -EFAULT;
  2065	
  2066		filp = fget(fd);
  2067		if (!filp)
  2068			return -EBADF;
  2069	
> 2070		if (filp->f_op != &bpf_prog_fops) {
  2071			ret = -EINVAL;
  2072			goto out;
  2073		}
  2074	
  2075		prog = filp->private_data;
  2076	
  2077		sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
  2078		if (!sfilter) {
  2079			ret = -ENOMEM;
  2080			goto out;
  2081		}
  2082	
  2083		sfilter->prog = prog;
  2084		refcount_set(&sfilter->refs, 1);
  2085		refcount_set(&sfilter->users, 1);
  2086		mutex_init(&sfilter->notify_lock);
  2087		init_waitqueue_head(&sfilter->wqh);
  2088	
  2089		spin_lock_irq(&current->sighand->siglock);
  2090	
  2091		ret = -EINVAL;
  2092		if (!seccomp_may_assign_mode(seccomp_mode))
  2093			goto out_unlock;
  2094	
  2095		ret = seccomp_do_attach_filter(flags, sfilter);
  2096		if (ret)
  2097			goto out_unlock;
  2098	
  2099		sfilter = NULL;
  2100		seccomp_assign_mode(current, seccomp_mode, flags);
  2101	
  2102	out_unlock:
  2103		spin_unlock_irq(&current->sighand->siglock);
  2104		seccomp_filter_free(sfilter);
  2105	out:
  2106		fput(filp);
  2107		return ret;
  2108	}
  2109	#else
  2110	static inline long seccomp_set_mode_filter(unsigned int flags,
  2111						   const char __user *filter)
  2112	{
  2113		return -EINVAL;
  2114	}
  2115	

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

Re: [RFC PATCH 0/2] seccomp: Split set filter into two steps

[Rodrigo Campos]

On 10/3/23 10:38, Hengqi Chen wrote:
> This patchset introduces two new operations which essentially
> splits the SECCOMP_SET_MODE_FILTER process into two steps:
> SECCOMP_LOAD_FILTER and SECCOMP_ATTACH_FILTER.
> 
> The 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.

A quick question to see if handling something else too is 
possible/reasonable to do here too.

Let me explain our use case first.

For us (Alban in cc) it would be great if we can extend the lifetime of 
the fd returned, so the process managing a seccomp notification in 
userspace can easly crash or be updated. Today, if the agent that got 
the fd crashes, all the "notify-syscalls" return ENOSYS in the target 
process.

Our use case is we created a seccomp agent to use in Kubernetes 
(github.com/kinvolk/seccompagent) and we need to handle either the agent 
crashing or upgrading it. We were thinking tricks to have another 
container that just stores fds and make sure that never crashes, but it 
is not ideal (we checked tricks to use systemd to store our fds, but it 
is not simpler either to use from containers).

If the agent crashes today, all the syscalls return ENOSYS. It will be 
great if we can make the process doing the syscall just wait until a new 
process to handle the notifications is up and the syscalls done in the 
meantime are just queued. A mode of saying "if the agent crashes, just 
queue notifications, one agent to pick them up will come back soon" (we 
can of course limit reasonably the notification queue).

It seems the split here would not just work for that use case. I think 
we would need to pin the attachment.

Do you think handling that is something reasonable to do in this series too?

I'll be afk until end next week. I'll catch up as soon as I'm back with 
internet :)



Best,
Rodrigo

Re: [RFC PATCH 0/2] seccomp: Split set filter into two steps

[Hengqi Chen]

+ BPF maintainers

On Wed, Oct 4, 2023 at 2:02 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Oct 03, 2023 at 08:38:34AM +0000, Hengqi Chen wrote:
> > This patchset introduces two new operations which essentially
> > splits the SECCOMP_SET_MODE_FILTER process into two steps:
> > SECCOMP_LOAD_FILTER and SECCOMP_ATTACH_FILTER.
> >
> > The 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 SECCOMP_ATTACH_FILTER is used to attach a loaded filter.
> > The filter is represented by a fd which is either returned
> > from SECCOMP_LOAD_FILTER or obtained from bpffs using bpf syscall.
>
> Interesting! I like this idea, thanks for writing it up.
>
> Two design notes:
>
> - Can you reuse/refactor seccomp_prepare_filter() instead of duplicating
>   the logic into two new functions?
>

Sure, will do.

> - Is there a way to make sure the BPF program coming from the fd is one
>   that was built via SECCOMP_LOAD_FILTER? (I want to make sure we can
>   never confuse a non-seccomp program into getting loaded into seccomp.)
>

Maybe we can add a new prog type enum like BPF_PROG_TYPE_SECCOMP
for seccomp filter.

> -Kees
>
> --
> Kees Cook
>

Cheers,
--
Hengqi

Re: [RFC PATCH 0/2] seccomp: Split set filter into two steps

[Hengqi Chen]

On Wed, Oct 4, 2023 at 10:03 PM Rodrigo Campos <rodrigo@sdfg.com.ar> wrote:
>
> On 10/3/23 10:38, Hengqi Chen wrote:
> > This patchset introduces two new operations which essentially
> > splits the SECCOMP_SET_MODE_FILTER process into two steps:
> > SECCOMP_LOAD_FILTER and SECCOMP_ATTACH_FILTER.
> >
> > The 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.
>
> A quick question to see if handling something else too is
> possible/reasonable to do here too.
>
> Let me explain our use case first.
>
> For us (Alban in cc) it would be great if we can extend the lifetime of
> the fd returned, so the process managing a seccomp notification in
> userspace can easly crash or be updated. Today, if the agent that got
> the fd crashes, all the "notify-syscalls" return ENOSYS in the target
> process.
>
> Our use case is we created a seccomp agent to use in Kubernetes
> (github.com/kinvolk/seccompagent) and we need to handle either the agent
> crashing or upgrading it. We were thinking tricks to have another
> container that just stores fds and make sure that never crashes, but it
> is not ideal (we checked tricks to use systemd to store our fds, but it
> is not simpler either to use from containers).
>
> If the agent crashes today, all the syscalls return ENOSYS. It will be
> great if we can make the process doing the syscall just wait until a new
> process to handle the notifications is up and the syscalls done in the
> meantime are just queued. A mode of saying "if the agent crashes, just
> queue notifications, one agent to pick them up will come back soon" (we
> can of course limit reasonably the notification queue).
>
> It seems the split here would not just work for that use case. I think
> we would need to pin the attachment.
>
> Do you think handling that is something reasonable to do in this series too?
>

I am not familiar with this notification mechanism, but it seems unrelated.
This patchset is trying to reuse the seccomp filter itself.

> I'll be afk until end next week. I'll catch up as soon as I'm back with
> internet :)
>
>
>
> Best,
> Rodrigo

--
Hengqi