Linux filesystem development
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: "Christian Brauner" <brauner@kernel.org>,
	"Jann Horn" <jannh@google.com>,
	"Josef Bacik" <josef@toxicpanda.com>,
	"Jeff Layton" <jlayton@kernel.org>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Daan De Meyer" <daan.j.demeyer@gmail.com>,
	"Jan Kara" <jack@suse.cz>,
	"Lennart Poettering" <lennart@poettering.net>,
	"Mike Yuan" <me@yhndnzj.com>,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
	"Alexander Mikhalitsyn" <aleksandr.mikhalitsyn@canonical.com>
Subject: [PATCH] coredump: cleanup coredump socket functions
Date: Thu,  5 Jun 2025 11:53:15 +0200	[thread overview]
Message-ID: <20250605-schlamm-touren-720ba2b60a85@brauner> (raw)

We currently use multiple CONFIG_UNIX guards. This looks messy and makes
the code harder to follow and maintain. Use a helper function
coredump_sock_connect() that handles the connect portion. This allows us
to remove the CONFIG_UNIX guard in the main do_coredump() function.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/coredump.c | 154 +++++++++++++++++++++++++++-----------------------
 1 file changed, 82 insertions(+), 72 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 26443018266e..11859d53afee 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -646,6 +646,77 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
 }
 
 #ifdef CONFIG_UNIX
+static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *cprm)
+{
+	struct file *file __free(fput) = NULL;
+	struct sockaddr_un addr = {
+		.sun_family = AF_UNIX,
+	};
+	ssize_t addr_len;
+	int retval;
+	struct socket *socket;
+
+	addr_len = strscpy(addr.sun_path, cn->corename);
+	if (addr_len < 0)
+		return false;
+	addr_len += offsetof(struct sockaddr_un, sun_path) + 1;
+
+	/*
+	 * It is possible that the userspace process which is supposed
+	 * to handle the coredump and is listening on the AF_UNIX socket
+	 * coredumps. Userspace should just mark itself non dumpable.
+	 */
+
+	retval = sock_create_kern(&init_net, AF_UNIX, SOCK_STREAM, 0, &socket);
+	if (retval < 0)
+		return false;
+
+	file = sock_alloc_file(socket, 0, NULL);
+	if (IS_ERR(file))
+		return false;
+
+	/*
+	 * Set the thread-group leader pid which is used for the peer
+	 * credentials during connect() below. Then immediately register
+	 * it in pidfs...
+	 */
+	cprm->pid = task_tgid(current);
+	retval = pidfs_register_pid(cprm->pid);
+	if (retval)
+		return false;
+
+	/*
+	 * ... and set the coredump information so userspace has it
+	 * available after connect()...
+	 */
+	pidfs_coredump(cprm);
+
+	retval = kernel_connect(socket, (struct sockaddr *)(&addr), addr_len,
+				O_NONBLOCK | SOCK_COREDUMP);
+	/*
+	 * ... Make sure to only put our reference after connect() took
+	 * its own reference keeping the pidfs entry alive ...
+	 */
+	pidfs_put_pid(cprm->pid);
+
+	if (retval) {
+		if (retval == -EAGAIN)
+			coredump_report_failure("Coredump socket %s receive queue full", addr.sun_path);
+		else
+			coredump_report_failure("Coredump socket connection %s failed %d", addr.sun_path, retval);
+		return false;
+	}
+
+	/* ... and validate that @sk_peer_pid matches @cprm.pid. */
+	if (WARN_ON_ONCE(unix_peer(socket->sk)->sk_peer_pid != cprm->pid))
+		return false;
+
+	cprm->limit = RLIM_INFINITY;
+	cprm->file = no_free_ptr(file);
+
+	return true;
+}
+
 static inline bool coredump_sock_recv(struct file *file, struct coredump_ack *ack, size_t size, int flags)
 {
 	struct msghdr msg = {};
@@ -709,7 +780,7 @@ static inline void coredump_sock_shutdown(struct file *file)
 	kernel_sock_shutdown(socket, SHUT_WR);
 }
 
