linux-kernel - [PATCH 4/5] seccomp: Add SECCOMP_ADDFD_FLAG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 24 May 2020 16:39:41 -0700
From:   Sargun Dhillon <sargun@...gun.me>
To:     linux-kernel@...r.kernel.org,
        containers@...ts.linux-foundation.org, linux-api@...r.kernel.org
Cc:     Sargun Dhillon <sargun@...gun.me>, christian.brauner@...ntu.com,
        tycho@...ho.ws, keescook@...omium.org, cyphar@...har.com,
        Jeffrey Vander Stoep <jeffv@...gle.com>, jannh@...gle.com,
        rsesek@...gle.com, palmer@...gle.com,
        Matt Denton <mpdenton@...gle.com>,
        Kees Cook <keescook@...gle.com>
Subject: [PATCH 4/5] seccomp: Add SECCOMP_ADDFD_FLAG_MOVE flag to add fd ioctl

Certain files, when moved to another process have metadata changed, such
as netprioidx, and classid. This is the default behaviour in sending
sockets with SCM_RIGHTS over unix sockets. Depending on the usecase,
this may or may not be desirable with the addfd ioctl. This allows
the user to opt-in.

Signed-off-by: Sargun Dhillon <sargun@...gun.me>
Suggested-by: Tycho Andersen <tycho@...ho.ws>
Cc: Matt Denton <mpdenton@...gle.com>
Cc: Kees Cook <keescook@...gle.com>,
Cc: Jann Horn <jannh@...gle.com>,
Cc: Robert Sesek <rsesek@...gle.com>,
Cc: Chris Palmer <palmer@...gle.com>
Cc: Christian Brauner <christian.brauner@...ntu.com>
---
 include/uapi/linux/seccomp.h |  8 ++++++++
 kernel/seccomp.c             | 31 +++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 7d450a9e4c29..ccd1c960372a 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -115,6 +115,14 @@ struct seccomp_notif_resp {
 
 /* valid flags for seccomp_notif_addfd */
 #define SECCOMP_ADDFD_FLAG_SETFD	(1UL << 0) /* Specify remote fd */
+/*
+ * Certain file descriptors are behave differently depending on the process
+ * they are created in. Specifcally, sockets, and their interactions with the
+ * net_cls and net_prio cgroup v1 controllers. This "moves" the file descriptor
+ * so that it takes on the cgroup controller's configuration in the process
+ * that the file descriptor is being added to.
+ */
+#define SECCOMP_ADDFD_FLAG_MOVE		(1UL << 1)
 
 /**
  * struct seccomp_notif_addfd
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 88940eeabaee..2e649f3cb10e 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -41,6 +41,9 @@
 #include <linux/tracehook.h>
 #include <linux/uaccess.h>
 #include <linux/anon_inodes.h>
+#include <net/netprio_cgroup.h>
+#include <net/sock.h>
+#include <net/cls_cgroup.h>
 
 enum notify_state {
 	SECCOMP_NOTIFY_INIT,
@@ -108,6 +111,7 @@ struct seccomp_kaddfd {
 	struct file *file;
 	int fd;
 	unsigned int flags;
+	bool move;
 
 	/* To only be set on reply */
 	int ret;
@@ -769,7 +773,8 @@ static u64 seccomp_next_notify_id(struct seccomp_filter *filter)
 
 static void seccomp_handle_addfd(struct seccomp_kaddfd *addfd)
 {
-	int ret;
+	struct socket *sock;
+	int err, ret;
 
 	/*
 	 * Remove the notification, and reset the list pointers, indicating
@@ -785,12 +790,29 @@ static void seccomp_handle_addfd(struct seccomp_kaddfd *addfd)
 		ret = replace_fd(addfd->fd, addfd->file, addfd->flags);
 		if (ret >= 0)
 			fput(addfd->file);
+		else
+			goto out;
 	} else {
 		ret = get_unused_fd_flags(addfd->flags);
 		if (ret >= 0)
 			fd_install(ret, addfd->file);
+		else
+			goto out;
 	}
 
+	if (addfd->move) {
+		sock = sock_from_file(addfd->file, &err);
+		if (sock) {
+			sock_update_netprioidx(&sock->sk->sk_cgrp_data);
+			sock_update_classid(&sock->sk->sk_cgrp_data);
+		}
+	}
+	/*
+	 * An extra reference is taken on the ioctl side, so upon success, we
+	 * must consume all references (and on failure, none).
+	 */
+	fput(addfd->file);
+
 out:
 	addfd->ret = ret;
 	complete(&addfd->completion);
@@ -1279,16 +1301,17 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter,
 	if (addfd.fd_flags & (~O_CLOEXEC))
 		return -EINVAL;
 
-	if (addfd.flags & ~(SECCOMP_ADDFD_FLAG_SETFD))
+	if (addfd.flags & ~(SECCOMP_ADDFD_FLAG_SETFD|SECCOMP_ADDFD_FLAG_MOVE))
 		return -EINVAL;
 
 	if (addfd.remote_fd && !(addfd.flags & SECCOMP_ADDFD_FLAG_SETFD))
 		return -EINVAL;
 
-	kaddfd.file = fget(addfd.fd);
+	kaddfd.file = fget_many(addfd.fd, 2);
 	if (!kaddfd.file)
 		return -EBADF;
 
+	kaddfd.move = (addfd.flags & SECCOMP_ADDFD_FLAG_MOVE);
 	kaddfd.flags = addfd.fd_flags;
 	kaddfd.fd = (addfd.flags & SECCOMP_ADDFD_FLAG_SETFD) ?
 		    addfd.remote_fd : -1;
@@ -1339,7 +1362,7 @@ static long seccomp_notify_addfd(struct seccomp_filter *filter,
 	mutex_unlock(&filter->notify_lock);
 out:
 	if (ret < 0)
-		fput(kaddfd.file);
+		fput_many(kaddfd.file, 2);
 
 	return ret;
 }
-- 
2.25.1