linux-kernel - [PATCH 1/2] coredump: fix race condition between connect and putting pidfs dentry

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]

Message-Id: <20250703120244.96908-2-laurabrehm@hey.com>
Date: Thu,  3 Jul 2025 14:02:43 +0200
From: Laura Brehm <laurajfbrehm@...il.com>
To: linux-kernel@...r.kernel.org
Cc: Laura Brehm <laurabrehm@....com>,
	brauner@...nel.org,
	linux-fsdevel@...r.kernel.org
Subject: [PATCH 1/2] coredump: fix race condition between connect and putting pidfs dentry

In Commit 1d8db6fd698de1f73b1a7d72aea578fdd18d9a87 ("pidfs, coredump:
add PIDFD_INFO_COREDUMP"), the coredump handling logic puts the pidfs
entry right after `connect`, and states:

    Make sure to only put our reference after connect() took
    its own reference keeping the pidfs entry alive ...

However, `connect` does not seem to take a reference to the pidfs
entry, just the pid struct (please correct me if I'm wrong here).
Since the expectation is that the coredump server makes a
PIDFD_GET_INFO ioctl to get the coredump info - see Commit
a3b4ca60f93ff3e8b41fffbf63bb02ef3b169c5e ("coredump: add coredump
socket"):

    The pidfd for the crashing task will contain information how the
    task coredumps. The PIDFD_GET_INFO ioctl gained a new flag
    PIDFD_INFO_COREDUMP which can be used to retreive the coredump
    information.

    If the coredump gets a new coredump client connection the kernel
    guarantees that PIDFD_INFO_COREDUMP information is available.

This seems to result in the coredump server racing with the kernel to
get the pidfd before the kernel puts the pidfs entry, and if it loses
it won't be able to retrieve the coredump information.

This patch simply moves the `pidfs_put_pid` call to after the kernel is
done handing off the coredump to the coredump server.

Signed-off-by: Laura Brehm <laurabrehm@....com>
Cc: brauner@...nel.org
Cc: linux-fsdevel@...r.kernel.org
---
 fs/coredump.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index f217ebf2b3b6..a379758d9ca9 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -898,12 +898,6 @@ void do_coredump(const kernel_siginfo_t *siginfo)
 		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);
@@ -1002,6 +996,8 @@ void do_coredump(const kernel_siginfo_t *siginfo)
 close_fail:
 	if (cprm.file)
 		filp_close(cprm.file, NULL);
+	if (cn.core_type == COREDUMP_SOCK)
+		pidfs_put_pid(cprm.pid);
 fail_dropcount:
 	if (cn.core_type == COREDUMP_PIPE)
 		atomic_dec(&core_dump_count);
-- 
2.39.5 (Apple Git-154)