-static bool coredump_request(struct core_name *cn, struct coredump_params *cprm)
+static bool coredump_sock_request(struct core_name *cn, struct coredump_params *cprm)
 {
 	struct coredump_req req = {
 		.size		= sizeof(struct coredump_req),
@@ -759,6 +830,14 @@ static bool coredump_request(struct core_name *cn, struct coredump_params *cprm)
 	return true;
 }
 #else
+static bool coredump_sock_connect(struct core_name *cn,
+				  struct coredump_params *cprm)
+{
+	coredump_report_failure("Core dump socket support %s disabled", cn->corename);
+	return false;
+}
+static bool coredump_sock_request(struct core_name *cn,
+				  struct coredump_params *cprm) { return false; }
 static inline void coredump_sock_wait(struct file *file) { }
 static inline void coredump_sock_shutdown(struct file *file) { }
 #endif
@@ -984,80 +1063,11 @@ void do_coredump(const kernel_siginfo_t *siginfo)
 	case COREDUMP_SOCK_REQ:
 		fallthrough;
 	case COREDUMP_SOCK: {
-#ifdef CONFIG_UNIX
-		struct file *file __free(fput) = NULL;
-		struct sockaddr_un addr = {
-			.sun_family = AF_UNIX,
-		};
-		ssize_t addr_len;
-		struct socket *socket;
-
-		addr_len = strscpy(addr.sun_path, cn.corename);
-		if (addr_len < 0)
-			goto close_fail;
-		addr_len += offsetof(struct sockaddr_un, sun_path) + 1;
-
-		/*
-		 * It is possible that the userspace process which is
-		 * supposed to handle the coredump and is listening on
-		 * the AF_UNIX socket coredumps. Userspace should just
-		 * mark itself non dumpable.
-		 */
-
-		retval = sock_create_kern(&init_net, AF_UNIX, SOCK_STREAM, 0, &socket);
-		if (retval < 0)
+		if (!coredump_sock_connect(&cn, &cprm))
 			goto close_fail;
 
-		file = sock_alloc_file(socket, 0, NULL);
-		if (IS_ERR(file))
+		if (!coredump_sock_request(&cn, &cprm))
 			goto close_fail;
-
-		/*
-		 * Set the thread-group leader pid which is used for the
-		 * peer credentials during connect() below. Then
-		 * immediately register it in pidfs...
-		 */
-		cprm.pid = task_tgid(current);
-		retval = pidfs_register_pid(cprm.pid);
-		if (retval)
-			goto close_fail;
-
-		/*
-		 * ... and set the coredump information so userspace
-		 * has it available after connect()...
-		 */
-		pidfs_coredump(&cprm);
-
-		retval = kernel_connect(socket, (struct sockaddr *)(&addr),
-					addr_len, O_NONBLOCK | SOCK_COREDUMP);
-
-		/*
-		 * ... Make sure to only put our reference after connect() took
-		 * its own reference keeping the pidfs entry alive ...
-		 */
-		pidfs_put_pid(cprm.pid);
-
-		if (retval) {
-			if (retval == -EAGAIN)
-				coredump_report_failure("Coredump socket %s receive queue full", addr.sun_path);
-			else
-				coredump_report_failure("Coredump socket connection %s failed %d", addr.sun_path, retval);
-			goto close_fail;
-		}
-
-		/* ... and validate that @sk_peer_pid matches @cprm.pid. */
-		if (WARN_ON_ONCE(unix_peer(socket->sk)->sk_peer_pid != cprm.pid))
-			goto close_fail;
-
-		cprm.limit = RLIM_INFINITY;
-		cprm.file = no_free_ptr(file);
-
-		if (!coredump_request(&cn, &cprm))
-			goto close_fail;
-#else
-		coredump_report_failure("Core dump socket support %s disabled", cn.corename);
-		goto close_fail;
-#endif
 		break;
 	}
 	default:
-- 
2.47.2


                 reply	other threads:[~2025-06-05  9:53 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20250605-schlamm-touren-720ba2b60a85@brauner \
    --to=brauner@kernel.org \
    --cc=aleksandr.mikhalitsyn@canonical.com \
    --cc=daan.j.demeyer@gmail.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=jlayton@kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=lennart@poettering.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=me@yhndnzj.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zbyszek@in.waw.pl \
    /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