linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sargun Dhillon <sargun@sargun.me>
To: Kees Cook <keescook@chromium.org>, linux-kernel@vger.kernel.org
Cc: Sargun Dhillon <sargun@sargun.me>,
	Tycho Andersen <tycho@tycho.ws>,
	Matt Denton <mpdenton@google.com>, Jann Horn <jannh@google.com>,
	Chris Palmer <palmer@google.com>,
	Aleksa Sarai <cyphar@cyphar.com>,
	Robert Sesek <rsesek@google.com>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	containers@lists.linux-foundation.org,
	Giuseppe Scrivano <gscrivan@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Daniel Wagner <daniel.wagner@bmw-carit.de>,
	"David S . Miller" <davem@davemloft.net>,
	John Fastabend <john.r.fastabend@intel.com>,
	Tejun Heo <tj@kernel.org>,
	stable@vger.kernel.org, cgroups@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: [PATCH v3 1/4] fs, net: Standardize on file_receive helper to move fds across processes
Date: Tue,  2 Jun 2020 18:10:41 -0700	[thread overview]
Message-ID: <20200603011044.7972-2-sargun@sargun.me> (raw)
In-Reply-To: <20200603011044.7972-1-sargun@sargun.me>

Previously there were two chunks of code where the logic to receive file
descriptors was duplicated in net. The compat version of copying
file descriptors via SCM_RIGHTS did not have logic to update cgroups.
Logic to change the cgroup data was added in:
commit 48a87cc26c13 ("net: netprio: fd passed in SCM_RIGHTS datagram not set correctly")
commit d84295067fc7 ("net: net_cls: fd passed in SCM_RIGHTS datagram not set correctly")

This was not copied to the compat path. This commit fixes that, and thus
should be cherry-picked into stable.

This introduces a helper (file_receive) which encapsulates the logic for
handling calling security hooks as well as manipulating cgroup information.
This helper can then be used other places in the kernel where file
descriptors are copied between processes

I tested cgroup classid setting on both the compat (x32) path, and the
native path to ensure that when moving the file descriptor the classid
is set.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
Suggested-by: Kees Cook <keescook@chromium.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jann Horn <jannh@google.com>,
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Tycho Andersen <tycho@tycho.ws>
Cc: stable@vger.kernel.org
Cc: cgroups@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 fs/file.c            | 35 +++++++++++++++++++++++++++++++++++
 include/linux/file.h |  1 +
 net/compat.c         | 10 +++++-----
 net/core/scm.c       | 14 ++++----------
 4 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index abb8b7081d7a..5afd76fca8c2 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -18,6 +18,9 @@
 #include <linux/bitops.h>
 #include <linux/spinlock.h>
 #include <linux/rcupdate.h>
+#include <net/sock.h>
+#include <net/netprio_cgroup.h>
+#include <net/cls_cgroup.h>
 
 unsigned int sysctl_nr_open __read_mostly = 1024*1024;
 unsigned int sysctl_nr_open_min = BITS_PER_LONG;
@@ -931,6 +934,38 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags)
 	return err;
 }
 
+/*
+ * File Receive - Receive a file from another process
+ *
+ * This function is designed to receive files from other tasks. It encapsulates
+ * logic around security and cgroups. The file descriptor provided must be a
+ * freshly allocated (unused) file descriptor.
+ *
+ * This helper does not consume a reference to the file, so the caller must put
+ * their reference.
+ *
+ * Returns 0 upon success.
+ */
+int file_receive(int fd, struct file *file)
+{
+	struct socket *sock;
+	int err;
+
+	err = security_file_receive(file);
+	if (err)
+		return err;
+
+	fd_install(fd, get_file(file));
+
+	sock = sock_from_file(file, &err);
+	if (sock) {
+		sock_update_netprioidx(&sock->sk->sk_cgrp_data);
+		sock_update_classid(&sock->sk->sk_cgrp_data);
+	}
+
+	return 0;
+}
+
 static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags)
 {
 	int err = -EBADF;
diff --git a/include/linux/file.h b/include/linux/file.h
index 142d102f285e..7b56dc23e560 100644
--- a/include/linux/file.h
+++ b/include/linux/file.h
@@ -94,4 +94,5 @@ extern void fd_install(unsigned int fd, struct file *file);
 extern void flush_delayed_fput(void);
 extern void __fput_sync(struct file *);
 
+extern int file_receive(int fd, struct file *file);
 #endif /* __LINUX_FILE_H */
diff --git a/net/compat.c b/net/compat.c
index 4bed96e84d9a..8ac0e7e09208 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -293,9 +293,6 @@ void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm)
 
 	for (i = 0, cmfptr = (int __user *) CMSG_COMPAT_DATA(cm); i < fdmax; i++, cmfptr++) {
 		int new_fd;
-		err = security_file_receive(fp[i]);
-		if (err)
-			break;
 		err = get_unused_fd_flags(MSG_CMSG_CLOEXEC & kmsg->msg_flags
 					  ? O_CLOEXEC : 0);
 		if (err < 0)
@@ -306,8 +303,11 @@ void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm)
 			put_unused_fd(new_fd);
 			break;
 		}
