diff mbox series

[RFC,bpf-next,seccomp,01/12] seccomp: Move no_new_privs check to after prepare_filter

Message ID 19c5ca314e69c7c3668370bcd624a2a475162cb2.1620499942.git.yifeifz2@illinois.edu (mailing list archive)
State New, archived
Headers show
Series eBPF seccomp filters | expand

Commit Message

YiFei Zhu May 10, 2021, 5:22 p.m. UTC
From: YiFei Zhu <yifeifz2@illinois.edu>

This is to make way for eBPF, so that this part of the code can be
shared by both cBPF and eBPF code paths.

Doing the privilege check after prepare_filter means that any
filter issues the caller would get -EINVAL, even when it does not
set no_new_privs or CAP_SYS_ADMIN.

Signed-off-by: YiFei Zhu <yifeifz2@illinois.edu>
---
 kernel/seccomp.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 1e63db4dbd9a..6e5ac0d686a1 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -642,16 +642,6 @@  static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
 
 	BUG_ON(INT_MAX / fprog->len < sizeof(struct sock_filter));
 
-	/*
-	 * Installing a seccomp filter requires that the task has
-	 * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
-	 * This avoids scenarios where unprivileged tasks can affect the
-	 * behavior of privileged children.
-	 */
-	if (!task_no_new_privs(current) &&
-			!ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
-		return ERR_PTR(-EACCES);
-
 	/* Allocate a new seccomp_filter */
 	sfilter = kzalloc(sizeof(*sfilter), GFP_KERNEL | __GFP_NOWARN);
 	if (!sfilter)
@@ -1805,6 +1795,22 @@  static long seccomp_set_mode_filter(unsigned int flags,
 	if (IS_ERR(prepared))
 		return PTR_ERR(prepared);
 
+	/*
+	 * Installing a seccomp filter requires that the task has
+	 * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
+	 * This avoids scenarios where unprivileged tasks can affect the
+	 * behavior of privileged children.
+	 *
+	 * This is checked after filter preparation because the user
+	 * will get an EINVAL if their filter is invalid prior to the
+	 * EACCES.
+	 */
+	if (!task_no_new_privs(current) &&
+	    !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) {
+		ret = -EACCES;
+		goto out_free;
+	}
+
 	if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) {
 		listener = get_unused_fd_flags(O_CLOEXEC);
 		if (listener < 0) {