-		/* Bump the usage count and install the file. */
-		fd_install(new_fd, get_file(fp[i]));
+		err = file_receive(new_fd, fp[i]);
+		if (err) {
+			put_unused_fd(new_fd);
+			break;
+		}
 	}
 
 	if (i > 0) {
diff --git a/net/core/scm.c b/net/core/scm.c
index dc6fed1f221c..ba93abf2881b 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -303,11 +303,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	for (i=0, cmfptr=(__force int __user *)CMSG_DATA(cm); i<fdmax;
 	     i++, cmfptr++)
 	{
-		struct socket *sock;
 		int new_fd;
-		err = security_file_receive(fp[i]);
-		if (err)
-			break;
 		err = get_unused_fd_flags(MSG_CMSG_CLOEXEC & msg->msg_flags
 					  ? O_CLOEXEC : 0);
 		if (err < 0)
@@ -318,13 +314,11 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 			put_unused_fd(new_fd);
 			break;
 		}
-		/* Bump the usage count and install the file. */
-		sock = sock_from_file(fp[i], &err);
-		if (sock) {
-			sock_update_netprioidx(&sock->sk->sk_cgrp_data);
-			sock_update_classid(&sock->sk->sk_cgrp_data);
+		err = file_receive(new_fd, fp[i]);
+		if (err) {
+			put_unused_fd(new_fd);
+			break;
 		}
-		fd_install(new_fd, get_file(fp[i]));
 	}
 
 	if (i > 0)
-- 
2.25.1


  reply	other threads:[~2020-06-03  1:13 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-03  1:10 [PATCH v3 0/4] Add seccomp notifier ioctl that enables adding fds Sargun Dhillon
2020-06-03  1:10 ` Sargun Dhillon [this message]
2020-06-04  1:24   ` [PATCH v3 1/4] fs, net: Standardize on file_receive helper to move fds across processes Christian Brauner
2020-06-04  2:22     ` Kees Cook
2020-06-04  5:20       ` Sargun Dhillon
2020-06-04 12:52       ` Christian Brauner
2020-06-04 13:28         ` David Laight
2020-06-05  7:54         ` Sargun Dhillon
2020-06-09 19:43           ` Kees Cook
2020-06-09 20:03             ` Christian Brauner
2020-06-09 20:55               ` Kees Cook
2020-06-09 21:27                 ` Christian Brauner
2020-06-10  5:27                   ` Kees Cook
2020-06-10  8:12                     ` Sargun Dhillon
2020-06-10  8:48                       ` David Laight
2020-06-11  3:02                         ` Kees Cook
2020-06-11  7:51                           ` David Laight
2020-06-10 17:10                       ` Kees Cook
2020-06-11  2:59                       ` Kees Cook
2020-06-11  4:41                         ` Sargun Dhillon
2020-06-11  9:19                         ` Christian Brauner
2020-06-11 10:39                           ` Sargun Dhillon
2020-06-11 23:23                             ` Kees Cook
2020-06-11 10:01                         ` Christian Brauner
2020-06-11 11:06                           ` Sargun Dhillon
2020-06-11 14:42                             ` Christian Brauner
2020-06-11 14:56                             ` David Laight
2020-06-11 23:49                               ` Kees Cook
2020-06-12  6:58                                 ` Kees Cook
2020-06-12  8:36                                 ` David Laight
2020-06-12 10:46                                   ` Sargun Dhillon
2020-06-12 15:13                                     ` Kees Cook
2020-06-12 15:55                                       ` David Laight
2020-06-12 18:28                                       ` Christian Brauner
2020-06-12 18:38                                         ` Kees Cook
2020-06-12 18:42                                           ` Christian Brauner
2020-06-15  8:27                                         ` David Laight
2020-06-10  9:30                   ` Christian Brauner
2020-06-04  3:39     ` Sargun Dhillon
2020-06-03  1:10 ` [PATCH v3 2/4] pid: Use file_receive helper to copy FDs Sargun Dhillon
2020-06-03  1:10 ` [PATCH v3 3/4] seccomp: Introduce addfd ioctl to seccomp user notifier Sargun Dhillon
2020-06-03  1:10 ` [PATCH v3 4/4] selftests/seccomp: Test SECCOMP_IOCTL_NOTIF_ADDFD Sargun Dhillon
2020-06-03 21:25 ` [PATCH v3 0/4] Add seccomp notifier ioctl that enables adding fds Robert Sesek
2020-06-03 23:42 ` Kees Cook
2020-06-03 23:56   ` Sargun Dhillon
2020-06-04  2:44     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200603011044.7972-2-sargun@sargun.me \
    --to=sargun@sargun.me \
    --cc=cgroups@vger.kernel.org \
    --cc=christian.brauner@ubuntu.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=cyphar@cyphar.com \
    --cc=daniel.wagner@bmw-carit.de \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=gscrivan@redhat.com \
    --cc=jannh@google.com \
    --cc=john.r.fastabend@intel.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpdenton@google.com \
    --cc=palmer@google.com \
    --cc=rsesek@google.com \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=tycho@tycho.ws \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).