* [PATCH v6 0/9] coredump: add coredump socket
@ 2025-05-12 8:55 Christian Brauner
2025-05-12 8:55 ` [PATCH v6 1/9] coredump: massage format_corname() Christian Brauner
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Coredumping currently supports two modes:
(1) Dumping directly into a file somewhere on the filesystem.
(2) Dumping into a pipe connected to a usermode helper process
spawned as a child of the system_unbound_wq or kthreadd.
For simplicity I'm mostly ignoring (1). There's probably still some
users of (1) out there but processing coredumps in this way can be
considered adventurous especially in the face of set*id binaries.
The most common option should be (2) by now. It works by allowing
userspace to put a string into /proc/sys/kernel/core_pattern like:
|/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
The "|" at the beginning indicates to the kernel that a pipe must be
used. The path following the pipe indicator is a path to a binary that
will be spawned as a usermode helper process. Any additional parameters
pass information about the task that is generating the coredump to the
binary that processes the coredump.
In the example core_pattern shown above systemd-coredump is spawned as a
usermode helper. There's various conceptual consequences of this
(non-exhaustive list):
- systemd-coredump is spawned with file descriptor number 0 (stdin)
connected to the read-end of the pipe. All other file descriptors are
closed. That specifically includes 1 (stdout) and 2 (stderr). This has
already caused bugs because userspace assumed that this cannot happen
(Whether or not this is a sane assumption is irrelevant.).
- systemd-coredump will be spawned as a child of system_unbound_wq. So
it is not a child of any userspace process and specifically not a
child of PID 1. It cannot be waited upon and is in a weird hybrid
upcall which are difficult for userspace to control correctly.
- systemd-coredump is spawned with full kernel privileges. This
necessitates all kinds of weird privilege dropping excercises in
userspace to make this safe.
- A new usermode helper has to be spawned for each crashing process.
This series adds a new mode:
(3) Dumping into an abstract AF_UNIX socket.
Userspace can set /proc/sys/kernel/core_pattern to:
@address SO_COOKIE
The "@" at the beginning indicates to the kernel that the abstract
AF_UNIX coredump socket will be used to process coredumps. The address
is given by @address and must be followed by the socket cookie of the
coredump listening socket.
The socket cookie is used to verify the socket connection. If the
coredump server restarts or crashes and someone recycles the socket
address the kernel will detect that the address has been recycled as the
socket cookie will have necessarily changed and refuse to connect.
The coredump socket is located in the initial network namespace. When a
task coredumps it opens a client socket in the initial network namespace
and connects to the coredump socket.
When a task coredumps it opens a client socket in the initial network
namespace and connects to the coredump socket.
- The coredump server should use SO_PEERPIDFD to get a stable handle on
the connected crashing task. The retrieved pidfd will provide a stable
reference even if the crashing task gets SIGKILLed while generating
the coredump.
- When a coredump connection is initiated use the socket cookie as the
coredump cookie and store it in the pidfd. The receiver can now easily
authenticate that the connection is coming from the kernel.
Unless the coredump server expects to handle connection from
non-crashing task it can validate that the connection has been made from
a crashing task:
fd_coredump = accept4(fd_socket, NULL, NULL, SOCK_CLOEXEC);
getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD, &fd_peer_pidfd, &fd_peer_pidfd_len);
struct pidfd_info info = {
info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP,
};
ioctl(pidfd, PIDFD_GET_INFO, &info);
/* Refuse connections that aren't from a crashing task. */
if (!(info.mask & PIDFD_INFO_COREDUMP) || !(info.coredump_mask & PIDFD_COREDUMPED) )
close(fd_coredump);
/*
* Make sure that the coredump cookie matches the connection cookie.
* If they don't it's not the coredump connection from the kernel.
* We'll get another connection request in a bit.
*/
getsocketop(fd_coredump, SOL_SOCKET, SO_COOKIE, &peer_cookie, &peer_cookie_len);
if (!info.coredump_cookie || (info.coredump_cookie != peer_cookie))
close(fd_coredump);
The kernel guarantees that by the time the connection is made the
coredump info is available.
- By setting core_pipe_limit non-zero userspace can guarantee that the
crashing task cannot be reaped behind it's back and thus process all
necessary information in /proc/<pid>. The SO_PEERPIDFD can be used to
detect whether /proc/<pid> still refers to the same process.
The core_pipe_limit isn't used to rate-limit connections to the
socket. This can simply be done via AF_UNIX socket directly.
- 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.
Currently the following information is provided in the new
@coredump_mask extension to struct pidfd_info:
* PIDFD_COREDUMPED is raised if the task did actually coredump.
* PIDFD_COREDUMP_SKIP is raised if the task skipped coredumping (e.g.,
undumpable).
* PIDFD_COREDUMP_USER is raised if this is a regular coredump and
doesn't need special care by the coredump server.
* PIDFD_COREDUMP_ROOT is raised if the generated coredump should be
treated as sensitive and the coredump server should restrict access
to the generated coredump to sufficiently privileged users.
- The coredump server should mark itself as non-dumpable.
- A container coredump server in a separate network namespace can simply
bind to another well-know address and systemd-coredump fowards
coredumps to the container.
- Coredumps could in the future also be handled via per-user/session
coredump servers that run only with that users privileges.
The coredump server listens on the coredump socket and accepts a
new coredump connection. It then retrieves SO_PEERPIDFD for the
client, inspects uid/gid and hands the accepted client to the users
own coredump handler which runs with the users privileges only
(It must of coure pay close attention to not forward crashing suid
binaries.).
The new coredump socket will allow userspace to not have to rely on
usermode helpers for processing coredumps and provides a safer way to
handle them instead of relying on super privileged coredumping helpers.
This will also be significantly more lightweight since no fork()+exec()
for the usermodehelper is required for each crashing process. The
coredump server in userspace can just keep a worker pool.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Changes in v6:
- Use the socket cookie to verify the coredump server.
- Link to v5: https://lore.kernel.org/20250509-work-coredump-socket-v5-0-23c5b14df1bc@kernel.org
Changes in v5:
- Don't use a prefix just the specific address.
- Link to v4: https://lore.kernel.org/20250507-work-coredump-socket-v4-0-af0ef317b2d0@kernel.org
Changes in v4:
- Expose the coredump socket cookie through the pidfd. This allows the
coredump server to easily recognize coredump socket connections.
- Link to v3: https://lore.kernel.org/20250505-work-coredump-socket-v3-0-e1832f0e1eae@kernel.org
Changes in v3:
- Use an abstract unix socket.
- Add documentation.
- Add selftests.
- Link to v2: https://lore.kernel.org/20250502-work-coredump-socket-v2-0-43259042ffc7@kernel.org
Changes in v2:
- Expose dumpability via PIDFD_GET_INFO.
- Place COREDUMP_SOCK handling under CONFIG_UNIX.
- Link to v1: https://lore.kernel.org/20250430-work-coredump-socket-v1-0-2faf027dbb47@kernel.org
---
Christian Brauner (9):
coredump: massage format_corname()
coredump: massage do_coredump()
coredump: reflow dump helpers a little
coredump: add coredump socket
pidfs, coredump: add PIDFD_INFO_COREDUMP
coredump: show supported coredump modes
coredump: validate socket name as it is written
selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure
selftests/coredump: add tests for AF_UNIX coredumps
fs/coredump.c | 398 +++++++--
fs/pidfs.c | 79 ++
include/linux/net.h | 1 +
include/linux/pidfs.h | 10 +
include/uapi/linux/pidfd.h | 22 +
net/socket.c | 5 +-
net/unix/af_unix.c | 31 +-
tools/testing/selftests/coredump/stackdump_test.c | 956 +++++++++++++++++++++-
tools/testing/selftests/pidfd/pidfd.h | 23 +
9 files changed, 1425 insertions(+), 100 deletions(-)
---
base-commit: 4dd6566b5a8ca1e8c9ff2652c2249715d6c64217
change-id: 20250429-work-coredump-socket-87cc0f17729c
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 1/9] coredump: massage format_corname()
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 2/9] coredump: massage do_coredump() Christian Brauner
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
We're going to extend the coredump code in follow-up patches.
Clean it up so we can do this more easily.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 41 ++++++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index d740a0411266..368751d98781 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -76,9 +76,15 @@ static char core_pattern[CORENAME_MAX_SIZE] = "core";
static int core_name_size = CORENAME_MAX_SIZE;
unsigned int core_file_note_size_limit = CORE_FILE_NOTE_SIZE_DEFAULT;
+enum coredump_type_t {
+ COREDUMP_FILE = 1,
+ COREDUMP_PIPE = 2,
+};
+
struct core_name {
char *corename;
int used, size;
+ enum coredump_type_t core_type;
};
static int expand_corename(struct core_name *cn, int size)
@@ -218,18 +224,21 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
{
const struct cred *cred = current_cred();
const char *pat_ptr = core_pattern;
- int ispipe = (*pat_ptr == '|');
bool was_space = false;
int pid_in_pattern = 0;
int err = 0;
cn->used = 0;
cn->corename = NULL;
+ if (*pat_ptr == '|')
+ cn->core_type = COREDUMP_PIPE;
+ else
+ cn->core_type = COREDUMP_FILE;
if (expand_corename(cn, core_name_size))
return -ENOMEM;
cn->corename[0] = '\0';
- if (ispipe) {
+ if (cn->core_type == COREDUMP_PIPE) {
int argvs = sizeof(core_pattern) / 2;
(*argv) = kmalloc_array(argvs, sizeof(**argv), GFP_KERNEL);
if (!(*argv))
@@ -247,7 +256,7 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
* Split on spaces before doing template expansion so that
* %e and %E don't get split if they have spaces in them
*/
- if (ispipe) {
+ if (cn->core_type == COREDUMP_PIPE) {
if (isspace(*pat_ptr)) {
if (cn->used != 0)
was_space = true;
@@ -353,7 +362,7 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
* Installing a pidfd only makes sense if
* we actually spawn a usermode helper.
*/
- if (!ispipe)
+ if (cn->core_type != COREDUMP_PIPE)
break;
/*
@@ -384,12 +393,12 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
* If core_pattern does not include a %p (as is the default)
* and core_uses_pid is set, then .%pid will be appended to
* the filename. Do not do this for piped commands. */
- if (!ispipe && !pid_in_pattern && core_uses_pid) {
+ if (!(cn->core_type == COREDUMP_PIPE) && !pid_in_pattern && core_uses_pid) {
err = cn_printf(cn, ".%d", task_tgid_vnr(current));
if (err)
return err;
}
- return ispipe;
+ return 0;
}
static int zap_process(struct signal_struct *signal, int exit_code)
@@ -583,7 +592,6 @@ void do_coredump(const kernel_siginfo_t *siginfo)
const struct cred *old_cred;
struct cred *cred;
int retval = 0;
- int ispipe;
size_t *argv = NULL;
int argc = 0;
/* require nonrelative corefile path and be extra careful */
@@ -632,19 +640,18 @@ void do_coredump(const kernel_siginfo_t *siginfo)
old_cred = override_creds(cred);
- ispipe = format_corename(&cn, &cprm, &argv, &argc);
+ retval = format_corename(&cn, &cprm, &argv, &argc);
+ if (retval < 0) {
+ coredump_report_failure("format_corename failed, aborting core");
+ goto fail_unlock;
+ }
- if (ispipe) {
+ if (cn.core_type == COREDUMP_PIPE) {
int argi;
int dump_count;
char **helper_argv;
struct subprocess_info *sub_info;
- if (ispipe < 0) {
- coredump_report_failure("format_corename failed, aborting core");
- goto fail_unlock;
- }
-
if (cprm.limit == 1) {
/* See umh_coredump_setup() which sets RLIMIT_CORE = 1.
*
@@ -695,7 +702,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
coredump_report_failure("|%s pipe failed", cn.corename);
goto close_fail;
}
- } else {
+ } else if (cn.core_type == COREDUMP_FILE) {
struct mnt_idmap *idmap;
struct inode *inode;
int open_flags = O_CREAT | O_WRONLY | O_NOFOLLOW |
@@ -823,13 +830,13 @@ void do_coredump(const kernel_siginfo_t *siginfo)
file_end_write(cprm.file);
free_vma_snapshot(&cprm);
}
- if (ispipe && core_pipe_limit)
+ if ((cn.core_type == COREDUMP_PIPE) && core_pipe_limit)
wait_for_dump_helpers(cprm.file);
close_fail:
if (cprm.file)
filp_close(cprm.file, NULL);
fail_dropcount:
- if (ispipe)
+ if (cn.core_type == COREDUMP_PIPE)
atomic_dec(&core_dump_count);
fail_unlock:
kfree(argv);
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 2/9] coredump: massage do_coredump()
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
2025-05-12 8:55 ` [PATCH v6 1/9] coredump: massage format_corname() Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 3/9] coredump: reflow dump helpers a little Christian Brauner
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
We're going to extend the coredump code in follow-up patches.
Clean it up so we can do this more easily.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 122 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 65 insertions(+), 57 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index 368751d98781..0e97c21b35e3 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -646,63 +646,8 @@ void do_coredump(const kernel_siginfo_t *siginfo)
goto fail_unlock;
}
- if (cn.core_type == COREDUMP_PIPE) {
- int argi;
- int dump_count;
- char **helper_argv;
- struct subprocess_info *sub_info;
-
- if (cprm.limit == 1) {
- /* See umh_coredump_setup() which sets RLIMIT_CORE = 1.
- *
- * Normally core limits are irrelevant to pipes, since
- * we're not writing to the file system, but we use
- * cprm.limit of 1 here as a special value, this is a
- * consistent way to catch recursive crashes.
- * We can still crash if the core_pattern binary sets
- * RLIM_CORE = !1, but it runs as root, and can do
- * lots of stupid things.
- *
- * Note that we use task_tgid_vnr here to grab the pid
- * of the process group leader. That way we get the
- * right pid if a thread in a multi-threaded
- * core_pattern process dies.
- */
- coredump_report_failure("RLIMIT_CORE is set to 1, aborting core");
- goto fail_unlock;
- }
- cprm.limit = RLIM_INFINITY;
-
- dump_count = atomic_inc_return(&core_dump_count);
- if (core_pipe_limit && (core_pipe_limit < dump_count)) {
- coredump_report_failure("over core_pipe_limit, skipping core dump");
- goto fail_dropcount;
- }
-
- helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv),
- GFP_KERNEL);
- if (!helper_argv) {
- coredump_report_failure("%s failed to allocate memory", __func__);
- goto fail_dropcount;
- }
- for (argi = 0; argi < argc; argi++)
- helper_argv[argi] = cn.corename + argv[argi];
- helper_argv[argi] = NULL;
-
- retval = -ENOMEM;
- sub_info = call_usermodehelper_setup(helper_argv[0],
- helper_argv, NULL, GFP_KERNEL,
- umh_coredump_setup, NULL, &cprm);
- if (sub_info)
- retval = call_usermodehelper_exec(sub_info,
- UMH_WAIT_EXEC);
-
- kfree(helper_argv);
- if (retval) {
- coredump_report_failure("|%s pipe failed", cn.corename);
- goto close_fail;
- }
- } else if (cn.core_type == COREDUMP_FILE) {
+ switch (cn.core_type) {
+ case COREDUMP_FILE: {
struct mnt_idmap *idmap;
struct inode *inode;
int open_flags = O_CREAT | O_WRONLY | O_NOFOLLOW |
@@ -796,6 +741,69 @@ void do_coredump(const kernel_siginfo_t *siginfo)
if (do_truncate(idmap, cprm.file->f_path.dentry,
0, 0, cprm.file))
goto close_fail;
+ break;
+ }
+ case COREDUMP_PIPE: {
+ int argi;
+ int dump_count;
+ char **helper_argv;
+ struct subprocess_info *sub_info;
+
+ if (cprm.limit == 1) {
+ /* See umh_coredump_setup() which sets RLIMIT_CORE = 1.
+ *
+ * Normally core limits are irrelevant to pipes, since
+ * we're not writing to the file system, but we use
+ * cprm.limit of 1 here as a special value, this is a
+ * consistent way to catch recursive crashes.
+ * We can still crash if the core_pattern binary sets
+ * RLIM_CORE = !1, but it runs as root, and can do
+ * lots of stupid things.
+ *
+ * Note that we use task_tgid_vnr here to grab the pid
+ * of the process group leader. That way we get the
+ * right pid if a thread in a multi-threaded
+ * core_pattern process dies.
+ */
+ coredump_report_failure("RLIMIT_CORE is set to 1, aborting core");
+ goto fail_unlock;
+ }
+ cprm.limit = RLIM_INFINITY;
+
+ dump_count = atomic_inc_return(&core_dump_count);
+ if (core_pipe_limit && (core_pipe_limit < dump_count)) {
+ coredump_report_failure("over core_pipe_limit, skipping core dump");
+ goto fail_dropcount;
+ }
+
+ helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv),
+ GFP_KERNEL);
+ if (!helper_argv) {
+ coredump_report_failure("%s failed to allocate memory", __func__);
+ goto fail_dropcount;
+ }
+ for (argi = 0; argi < argc; argi++)
+ helper_argv[argi] = cn.corename + argv[argi];
+ helper_argv[argi] = NULL;
+
+ retval = -ENOMEM;
+ sub_info = call_usermodehelper_setup(helper_argv[0],
+ helper_argv, NULL, GFP_KERNEL,
+ umh_coredump_setup, NULL, &cprm);
+ if (sub_info)
+ retval = call_usermodehelper_exec(sub_info,
+ UMH_WAIT_EXEC);
+
+ kfree(helper_argv);
+ if (retval) {
+ coredump_report_failure("|%s pipe failed", cn.corename);
+ goto close_fail;
+ }
+ break;
+ }
+ default:
+ WARN_ON_ONCE(true);
+ goto close_fail;
}
/* get us an unshared descriptor table; almost always a no-op */
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 3/9] coredump: reflow dump helpers a little
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
2025-05-12 8:55 ` [PATCH v6 1/9] coredump: massage format_corname() Christian Brauner
2025-05-12 8:55 ` [PATCH v6 2/9] coredump: massage do_coredump() Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
They look rather messy right now.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index 0e97c21b35e3..a70929c3585b 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -867,10 +867,9 @@ static int __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
struct file *file = cprm->file;
loff_t pos = file->f_pos;
ssize_t n;
+
if (cprm->written + nr > cprm->limit)
return 0;
-
-
if (dump_interrupted())
return 0;
n = __kernel_write(file, addr, nr, &pos);
@@ -887,20 +886,21 @@ static int __dump_skip(struct coredump_params *cprm, size_t nr)
{
static char zeroes[PAGE_SIZE];
struct file *file = cprm->file;
+
if (file->f_mode & FMODE_LSEEK) {
- if (dump_interrupted() ||
- vfs_llseek(file, nr, SEEK_CUR) < 0)
+ if (dump_interrupted() || vfs_llseek(file, nr, SEEK_CUR) < 0)
return 0;
cprm->pos += nr;
return 1;
- } else {
- while (nr > PAGE_SIZE) {
- if (!__dump_emit(cprm, zeroes, PAGE_SIZE))
- return 0;
- nr -= PAGE_SIZE;
- }
- return __dump_emit(cprm, zeroes, nr);
}
+
+ while (nr > PAGE_SIZE) {
+ if (!__dump_emit(cprm, zeroes, PAGE_SIZE))
+ return 0;
+ nr -= PAGE_SIZE;
+ }
+
+ return __dump_emit(cprm, zeroes, nr);
}
int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 4/9] coredump: add coredump socket
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (2 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 3/9] coredump: reflow dump helpers a little Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 10:58 ` Luca Boccassi
2025-05-13 0:06 ` Kuniyuki Iwashima
2025-05-12 8:55 ` [PATCH v6 5/9] pidfs, coredump: add PIDFD_INFO_COREDUMP Christian Brauner
` (4 subsequent siblings)
8 siblings, 2 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Coredumping currently supports two modes:
(1) Dumping directly into a file somewhere on the filesystem.
(2) Dumping into a pipe connected to a usermode helper process
spawned as a child of the system_unbound_wq or kthreadd.
For simplicity I'm mostly ignoring (1). There's probably still some
users of (1) out there but processing coredumps in this way can be
considered adventurous especially in the face of set*id binaries.
The most common option should be (2) by now. It works by allowing
userspace to put a string into /proc/sys/kernel/core_pattern like:
|/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
The "|" at the beginning indicates to the kernel that a pipe must be
used. The path following the pipe indicator is a path to a binary that
will be spawned as a usermode helper process. Any additional parameters
pass information about the task that is generating the coredump to the
binary that processes the coredump.
In the example core_pattern shown above systemd-coredump is spawned as a
usermode helper. There's various conceptual consequences of this
(non-exhaustive list):
- systemd-coredump is spawned with file descriptor number 0 (stdin)
connected to the read-end of the pipe. All other file descriptors are
closed. That specifically includes 1 (stdout) and 2 (stderr). This has
already caused bugs because userspace assumed that this cannot happen
(Whether or not this is a sane assumption is irrelevant.).
- systemd-coredump will be spawned as a child of system_unbound_wq. So
it is not a child of any userspace process and specifically not a
child of PID 1. It cannot be waited upon and is in a weird hybrid
upcall which are difficult for userspace to control correctly.
- systemd-coredump is spawned with full kernel privileges. This
necessitates all kinds of weird privilege dropping excercises in
userspace to make this safe.
- A new usermode helper has to be spawned for each crashing process.
This series adds a new mode:
(3) Dumping into an abstract AF_UNIX socket.
Userspace can set /proc/sys/kernel/core_pattern to:
@address SO_COOKIE
The "@" at the beginning indicates to the kernel that the abstract
AF_UNIX coredump socket will be used to process coredumps. The address
is given by @address and must be followed by the socket cookie of the
coredump listening socket.
The socket cookie is used to verify the socket connection. If the
coredump server restarts or crashes and someone recycles the socket
address the kernel will detect that the address has been recycled as the
socket cookie will have necessarily changed and refuse to connect.
The coredump socket is located in the initial network namespace. When a
task coredumps it opens a client socket in the initial network namespace
and connects to the coredump socket.
- The coredump server uses SO_PEERPIDFD to get a stable handle on the
connected crashing task. The retrieved pidfd will provide a stable
reference even if the crashing task gets SIGKILLed while generating
the coredump.
- By setting core_pipe_limit non-zero userspace can guarantee that the
crashing task cannot be reaped behind it's back and thus process all
necessary information in /proc/<pid>. The SO_PEERPIDFD can be used to
detect whether /proc/<pid> still refers to the same process.
The core_pipe_limit isn't used to rate-limit connections to the
socket. This can simply be done via AF_UNIX sockets directly.
- The pidfd for the crashing task will grow new information how the task
coredumps.
- The coredump server should mark itself as non-dumpable.
- A container coredump server in a separate network namespace can simply
bind to another well-know address and systemd-coredump fowards
coredumps to the container.
- Coredumps could in the future also be handled via per-user/session
coredump servers that run only with that users privileges.
The coredump server listens on the coredump socket and accepts a
new coredump connection. It then retrieves SO_PEERPIDFD for the
client, inspects uid/gid and hands the accepted client to the users
own coredump handler which runs with the users privileges only
(It must of coure pay close attention to not forward crashing suid
binaries.).
The new coredump socket will allow userspace to not have to rely on
usermode helpers for processing coredumps and provides a safer way to
handle them instead of relying on super privileged coredumping helpers
that have and continue to cause significant CVEs.
This will also be significantly more lightweight since no fork()+exec()
for the usermodehelper is required for each crashing process. The
coredump server in userspace can e.g., just keep a worker pool.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++---
include/linux/net.h | 1 +
net/socket.c | 5 +-
net/unix/af_unix.c | 24 ++++++---
4 files changed, 161 insertions(+), 16 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index a70929c3585b..15e9d9a252cd 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -44,7 +44,11 @@
#include <linux/sysctl.h>
#include <linux/elf.h>
#include <linux/pidfs.h>
+#include <linux/net.h>
+#include <linux/socket.h>
+#include <net/net_namespace.h>
#include <uapi/linux/pidfd.h>
+#include <uapi/linux/un.h>
#include <linux/uaccess.h>
#include <asm/mmu_context.h>
@@ -79,6 +83,7 @@ unsigned int core_file_note_size_limit = CORE_FILE_NOTE_SIZE_DEFAULT;
enum coredump_type_t {
COREDUMP_FILE = 1,
COREDUMP_PIPE = 2,
+ COREDUMP_SOCK = 3,
};
struct core_name {
@@ -232,13 +237,16 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
cn->corename = NULL;
if (*pat_ptr == '|')
cn->core_type = COREDUMP_PIPE;
+ else if (*pat_ptr == '@')
+ cn->core_type = COREDUMP_SOCK;
else
cn->core_type = COREDUMP_FILE;
if (expand_corename(cn, core_name_size))
return -ENOMEM;
cn->corename[0] = '\0';
- if (cn->core_type == COREDUMP_PIPE) {
+ switch (cn->core_type) {
+ case COREDUMP_PIPE: {
int argvs = sizeof(core_pattern) / 2;
(*argv) = kmalloc_array(argvs, sizeof(**argv), GFP_KERNEL);
if (!(*argv))
@@ -247,6 +255,29 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
++pat_ptr;
if (!(*pat_ptr))
return -ENOMEM;
+ break;
+ }
+ case COREDUMP_SOCK: {
+ /* skip the @ */
+ pat_ptr++;
+ err = cn_printf(cn, "%s", pat_ptr);
+ if (err)
+ return err;
+
+ /*
+ * Currently no need to parse any other options.
+ * Relevant information can be retrieved from the peer
+ * pidfd retrievable via SO_PEERPIDFD by the receiver or
+ * via /proc/<pid>, using the SO_PEERPIDFD to guard
+ * against pid recycling when opening /proc/<pid>.
+ */
+ return 0;
+ }
+ case COREDUMP_FILE:
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ return -EINVAL;
}
/* Repeat as long as we have more pattern to process and more output
@@ -393,11 +424,20 @@ static int format_corename(struct core_name *cn, struct coredump_params *cprm,
* If core_pattern does not include a %p (as is the default)
* and core_uses_pid is set, then .%pid will be appended to
* the filename. Do not do this for piped commands. */
- if (!(cn->core_type == COREDUMP_PIPE) && !pid_in_pattern && core_uses_pid) {
- err = cn_printf(cn, ".%d", task_tgid_vnr(current));
- if (err)
- return err;
+ if (!pid_in_pattern && core_uses_pid) {
+ switch (cn->core_type) {
+ case COREDUMP_FILE:
+ return cn_printf(cn, ".%d", task_tgid_vnr(current));
+ case COREDUMP_PIPE:
+ break;
+ case COREDUMP_SOCK:
+ break;
+ default:
+ WARN_ON_ONCE(true);
+ return -EINVAL;
+ }
}
+
return 0;
}
@@ -801,6 +841,73 @@ void do_coredump(const kernel_siginfo_t *siginfo)
}
break;
}
+ case COREDUMP_SOCK: {
+#ifdef CONFIG_UNIX
+ struct file *file __free(fput) = NULL;
+ struct sockaddr_un addr = {
+ .sun_family = AF_UNIX,
+ };
+ unsigned int addr_len;
+ struct socket *socket;
+ char *p;
+ u64 sock_cookie;
+
+ p = strchr(cn.corename, ' ');
+ if (!p) {
+ coredump_report_failure("Missing socket cookie");
+ goto close_fail;
+ }
+ *p++ = '\0';
+
+ /* Leave room for the socket cookie. */
+ retval = strscpy(addr.sun_path + 1, cn.corename,
+ sizeof(addr.sun_path) - sizeof(sock_cookie) - 1);
+ if (retval < 0)
+ goto close_fail;
+ addr_len = offsetof(struct sockaddr_un, sun_path) + retval + 1;
+
+ retval = kstrtou64(p, 0, &sock_cookie);
+ if (retval || !sock_cookie) {
+ coredump_report_failure("Invalid socket cookie");
+ goto close_fail;
+ }
+
+ /* append socket cookie */
+ memcpy((char *)&addr + addr_len, &sock_cookie, sizeof(sock_cookie));
+
+ /*
+ * 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)
+ goto close_fail;
+
+ file = sock_alloc_file(socket, 0, NULL);
+ if (IS_ERR(file)) {
+ sock_release(socket);
+ goto close_fail;
+ }
+
+ retval = kernel_connect(socket, (struct sockaddr *)(&addr),
+ addr_len, O_NONBLOCK | SOCK_COREDUMP);
+ if (retval) {
+ if (retval == -EAGAIN)
+ coredump_report_failure("Skipping as coredump socket connection %s couldn't complete immediately", cn.corename);
+ goto close_fail;
+ }
+
+ cprm.limit = RLIM_INFINITY;
+ cprm.file = no_free_ptr(file);
+#else
+ coredump_report_failure("Core dump socket support %s disabled", cn.corename);
+ goto close_fail;
+#endif
+ break;
+ }
default:
WARN_ON_ONCE(true);
goto close_fail;
@@ -838,8 +945,32 @@ void do_coredump(const kernel_siginfo_t *siginfo)
file_end_write(cprm.file);
free_vma_snapshot(&cprm);
}
- if ((cn.core_type == COREDUMP_PIPE) && core_pipe_limit)
- wait_for_dump_helpers(cprm.file);
+
+ /*
+ * When core_pipe_limit is set we wait for the coredump server
+ * or usermodehelper to finish before exiting so it can e.g.,
+ * inspect /proc/<pid>.
+ */
+ if (core_pipe_limit) {
+ switch (cn.core_type) {
+ case COREDUMP_PIPE:
+ wait_for_dump_helpers(cprm.file);
+ break;
+ case COREDUMP_SOCK: {
+ /*
+ * We use a simple read to wait for the coredump
+ * processing to finish. Either the socket is
+ * closed or we get sent unexpected data. In
+ * both cases, we're done.
+ */
+ __kernel_read(cprm.file, &(char){ 0 }, 1, NULL);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
close_fail:
if (cprm.file)
filp_close(cprm.file, NULL);
@@ -1069,7 +1200,7 @@ EXPORT_SYMBOL(dump_align);
void validate_coredump_safety(void)
{
if (suid_dumpable == SUID_DUMP_ROOT &&
- core_pattern[0] != '/' && core_pattern[0] != '|') {
+ core_pattern[0] != '/' && core_pattern[0] != '|' && core_pattern[0] != '@') {
coredump_report_failure("Unsafe core_pattern used with fs.suid_dumpable=2: "
"pipe handler or fully qualified core dump path required. "
diff --git a/include/linux/net.h b/include/linux/net.h
index 0ff950eecc6b..139c85d0f2ea 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -81,6 +81,7 @@ enum sock_type {
#ifndef SOCK_NONBLOCK
#define SOCK_NONBLOCK O_NONBLOCK
#endif
+#define SOCK_COREDUMP O_NOCTTY
#endif /* ARCH_HAS_SOCKET_TYPES */
diff --git a/net/socket.c b/net/socket.c
index 9a0e720f0859..d62a57a57a28 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3603,7 +3603,10 @@ int kernel_connect(struct socket *sock, struct sockaddr *addr, int addrlen,
{
struct sockaddr_storage address;
- memcpy(&address, addr, addrlen);
+ if (flags & SOCK_COREDUMP)
+ memcpy(&address, addr, addrlen + sizeof(sock->sk->sk_cookie));
+ else
+ memcpy(&address, addr, addrlen);
return READ_ONCE(sock->ops)->connect(sock, (struct sockaddr *)&address,
addrlen, flags);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 472f8aa9ea15..6b8a7863b41c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -89,6 +89,8 @@
#include <linux/kernel.h>
#include <linux/mount.h>
#include <linux/namei.h>
+#include <linux/net.h>
+#include <linux/pidfs.h>
#include <linux/poll.h>
#include <linux/proc_fs.h>
#include <linux/sched/signal.h>
@@ -100,7 +102,6 @@
#include <linux/splice.h>
#include <linux/string.h>
#include <linux/uaccess.h>
-#include <linux/pidfs.h>
#include <net/af_unix.h>
#include <net/net_namespace.h>
#include <net/scm.h>
@@ -1191,7 +1192,7 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
static struct sock *unix_find_abstract(struct net *net,
struct sockaddr_un *sunaddr,
- int addr_len, int type)
+ int addr_len, int type, int flags)
{
unsigned int hash = unix_abstract_hash(sunaddr, addr_len, type);
struct dentry *dentry;
@@ -1201,6 +1202,15 @@ static struct sock *unix_find_abstract(struct net *net,
if (!sk)
return ERR_PTR(-ECONNREFUSED);
+ if (flags & SOCK_COREDUMP) {
+ u64 sock_cookie;
+
+ memcpy(&sock_cookie, (char *)sunaddr + addr_len, sizeof(sock_cookie));
+ DEBUG_NET_WARN_ON_ONCE(!sock_cookie);
+ if (sock_cookie != atomic64_read(&sk->sk_cookie))
+ return ERR_PTR(-ECONNREFUSED);
+ }
+
dentry = unix_sk(sk)->path.dentry;
if (dentry)
touch_atime(&unix_sk(sk)->path);
@@ -1210,14 +1220,14 @@ static struct sock *unix_find_abstract(struct net *net,
static struct sock *unix_find_other(struct net *net,
struct sockaddr_un *sunaddr,
- int addr_len, int type)
+ int addr_len, int type, int flags)
{
struct sock *sk;
if (sunaddr->sun_path[0])
sk = unix_find_bsd(sunaddr, addr_len, type);
else
- sk = unix_find_abstract(net, sunaddr, addr_len, type);
+ sk = unix_find_abstract(net, sunaddr, addr_len, type, flags);
return sk;
}
@@ -1473,7 +1483,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
}
restart:
- other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type);
+ other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type, 0);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out;
@@ -1620,7 +1630,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
restart:
/* Find listening sock. */
- other = unix_find_other(net, sunaddr, addr_len, sk->sk_type);
+ other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out_free_skb;
@@ -2089,7 +2099,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
if (msg->msg_namelen) {
lookup:
other = unix_find_other(sock_net(sk), msg->msg_name,
- msg->msg_namelen, sk->sk_type);
+ msg->msg_namelen, sk->sk_type, 0);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out_free;
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 5/9] pidfs, coredump: add PIDFD_INFO_COREDUMP
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (3 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 6/9] coredump: show supported coredump modes Christian Brauner
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Extend the PIDFD_INFO_COREDUMP ioctl() with the new PIDFD_INFO_COREDUMP
mask flag. This adds the fields @coredump_mask and @coredump_cookie to
struct pidfd_info.
When a task coredumps the kernel will provide the following information
to userspace in @coredump_mask:
* PIDFD_COREDUMPED is raised if the task did actually coredump.
* PIDFD_COREDUMP_SKIP is raised if the task skipped coredumping (e.g.,
undumpable).
* PIDFD_COREDUMP_USER is raised if this is a regular coredump and
doesn't need special care by the coredump server.
* PIDFD_COREDUMP_ROOT is raised if the generated coredump should be
treated as sensitive and the coredump server should restrict to the
generated coredump to sufficiently privileged users.
If userspace uses the coredump socket to process coredumps it needs to
be able to discern connection from the kernel from connects from
userspace (e.g., Python generating it's own coredumps and forwarding
them to systemd). The @coredump_cookie extension uses the SO_COOKIE of
the new connection. This allows userspace to validate that the
connection has been made from the kernel by a crashing task:
fd_coredump = accept4(fd_socket, NULL, NULL, SOCK_CLOEXEC);
getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD, &fd_peer_pidfd, &fd_peer_pidfd_len);
struct pidfd_info info = {
info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP,
};
ioctl(pidfd, PIDFD_GET_INFO, &info);
/* Refuse connections that aren't from a crashing task. */
if (!(info.mask & PIDFD_INFO_COREDUMP) || !(info.coredump_mask & PIDFD_COREDUMPED) )
close(fd_coredump);
/*
* Make sure that the coredump cookie matches the connection cookie.
* If they don't it's not the coredump connection from the kernel.
* We'll get another connection request in a bit.
*/
getsocketop(fd_coredump, SOL_SOCKET, SO_COOKIE, &peer_cookie, &peer_cookie_len);
if (!info.coredump_cookie || (info.coredump_cookie != peer_cookie))
close(fd_coredump);
The kernel guarantees that by the time the connection is made the all
PIDFD_INFO_COREDUMP info is available.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 34 ++++++++++++++++++++
fs/pidfs.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pidfs.h | 10 ++++++
include/uapi/linux/pidfd.h | 22 +++++++++++++
net/unix/af_unix.c | 7 ++++
5 files changed, 152 insertions(+)
diff --git a/fs/coredump.c b/fs/coredump.c
index 15e9d9a252cd..5ea0c93dd5ac 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -46,7 +46,9 @@
#include <linux/pidfs.h>
#include <linux/net.h>
#include <linux/socket.h>
+#include <net/af_unix.h>
#include <net/net_namespace.h>
+#include <net/sock.h>
#include <uapi/linux/pidfd.h>
#include <uapi/linux/un.h>
@@ -594,6 +596,8 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
if (IS_ERR(pidfs_file))
return PTR_ERR(pidfs_file);
+ pidfs_coredump(cp);
+
/*
* Usermode helpers are childen of either
* system_unbound_wq or of kthreadd. So we know that
@@ -892,14 +896,44 @@ void do_coredump(const kernel_siginfo_t *siginfo)
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) {
+ sock_release(socket);
+ goto close_fail;
+ }
+
+ /*
+ * ... and set the coredump information so userspace
+ * has it available after connect()...
+ */
+ pidfs_coredump(&cprm);
+
+ /*
+ * ... On connect() the peer credentials are recorded
+ * and @cprm.pid registered in pidfs...
+ */
retval = kernel_connect(socket, (struct sockaddr *)(&addr),
addr_len, O_NONBLOCK | SOCK_COREDUMP);
+
+ /* ... So we can safely put our pidfs reference now... */
+ pidfs_put_pid(cprm.pid);
+
if (retval) {
if (retval == -EAGAIN)
coredump_report_failure("Skipping as coredump socket connection %s couldn't complete immediately", cn.corename);
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);
#else
diff --git a/fs/pidfs.c b/fs/pidfs.c
index 3b39e471840b..d7b9a0dd2db6 100644
--- a/fs/pidfs.c
+++ b/fs/pidfs.c
@@ -20,6 +20,7 @@
#include <linux/time_namespace.h>
#include <linux/utsname.h>
#include <net/net_namespace.h>
+#include <linux/coredump.h>
#include "internal.h"
#include "mount.h"
@@ -33,6 +34,8 @@ static struct kmem_cache *pidfs_cachep __ro_after_init;
struct pidfs_exit_info {
__u64 cgroupid;
__s32 exit_code;
+ __u32 coredump_mask;
+ __u64 coredump_cookie;
};
struct pidfs_inode {
@@ -240,6 +243,22 @@ static inline bool pid_in_current_pidns(const struct pid *pid)
return false;
}
+static __u32 pidfs_coredump_mask(unsigned long mm_flags)
+{
+ switch (__get_dumpable(mm_flags)) {
+ case SUID_DUMP_USER:
+ return PIDFD_COREDUMP_USER;
+ case SUID_DUMP_ROOT:
+ return PIDFD_COREDUMP_ROOT;
+ case SUID_DUMP_DISABLE:
+ return PIDFD_COREDUMP_SKIP;
+ default:
+ WARN_ON_ONCE(true);
+ }
+
+ return 0;
+}
+
static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
{
struct pidfd_info __user *uinfo = (struct pidfd_info __user *)arg;
@@ -280,6 +299,13 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
}
}
+ if (mask & PIDFD_INFO_COREDUMP) {
+ kinfo.mask |= PIDFD_INFO_COREDUMP;
+ smp_rmb();
+ kinfo.coredump_cookie = READ_ONCE(pidfs_i(inode)->__pei.coredump_cookie);
+ kinfo.coredump_mask = READ_ONCE(pidfs_i(inode)->__pei.coredump_mask);
+ }
+
task = get_pid_task(pid, PIDTYPE_PID);
if (!task) {
/*
@@ -296,6 +322,16 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
if (!c)
return -ESRCH;
+ if (!(kinfo.mask & PIDFD_INFO_COREDUMP)) {
+ task_lock(task);
+ if (task->mm) {
+ smp_rmb();
+ kinfo.coredump_cookie = READ_ONCE(pidfs_i(inode)->__pei.coredump_cookie);
+ kinfo.coredump_mask = pidfs_coredump_mask(task->mm->flags);
+ }
+ task_unlock(task);
+ }
+
/* Unconditionally return identifiers and credentials, the rest only on request */
user_ns = current_user_ns();
@@ -559,6 +595,49 @@ void pidfs_exit(struct task_struct *tsk)
}
}
+#if defined(CONFIG_COREDUMP) && defined(CONFIG_UNIX)
+void pidfs_coredump_cookie(struct pid *pid, u64 coredump_cookie)
+{
+ struct pidfs_exit_info *exit_info;
+ struct dentry *dentry = pid->stashed;
+ struct inode *inode;
+
+ if (WARN_ON_ONCE(!dentry))
+ return;
+
+ inode = d_inode(dentry);
+ exit_info = &pidfs_i(inode)->__pei;
+ /* Can't use smp_store_release() because of 32bit. */
+ smp_wmb();
+ WRITE_ONCE(exit_info->coredump_cookie, coredump_cookie);
+}
+#endif
+
+#ifdef CONFIG_COREDUMP
+void pidfs_coredump(const struct coredump_params *cprm)
+{
+ struct pid *pid = cprm->pid;
+ struct pidfs_exit_info *exit_info;
+ struct dentry *dentry;
+ struct inode *inode;
+ __u32 coredump_mask = 0;
+
+ dentry = pid->stashed;
+ if (WARN_ON_ONCE(!dentry))
+ return;
+
+ inode = d_inode(dentry);
+ exit_info = &pidfs_i(inode)->__pei;
+ /* Note how we were coredumped. */
+ coredump_mask = pidfs_coredump_mask(cprm->mm_flags);
+ /* Note that we actually did coredump. */
+ coredump_mask |= PIDFD_COREDUMPED;
+ /* If coredumping is set to skip we should never end up here. */
+ VFS_WARN_ON_ONCE(coredump_mask & PIDFD_COREDUMP_SKIP);
+ smp_store_release(&exit_info->coredump_mask, coredump_mask);
+}
+#endif
+
static struct vfsmount *pidfs_mnt __ro_after_init;
/*
diff --git a/include/linux/pidfs.h b/include/linux/pidfs.h
index 2676890c4d0d..497997bc5e34 100644
--- a/include/linux/pidfs.h
+++ b/include/linux/pidfs.h
@@ -2,11 +2,21 @@
#ifndef _LINUX_PID_FS_H
#define _LINUX_PID_FS_H
+struct coredump_params;
+
struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags);
void __init pidfs_init(void);
void pidfs_add_pid(struct pid *pid);
void pidfs_remove_pid(struct pid *pid);
void pidfs_exit(struct task_struct *tsk);
+#ifdef CONFIG_COREDUMP
+void pidfs_coredump(const struct coredump_params *cprm);
+#endif
+#if defined(CONFIG_COREDUMP) && defined(CONFIG_UNIX)
+void pidfs_coredump_cookie(struct pid *pid, u64 coredump_cookie);
+#elif defined(CONFIG_UNIX)
+static inline void pidfs_coredump_cookie(struct pid *pid, u64 coredump_cookie) { }
+#endif
extern const struct dentry_operations pidfs_dentry_operations;
int pidfs_register_pid(struct pid *pid);
void pidfs_get_pid(struct pid *pid);
diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h
index 8c1511edd0e9..69267c5ae6d0 100644
--- a/include/uapi/linux/pidfd.h
+++ b/include/uapi/linux/pidfd.h
@@ -25,9 +25,28 @@
#define PIDFD_INFO_CREDS (1UL << 1) /* Always returned, even if not requested */
#define PIDFD_INFO_CGROUPID (1UL << 2) /* Always returned if available, even if not requested */
#define PIDFD_INFO_EXIT (1UL << 3) /* Only returned if requested. */
+#define PIDFD_INFO_COREDUMP (1UL << 4) /* Only returned if requested. */
#define PIDFD_INFO_SIZE_VER0 64 /* sizeof first published struct */
+/*
+ * Values for @coredump_mask in pidfd_info.
+ * Only valid if PIDFD_INFO_COREDUMP is set in @mask.
+ *
+ * Note, the @PIDFD_COREDUMP_ROOT flag indicates that the generated
+ * coredump should be treated as sensitive and access should only be
+ * granted to privileged users.
+ *
+ * If the coredump AF_UNIX socket is used for processing coredumps
+ * @coredump_cookie will be set to the socket SO_COOKIE of the receivers
+ * client socket. This allows the coredump handler to detect whether an
+ * incoming coredump connection was initiated from the crashing task.
+ */
+#define PIDFD_COREDUMPED (1U << 0) /* Did crash and... */
+#define PIDFD_COREDUMP_SKIP (1U << 1) /* coredumping generation was skipped. */
+#define PIDFD_COREDUMP_USER (1U << 2) /* coredump was done as the user. */
+#define PIDFD_COREDUMP_ROOT (1U << 3) /* coredump was done as root. */
+
/*
* The concept of process and threads in userland and the kernel is a confusing
* one - within the kernel every thread is a 'task' with its own individual PID,
@@ -92,6 +111,9 @@ struct pidfd_info {
__u32 fsuid;
__u32 fsgid;
__s32 exit_code;
+ __u32 coredump_mask;
+ __u32 __spare1;
+ __u64 coredump_cookie;
};
#define PIDFS_IOCTL_MAGIC 0xFF
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 6b8a7863b41c..8308f74f8490 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -98,6 +98,7 @@
#include <linux/seq_file.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
+#include <linux/sock_diag.h>
#include <linux/socket.h>
#include <linux/splice.h>
#include <linux/string.h>
@@ -741,6 +742,7 @@ static void unix_release_sock(struct sock *sk, int embrion)
struct unix_peercred {
struct pid *peer_pid;
+ u64 cookie;
const struct cred *peer_cred;
};
@@ -776,6 +778,8 @@ static void drop_peercred(struct unix_peercred *peercred)
static inline void init_peercred(struct sock *sk,
const struct unix_peercred *peercred)
{
+ if (peercred->cookie)
+ pidfs_coredump_cookie(peercred->peer_pid, peercred->cookie);
sk->sk_peer_pid = peercred->peer_pid;
sk->sk_peer_cred = peercred->peer_cred;
}
@@ -1696,6 +1700,9 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
unix_peer(newsk) = sk;
newsk->sk_state = TCP_ESTABLISHED;
newsk->sk_type = sk->sk_type;
+ /* Prepare a new socket cookie for the receiver. */
+ if (flags & SOCK_COREDUMP)
+ peercred.cookie = sock_gen_cookie(newsk);
init_peercred(newsk, &peercred);
newu = unix_sk(newsk);
newu->listener = other;
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 6/9] coredump: show supported coredump modes
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (4 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 5/9] pidfs, coredump: add PIDFD_INFO_COREDUMP Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 7/9] coredump: validate socket name as it is written Christian Brauner
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Allow userspace to discover what coredump modes are supported.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/fs/coredump.c b/fs/coredump.c
index 5ea0c93dd5ac..deee52bff6bc 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1254,6 +1254,12 @@ static int proc_dostring_coredump(const struct ctl_table *table, int write,
static const unsigned int core_file_note_size_min = CORE_FILE_NOTE_SIZE_DEFAULT;
static const unsigned int core_file_note_size_max = CORE_FILE_NOTE_SIZE_MAX;
+static char core_modes[] = {
+ "file\npipe"
+#ifdef CONFIG_UNIX
+ "\nsocket"
+#endif
+};
static const struct ctl_table coredump_sysctls[] = {
{
@@ -1297,6 +1303,13 @@ static const struct ctl_table coredump_sysctls[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
+ {
+ .procname = "core_modes",
+ .data = core_modes,
+ .maxlen = sizeof(core_modes) - 1,
+ .mode = 0444,
+ .proc_handler = proc_dostring,
+ },
};
static int __init init_fs_coredump_sysctls(void)
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 7/9] coredump: validate socket name as it is written
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (5 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 6/9] coredump: show supported coredump modes Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 8/9] selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure Christian Brauner
2025-05-12 8:55 ` [PATCH v6 9/9] selftests/coredump: add tests for AF_UNIX coredumps Christian Brauner
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
In contrast to other parameters written into
/proc/sys/kernel/core_pattern that never fail we can validate enabling
the new AF_UNIX support. This is obviously racy as hell but it's always
been that way.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
fs/coredump.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index deee52bff6bc..5b9d2e063f8f 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1242,13 +1242,36 @@ void validate_coredump_safety(void)
}
}
+static inline bool check_coredump_socket_address(void)
+{
+ if (core_pattern[0] != '@')
+ return true;
+
+ /* Leave enough space for the socket cookie. */
+ if (strcspn(core_pattern, " ") >= (UNIX_PATH_MAX - sizeof(u64)))
+ return false;
+
+ return true;
+}
+
static int proc_dostring_coredump(const struct ctl_table *table, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
- int error = proc_dostring(table, write, buffer, lenp, ppos);
+ int error;
+ ssize_t retval;
+ char old_core_pattern[CORENAME_MAX_SIZE];
+
+ retval = strscpy(old_core_pattern, core_pattern, CORENAME_MAX_SIZE);
+
+ error = proc_dostring(table, write, buffer, lenp, ppos);
+ if (error)
+ return error;
+ if (!check_coredump_socket_address()) {
+ strscpy(core_pattern, old_core_pattern, retval + 1);
+ return -EINVAL;
+ }
- if (!error)
- validate_coredump_safety();
+ validate_coredump_safety();
return error;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 8/9] selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (6 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 7/9] coredump: validate socket name as it is written Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
2025-05-12 8:55 ` [PATCH v6 9/9] selftests/coredump: add tests for AF_UNIX coredumps Christian Brauner
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Add PIDFD_INFO_COREDUMP infrastructure so we can use it in tests.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
tools/testing/selftests/pidfd/pidfd.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
index 55bcf81a2b9a..887c74007086 100644
--- a/tools/testing/selftests/pidfd/pidfd.h
+++ b/tools/testing/selftests/pidfd/pidfd.h
@@ -131,6 +131,26 @@
#define PIDFD_INFO_EXIT (1UL << 3) /* Always returned if available, even if not requested */
#endif
+#ifndef PIDFD_INFO_COREDUMP
+#define PIDFD_INFO_COREDUMP (1UL << 4)
+#endif
+
+#ifndef PIDFD_COREDUMPED
+#define PIDFD_COREDUMPED (1U << 0) /* Did crash and... */
+#endif
+
+#ifndef PIDFD_COREDUMP_SKIP
+#define PIDFD_COREDUMP_SKIP (1U << 1) /* coredumping generation was skipped. */
+#endif
+
+#ifndef PIDFD_COREDUMP_USER
+#define PIDFD_COREDUMP_USER (1U << 2) /* coredump was done as the user. */
+#endif
+
+#ifndef PIDFD_COREDUMP_ROOT
+#define PIDFD_COREDUMP_ROOT (1U << 3) /* coredump was done as root. */
+#endif
+
#ifndef PIDFD_THREAD
#define PIDFD_THREAD O_EXCL
#endif
@@ -150,6 +170,9 @@ struct pidfd_info {
__u32 fsuid;
__u32 fsgid;
__s32 exit_code;
+ __u32 coredump_mask;
+ __u32 __spare1;
+ __u64 coredump_cookie;
};
/*
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6 9/9] selftests/coredump: add tests for AF_UNIX coredumps
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
` (7 preceding siblings ...)
2025-05-12 8:55 ` [PATCH v6 8/9] selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure Christian Brauner
@ 2025-05-12 8:55 ` Christian Brauner
8 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-12 8:55 UTC (permalink / raw)
To: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima
Cc: Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Luca Boccassi, Mike Yuan, Paolo Abeni,
Simon Horman, Zbigniew Jędrzejewski-Szmek, linux-kernel,
netdev, linux-security-module, Christian Brauner,
Alexander Mikhalitsyn
Add a simple test for generating coredumps via AF_UNIX sockets.
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
tools/testing/selftests/coredump/stackdump_test.c | 956 +++++++++++++++++++++-
1 file changed, 955 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/coredump/stackdump_test.c b/tools/testing/selftests/coredump/stackdump_test.c
index fe3c728cd6be..5a74612ae42e 100644
--- a/tools/testing/selftests/coredump/stackdump_test.c
+++ b/tools/testing/selftests/coredump/stackdump_test.c
@@ -1,14 +1,20 @@
// SPDX-License-Identifier: GPL-2.0
#include <fcntl.h>
+#include <inttypes.h>
#include <libgen.h>
#include <linux/limits.h>
#include <pthread.h>
#include <string.h>
+#include <sys/mount.h>
#include <sys/resource.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/un.h>
#include <unistd.h>
#include "../kselftest_harness.h"
+#include "../pidfd/pidfd.h"
#define STACKDUMP_FILE "stack_values"
#define STACKDUMP_SCRIPT "stackdump"
@@ -35,6 +41,7 @@ static void crashing_child(void)
FIXTURE(coredump)
{
char original_core_pattern[256];
+ pid_t pid_coredump_server;
};
FIXTURE_SETUP(coredump)
@@ -44,6 +51,7 @@ FIXTURE_SETUP(coredump)
char *dir;
int ret;
+ self->pid_coredump_server = -ESRCH;
file = fopen("/proc/sys/kernel/core_pattern", "r");
ASSERT_NE(NULL, file);
@@ -61,10 +69,15 @@ FIXTURE_TEARDOWN(coredump)
{
const char *reason;
FILE *file;
- int ret;
+ int ret, status;
unlink(STACKDUMP_FILE);
+ if (self->pid_coredump_server > 0) {
+ kill(self->pid_coredump_server, SIGTERM);
+ waitpid(self->pid_coredump_server, &status, 0);
+ }
+
file = fopen("/proc/sys/kernel/core_pattern", "w");
if (!file) {
reason = "Unable to open core_pattern";
@@ -154,4 +167,945 @@ TEST_F_TIMEOUT(coredump, stackdump, 120)
fclose(file);
}
+TEST_F(coredump, socket)
+{
+ int fd, pidfd, ret, status;
+ FILE *file;
+ pid_t pid, pid_coredump_server;
+ struct stat st;
+ char core_file[PATH_MAX];
+ struct pidfd_info info = {};
+ int ipc_sockets[2];
+ char c;
+
+ ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+ ASSERT_EQ(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL), 0);
+ ASSERT_EQ(mount(NULL, "/tmp", "tmpfs", 0, NULL), 0);
+
+ ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
+ ASSERT_EQ(ret, 0);
+
+ pid_coredump_server = fork();
+ ASSERT_GE(pid_coredump_server, 0);
+ if (pid_coredump_server == 0) {
+ int fd_server, fd_coredump, fd_peer_pidfd, fd_core_file;
+ __u64 peer_cookie, server_cookie;
+ socklen_t fd_peer_pidfd_len, peer_cookie_len, server_cookie_len;
+ struct sockaddr_un coredump_sk = {
+ .sun_family = AF_UNIX,
+ };
+ size_t coredump_sk_len;
+
+ close(ipc_sockets[0]);
+
+ fd_server = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (fd_server < 0)
+ _exit(EXIT_FAILURE);
+
+ server_cookie_len = sizeof(server_cookie);
+ ret = getsockopt(fd_server, SOL_SOCKET, SO_COOKIE,
+ &server_cookie, &server_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket server\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk_len = snprintf(coredump_sk.sun_path,
+ sizeof(coredump_sk.sun_path),
+ "@coredump.socket %" PRIu64, server_cookie);
+ if (coredump_sk_len < 0 || coredump_sk_len >= sizeof(coredump_sk.sun_path)) {
+ fprintf(stderr, "Unable to create coredump socket path\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ file = fopen("/proc/sys/kernel/core_pattern", "w");
+ if (!file) {
+ fprintf(stderr, "Unable to open core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = fprintf(file, "%s", coredump_sk.sun_path);
+ fclose(file);
+ if (ret < 0) {
+ fprintf(stderr, "Unable to write to core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk.sun_path[0] = '\0';
+ memcpy(coredump_sk.sun_path, "\0coredump.socket", sizeof("coredump.socket"));
+ coredump_sk_len = offsetof(struct sockaddr_un, sun_path) + sizeof("coredump.socket");
+ ret = bind(fd_server, (const struct sockaddr *)&coredump_sk, coredump_sk_len);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to bind coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = listen(fd_server, 1);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to listen on coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (write_nointr(ipc_sockets[1], "1", 1) < 0) {
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ close(ipc_sockets[1]);
+
+ fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+ if (fd_coredump < 0) {
+ fprintf(stderr, "Failed to accept coredump socket connection\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ peer_cookie_len = sizeof(peer_cookie);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_COOKIE,
+ &peer_cookie, &peer_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_peer_pidfd_len = sizeof(fd_peer_pidfd);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD,
+ &fd_peer_pidfd, &fd_peer_pidfd_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ret = ioctl(fd_peer_pidfd, PIDFD_GET_INFO, &info);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to retrieve pidfd info from peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.mask & PIDFD_INFO_COREDUMP)) {
+ fprintf(stderr, "Missing coredump information from coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.coredump_mask & PIDFD_COREDUMPED)) {
+ fprintf(stderr, "Received connection from non-coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!info.coredump_cookie) {
+ fprintf(stderr, "Missing coredump cookie\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (info.coredump_cookie != peer_cookie) {
+ fprintf(stderr, "Mismatching coredump cookies\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_core_file = creat("/tmp/coredump.file", 0644);
+ if (fd_core_file < 0) {
+ fprintf(stderr, "Failed to create coredump file\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ for (;;) {
+ char buffer[4096];
+ ssize_t bytes_read, bytes_write;
+
+ bytes_read = read(fd_coredump, buffer, sizeof(buffer));
+ if (bytes_read < 0) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (bytes_read == 0)
+ break;
+
+ bytes_write = write(fd_core_file, buffer, bytes_read);
+ if (bytes_read != bytes_write) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+ }
+
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_SUCCESS);
+ }
+ self->pid_coredump_server = pid_coredump_server;
+
+ EXPECT_EQ(close(ipc_sockets[1]), 0);
+ ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+ EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+ if (pid == 0)
+ crashing_child();
+
+ pidfd = sys_pidfd_open(pid, 0);
+ ASSERT_GE(pidfd, 0);
+
+ waitpid(pid, &status, 0);
+ ASSERT_TRUE(WIFSIGNALED(status));
+ ASSERT_TRUE(WCOREDUMP(status));
+
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ASSERT_EQ(ioctl(pidfd, PIDFD_GET_INFO, &info), 0);
+ ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+ ASSERT_GT((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+ waitpid(pid_coredump_server, &status, 0);
+ self->pid_coredump_server = -ESRCH;
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(WEXITSTATUS(status), 0);
+
+ ASSERT_EQ(stat("/tmp/coredump.file", &st), 0);
+ ASSERT_GT(st.st_size, 0);
+ /*
+ * We should somehow validate the produced core file.
+ * For now just allow for visual inspection
+ */
+ system("file /tmp/coredump.file");
+}
+
+TEST_F(coredump, socket_recycled_cookie)
+{
+ int fd, pidfd, ret, status;
+ FILE *file;
+ pid_t pid, pid_coredump_server;
+ struct stat st;
+ char core_file[PATH_MAX];
+ struct pidfd_info info = {};
+ int ipc_sockets[2];
+ char c;
+
+ ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+ ASSERT_EQ(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL), 0);
+ ASSERT_EQ(mount(NULL, "/tmp", "tmpfs", 0, NULL), 0);
+
+ ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
+ ASSERT_EQ(ret, 0);
+
+ pid_coredump_server = fork();
+ ASSERT_GE(pid_coredump_server, 0);
+ if (pid_coredump_server == 0) {
+ int fd_server, fd_coredump, fd_peer_pidfd, fd_core_file;
+ __u64 peer_cookie, server_cookie;
+ socklen_t fd_peer_pidfd_len, peer_cookie_len, server_cookie_len;
+ struct sockaddr_un coredump_sk = {
+ .sun_family = AF_UNIX,
+ };
+ size_t coredump_sk_len;
+
+ close(ipc_sockets[0]);
+
+ fd_server = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (fd_server < 0)
+ _exit(EXIT_FAILURE);
+
+ server_cookie_len = sizeof(server_cookie);
+ ret = getsockopt(fd_server, SOL_SOCKET, SO_COOKIE,
+ &server_cookie, &server_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket server\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ /* Write invalid socket cookie to core_pattern. */
+ coredump_sk_len = snprintf(coredump_sk.sun_path,
+ sizeof(coredump_sk.sun_path),
+ "@coredump.socket %" PRIu64, 0);
+ if (coredump_sk_len < 0 || coredump_sk_len >= sizeof(coredump_sk.sun_path)) {
+ fprintf(stderr, "Unable to create coredump socket path\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ file = fopen("/proc/sys/kernel/core_pattern", "w");
+ if (!file) {
+ fprintf(stderr, "Unable to open core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = fprintf(file, "%s", coredump_sk.sun_path);
+ fclose(file);
+ if (ret < 0) {
+ fprintf(stderr, "Unable to write to core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk.sun_path[0] = '\0';
+ memcpy(coredump_sk.sun_path, "\0coredump.socket", sizeof("coredump.socket"));
+ coredump_sk_len = offsetof(struct sockaddr_un, sun_path) + sizeof("coredump.socket");
+ ret = bind(fd_server, (const struct sockaddr *)&coredump_sk, coredump_sk_len);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to bind coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = listen(fd_server, 1);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to listen on coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (write_nointr(ipc_sockets[1], "1", 1) < 0) {
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ close(ipc_sockets[1]);
+
+ fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+ if (fd_coredump < 0) {
+ fprintf(stderr, "Failed to accept coredump socket connection\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ peer_cookie_len = sizeof(peer_cookie);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_COOKIE,
+ &peer_cookie, &peer_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_peer_pidfd_len = sizeof(fd_peer_pidfd);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD,
+ &fd_peer_pidfd, &fd_peer_pidfd_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ret = ioctl(fd_peer_pidfd, PIDFD_GET_INFO, &info);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to retrieve pidfd info from peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.mask & PIDFD_INFO_COREDUMP)) {
+ fprintf(stderr, "Missing coredump information from coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.coredump_mask & PIDFD_COREDUMPED)) {
+ fprintf(stderr, "Received connection from non-coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!info.coredump_cookie) {
+ fprintf(stderr, "Missing coredump cookie\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (info.coredump_cookie != peer_cookie) {
+ fprintf(stderr, "Mismatching coredump cookies\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_core_file = creat("/tmp/coredump.file", 0644);
+ if (fd_core_file < 0) {
+ fprintf(stderr, "Failed to create coredump file\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ for (;;) {
+ char buffer[4096];
+ ssize_t bytes_read, bytes_write;
+
+ bytes_read = read(fd_coredump, buffer, sizeof(buffer));
+ if (bytes_read < 0) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (bytes_read == 0)
+ break;
+
+ bytes_write = write(fd_core_file, buffer, bytes_read);
+ if (bytes_read != bytes_write) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+ }
+
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_SUCCESS);
+ }
+ self->pid_coredump_server = pid_coredump_server;
+
+ EXPECT_EQ(close(ipc_sockets[1]), 0);
+ ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+ EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+ if (pid == 0)
+ crashing_child();
+
+ pidfd = sys_pidfd_open(pid, 0);
+ ASSERT_GE(pidfd, 0);
+
+ waitpid(pid, &status, 0);
+ ASSERT_TRUE(WIFSIGNALED(status));
+ ASSERT_FALSE(WCOREDUMP(status));
+
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ASSERT_EQ(ioctl(pidfd, PIDFD_GET_INFO, &info), 0);
+ ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+ ASSERT_EQ((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+ ASSERT_EQ(kill(pid_coredump_server, SIGKILL), 0);
+ waitpid(pid_coredump_server, &status, 0);
+ self->pid_coredump_server = -ESRCH;
+ ASSERT_TRUE(WIFSIGNALED(status));
+ ASSERT_EQ(WTERMSIG(status), SIGKILL);
+
+ ASSERT_NE(stat("/tmp/coredump.file", &st), 0);
+ ASSERT_EQ(errno, ENOENT);
+}
+
+TEST_F(coredump, socket_missing_cookie)
+{
+ int fd, pidfd, ret, status;
+ FILE *file;
+ pid_t pid, pid_coredump_server;
+ struct stat st;
+ char core_file[PATH_MAX];
+ struct pidfd_info info = {};
+ int ipc_sockets[2];
+ char c;
+
+ ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+ ASSERT_EQ(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL), 0);
+ ASSERT_EQ(mount(NULL, "/tmp", "tmpfs", 0, NULL), 0);
+
+ ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
+ ASSERT_EQ(ret, 0);
+
+ pid_coredump_server = fork();
+ ASSERT_GE(pid_coredump_server, 0);
+ if (pid_coredump_server == 0) {
+ int fd_server, fd_coredump, fd_peer_pidfd, fd_core_file;
+ __u64 peer_cookie, server_cookie;
+ socklen_t fd_peer_pidfd_len, peer_cookie_len, server_cookie_len;
+ struct sockaddr_un coredump_sk = {
+ .sun_family = AF_UNIX,
+ };
+ size_t coredump_sk_len;
+
+ close(ipc_sockets[0]);
+
+ fd_server = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (fd_server < 0)
+ _exit(EXIT_FAILURE);
+
+ server_cookie_len = sizeof(server_cookie);
+ ret = getsockopt(fd_server, SOL_SOCKET, SO_COOKIE,
+ &server_cookie, &server_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket server\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ /* Don't write socket cookie to core_pattern. */
+ coredump_sk_len = snprintf(coredump_sk.sun_path,
+ sizeof(coredump_sk.sun_path),
+ "@coredump.socket");
+ if (coredump_sk_len < 0 || coredump_sk_len >= sizeof(coredump_sk.sun_path)) {
+ fprintf(stderr, "Unable to create coredump socket path\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ file = fopen("/proc/sys/kernel/core_pattern", "w");
+ if (!file) {
+ fprintf(stderr, "Unable to open core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = fprintf(file, "%s", coredump_sk.sun_path);
+ fclose(file);
+ if (ret < 0) {
+ fprintf(stderr, "Unable to write to core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk.sun_path[0] = '\0';
+ memcpy(coredump_sk.sun_path, "\0coredump.socket", sizeof("coredump.socket"));
+ coredump_sk_len = offsetof(struct sockaddr_un, sun_path) + sizeof("coredump.socket");
+ ret = bind(fd_server, (const struct sockaddr *)&coredump_sk, coredump_sk_len);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to bind coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = listen(fd_server, 1);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to listen on coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (write_nointr(ipc_sockets[1], "1", 1) < 0) {
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ close(ipc_sockets[1]);
+
+ fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+ if (fd_coredump < 0) {
+ fprintf(stderr, "Failed to accept coredump socket connection\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ peer_cookie_len = sizeof(peer_cookie);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_COOKIE,
+ &peer_cookie, &peer_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_peer_pidfd_len = sizeof(fd_peer_pidfd);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD,
+ &fd_peer_pidfd, &fd_peer_pidfd_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ret = ioctl(fd_peer_pidfd, PIDFD_GET_INFO, &info);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to retrieve pidfd info from peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.mask & PIDFD_INFO_COREDUMP)) {
+ fprintf(stderr, "Missing coredump information from coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.coredump_mask & PIDFD_COREDUMPED)) {
+ fprintf(stderr, "Received connection from non-coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!info.coredump_cookie) {
+ fprintf(stderr, "Missing coredump cookie\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (info.coredump_cookie != peer_cookie) {
+ fprintf(stderr, "Mismatching coredump cookies\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_core_file = creat("/tmp/coredump.file", 0644);
+ if (fd_core_file < 0) {
+ fprintf(stderr, "Failed to create coredump file\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ for (;;) {
+ char buffer[4096];
+ ssize_t bytes_read, bytes_write;
+
+ bytes_read = read(fd_coredump, buffer, sizeof(buffer));
+ if (bytes_read < 0) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (bytes_read == 0)
+ break;
+
+ bytes_write = write(fd_core_file, buffer, bytes_read);
+ if (bytes_read != bytes_write) {
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_FAILURE);
+ }
+ }
+
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_SUCCESS);
+ }
+ self->pid_coredump_server = pid_coredump_server;
+
+ EXPECT_EQ(close(ipc_sockets[1]), 0);
+ ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+ EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+ if (pid == 0)
+ crashing_child();
+
+ pidfd = sys_pidfd_open(pid, 0);
+ ASSERT_GE(pidfd, 0);
+
+ waitpid(pid, &status, 0);
+ ASSERT_TRUE(WIFSIGNALED(status));
+ ASSERT_FALSE(WCOREDUMP(status));
+
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ASSERT_EQ(ioctl(pidfd, PIDFD_GET_INFO, &info), 0);
+ ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+ ASSERT_EQ((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+ ASSERT_EQ(kill(pid_coredump_server, SIGKILL), 0);
+ waitpid(pid_coredump_server, &status, 0);
+ self->pid_coredump_server = -ESRCH;
+ ASSERT_TRUE(WIFSIGNALED(status));
+ ASSERT_EQ(WTERMSIG(status), SIGKILL);
+
+ ASSERT_NE(stat("/tmp/coredump.file", &st), 0);
+ ASSERT_EQ(errno, ENOENT);
+}
+
+TEST_F(coredump, socket_detect_userspace_client)
+{
+ int fd, pidfd, ret, status;
+ FILE *file;
+ pid_t pid, pid_coredump_server;
+ struct stat st;
+ char core_file[PATH_MAX];
+ struct pidfd_info info = {};
+ int ipc_sockets[2];
+ char c;
+
+ ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+ ASSERT_EQ(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL), 0);
+ ASSERT_EQ(mount(NULL, "/tmp", "tmpfs", 0, NULL), 0);
+
+ ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
+ ASSERT_EQ(ret, 0);
+
+ pid_coredump_server = fork();
+ ASSERT_GE(pid_coredump_server, 0);
+ if (pid_coredump_server == 0) {
+ int fd_server, fd_coredump, fd_peer_pidfd, fd_core_file;
+ __u64 peer_cookie, server_cookie;
+ socklen_t fd_peer_pidfd_len, peer_cookie_len, server_cookie_len;
+ struct sockaddr_un coredump_sk = {
+ .sun_family = AF_UNIX,
+ };
+ size_t coredump_sk_len;
+
+ close(ipc_sockets[0]);
+
+ fd_server = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ if (fd_server < 0)
+ _exit(EXIT_FAILURE);
+
+ server_cookie_len = sizeof(server_cookie);
+ ret = getsockopt(fd_server, SOL_SOCKET, SO_COOKIE,
+ &server_cookie, &server_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket server\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk_len = snprintf(coredump_sk.sun_path,
+ sizeof(coredump_sk.sun_path),
+ "@coredump.socket %" PRIu64, server_cookie);
+ if (coredump_sk_len < 0 || coredump_sk_len >= sizeof(coredump_sk.sun_path)) {
+ fprintf(stderr, "Unable to create coredump socket path\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ file = fopen("/proc/sys/kernel/core_pattern", "w");
+ if (!file) {
+ fprintf(stderr, "Unable to open core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = fprintf(file, "%s", coredump_sk.sun_path);
+ fclose(file);
+ if (ret < 0) {
+ fprintf(stderr, "Unable to write to core_pattern\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ coredump_sk.sun_path[0] = '\0';
+ memcpy(coredump_sk.sun_path, "\0coredump.socket", sizeof("coredump.socket"));
+ coredump_sk_len = offsetof(struct sockaddr_un, sun_path) + sizeof("coredump.socket");
+ ret = bind(fd_server, (const struct sockaddr *)&coredump_sk, coredump_sk_len);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to bind coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = listen(fd_server, 1);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to listen on coredump socket\n");
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (write_nointr(ipc_sockets[1], "1", 1) < 0) {
+ close(fd_server);
+ close(ipc_sockets[1]);
+ _exit(EXIT_FAILURE);
+ }
+
+ close(ipc_sockets[1]);
+
+ fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+ if (fd_coredump < 0) {
+ fprintf(stderr, "Failed to accept coredump socket connection\n");
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ peer_cookie_len = sizeof(peer_cookie);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_COOKIE,
+ &peer_cookie, &peer_cookie_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve cookie for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ fd_peer_pidfd_len = sizeof(fd_peer_pidfd);
+ ret = getsockopt(fd_coredump, SOL_SOCKET, SO_PEERPIDFD,
+ &fd_peer_pidfd, &fd_peer_pidfd_len);
+ if (ret < 0) {
+ fprintf(stderr, "%m - Failed to retrieve peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ _exit(EXIT_FAILURE);
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ret = ioctl(fd_peer_pidfd, PIDFD_GET_INFO, &info);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to retrieve pidfd info from peer pidfd for coredump socket connection\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (!(info.mask & PIDFD_INFO_COREDUMP)) {
+ fprintf(stderr, "Missing coredump information from coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (info.coredump_mask & PIDFD_COREDUMPED) {
+ fprintf(stderr, "Received unexpected connection from coredumping task\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (info.coredump_cookie) {
+ fprintf(stderr, "Received unexpected coredump cookie\n");
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ _exit(EXIT_FAILURE);
+ }
+
+ close(fd_coredump);
+ close(fd_server);
+ close(fd_peer_pidfd);
+ close(fd_core_file);
+ _exit(EXIT_SUCCESS);
+ }
+ self->pid_coredump_server = pid_coredump_server;
+
+ EXPECT_EQ(close(ipc_sockets[1]), 0);
+ ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+ EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+ if (pid == 0) {
+ struct sockaddr_un coredump_sk = {
+ .sun_family = AF_UNIX,
+ };
+ socklen_t coredump_sk_len;
+ int fd_socket;
+ ssize_t ret;
+
+ fd_socket = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd_socket < 0)
+ _exit(EXIT_FAILURE);
+
+
+ coredump_sk.sun_path[0] = '\0';
+ memcpy(coredump_sk.sun_path, "\0coredump.socket", sizeof("coredump.socket"));
+ coredump_sk_len = offsetof(struct sockaddr_un, sun_path) + sizeof("coredump.socket");
+
+ ret = connect(fd_socket, (const struct sockaddr *)&coredump_sk, coredump_sk_len);
+ if (ret < 0)
+ _exit(EXIT_FAILURE);
+
+ (void *)write(fd_socket, &(char){ 0 }, 1);
+ close(fd_socket);
+ _exit(EXIT_SUCCESS);
+ }
+
+ pidfd = sys_pidfd_open(pid, 0);
+ ASSERT_GE(pidfd, 0);
+
+ waitpid(pid, &status, 0);
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(WEXITSTATUS(status), 0);
+
+ info.mask = PIDFD_INFO_EXIT | PIDFD_INFO_COREDUMP;
+ ASSERT_EQ(ioctl(pidfd, PIDFD_GET_INFO, &info), 0);
+ ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+ ASSERT_EQ((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+ waitpid(pid_coredump_server, &status, 0);
+ self->pid_coredump_server = -ESRCH;
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(WEXITSTATUS(status), 0);
+
+ ASSERT_NE(stat("/tmp/coredump.file", &st), 0);
+ ASSERT_EQ(errno, ENOENT);
+}
+
TEST_HARNESS_MAIN
--
2.47.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
@ 2025-05-12 10:58 ` Luca Boccassi
2025-05-13 0:17 ` Kuniyuki Iwashima
2025-05-13 0:06 ` Kuniyuki Iwashima
1 sibling, 1 reply; 17+ messages in thread
From: Luca Boccassi @ 2025-05-12 10:58 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-fsdevel, Jann Horn, Daniel Borkmann, Kuniyuki Iwashima,
Eric Dumazet, Oleg Nesterov, David S. Miller, Alexander Viro,
Daan De Meyer, David Rheinsberg, Jakub Kicinski, Jan Kara,
Lennart Poettering, Mike Yuan, Paolo Abeni, Simon Horman,
Zbigniew Jędrzejewski-Szmek, linux-kernel, netdev,
linux-security-module, Alexander Mikhalitsyn
On Mon, 12 May 2025 at 09:56, Christian Brauner <brauner@kernel.org> wrote:
>
> Coredumping currently supports two modes:
>
> (1) Dumping directly into a file somewhere on the filesystem.
> (2) Dumping into a pipe connected to a usermode helper process
> spawned as a child of the system_unbound_wq or kthreadd.
>
> For simplicity I'm mostly ignoring (1). There's probably still some
> users of (1) out there but processing coredumps in this way can be
> considered adventurous especially in the face of set*id binaries.
>
> The most common option should be (2) by now. It works by allowing
> userspace to put a string into /proc/sys/kernel/core_pattern like:
>
> |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
>
> The "|" at the beginning indicates to the kernel that a pipe must be
> used. The path following the pipe indicator is a path to a binary that
> will be spawned as a usermode helper process. Any additional parameters
> pass information about the task that is generating the coredump to the
> binary that processes the coredump.
>
> In the example core_pattern shown above systemd-coredump is spawned as a
> usermode helper. There's various conceptual consequences of this
> (non-exhaustive list):
>
> - systemd-coredump is spawned with file descriptor number 0 (stdin)
> connected to the read-end of the pipe. All other file descriptors are
> closed. That specifically includes 1 (stdout) and 2 (stderr). This has
> already caused bugs because userspace assumed that this cannot happen
> (Whether or not this is a sane assumption is irrelevant.).
>
> - systemd-coredump will be spawned as a child of system_unbound_wq. So
> it is not a child of any userspace process and specifically not a
> child of PID 1. It cannot be waited upon and is in a weird hybrid
> upcall which are difficult for userspace to control correctly.
>
> - systemd-coredump is spawned with full kernel privileges. This
> necessitates all kinds of weird privilege dropping excercises in
> userspace to make this safe.
>
> - A new usermode helper has to be spawned for each crashing process.
>
> This series adds a new mode:
>
> (3) Dumping into an abstract AF_UNIX socket.
>
> Userspace can set /proc/sys/kernel/core_pattern to:
>
> @address SO_COOKIE
>
> The "@" at the beginning indicates to the kernel that the abstract
> AF_UNIX coredump socket will be used to process coredumps. The address
> is given by @address and must be followed by the socket cookie of the
> coredump listening socket.
>
> The socket cookie is used to verify the socket connection. If the
> coredump server restarts or crashes and someone recycles the socket
> address the kernel will detect that the address has been recycled as the
> socket cookie will have necessarily changed and refuse to connect.
This dynamic/cookie prefix makes it impossible to use this with socket
activation units. The way systemd-coredump works is that every
instance is an independent templated unit, spawned when there's a
connection to the private socket. If the path was fixed, we could just
reuse the same mechanism, it would fit very nicely with minimal
changes.
But because you need a "server" to be permanently running, this means
socket-based activation can no longer work, and systemd-coredump must
switch to a persistently-running mode. This is a severe degradation of
functionality, will continuously waste CPU/memory resources for no
good reasons, and makes the whole thing more fragile and complex, as
if there are any issues with this server, you start losing core files.
And honestly I don't really see the point? Setting the pattern is a
privileged operation anyway. systemd manages the socket with a socket
unit and again that's privileged already.
Could we drop this cookie prefix and go back to the previous version
(v5), please? Or if there is some specific non-systemd use case in
mind that I am not aware of, have both options, so that we can use the
simpler and more straightforward one with systemd-coredump.
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
2025-05-12 10:58 ` Luca Boccassi
@ 2025-05-13 0:06 ` Kuniyuki Iwashima
1 sibling, 0 replies; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-05-13 0:06 UTC (permalink / raw)
To: brauner
Cc: alexander, bluca, daan.j.demeyer, daniel, davem, david, edumazet,
horms, jack, jannh, kuba, kuniyu, lennart, linux-fsdevel,
linux-kernel, linux-security-module, me, netdev, oleg, pabeni,
viro, zbyszek
From: Christian Brauner <brauner@kernel.org>
Date: Mon, 12 May 2025 10:55:23 +0200
> Coredumping currently supports two modes:
>
> (1) Dumping directly into a file somewhere on the filesystem.
> (2) Dumping into a pipe connected to a usermode helper process
> spawned as a child of the system_unbound_wq or kthreadd.
>
> For simplicity I'm mostly ignoring (1). There's probably still some
> users of (1) out there but processing coredumps in this way can be
> considered adventurous especially in the face of set*id binaries.
>
> The most common option should be (2) by now. It works by allowing
> userspace to put a string into /proc/sys/kernel/core_pattern like:
>
> |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
>
> The "|" at the beginning indicates to the kernel that a pipe must be
> used. The path following the pipe indicator is a path to a binary that
> will be spawned as a usermode helper process. Any additional parameters
> pass information about the task that is generating the coredump to the
> binary that processes the coredump.
>
> In the example core_pattern shown above systemd-coredump is spawned as a
> usermode helper. There's various conceptual consequences of this
> (non-exhaustive list):
>
> - systemd-coredump is spawned with file descriptor number 0 (stdin)
> connected to the read-end of the pipe. All other file descriptors are
> closed. That specifically includes 1 (stdout) and 2 (stderr). This has
> already caused bugs because userspace assumed that this cannot happen
> (Whether or not this is a sane assumption is irrelevant.).
>
> - systemd-coredump will be spawned as a child of system_unbound_wq. So
> it is not a child of any userspace process and specifically not a
> child of PID 1. It cannot be waited upon and is in a weird hybrid
> upcall which are difficult for userspace to control correctly.
>
> - systemd-coredump is spawned with full kernel privileges. This
> necessitates all kinds of weird privilege dropping excercises in
> userspace to make this safe.
>
> - A new usermode helper has to be spawned for each crashing process.
>
> This series adds a new mode:
>
> (3) Dumping into an abstract AF_UNIX socket.
>
> Userspace can set /proc/sys/kernel/core_pattern to:
>
> @address SO_COOKIE
>
> The "@" at the beginning indicates to the kernel that the abstract
> AF_UNIX coredump socket will be used to process coredumps. The address
> is given by @address and must be followed by the socket cookie of the
> coredump listening socket.
>
> The socket cookie is used to verify the socket connection. If the
> coredump server restarts or crashes and someone recycles the socket
> address the kernel will detect that the address has been recycled as the
> socket cookie will have necessarily changed and refuse to connect.
>
> The coredump socket is located in the initial network namespace. When a
> task coredumps it opens a client socket in the initial network namespace
> and connects to the coredump socket.
>
> - The coredump server uses SO_PEERPIDFD to get a stable handle on the
> connected crashing task. The retrieved pidfd will provide a stable
> reference even if the crashing task gets SIGKILLed while generating
> the coredump.
>
> - By setting core_pipe_limit non-zero userspace can guarantee that the
> crashing task cannot be reaped behind it's back and thus process all
> necessary information in /proc/<pid>. The SO_PEERPIDFD can be used to
> detect whether /proc/<pid> still refers to the same process.
>
> The core_pipe_limit isn't used to rate-limit connections to the
> socket. This can simply be done via AF_UNIX sockets directly.
>
> - The pidfd for the crashing task will grow new information how the task
> coredumps.
>
> - The coredump server should mark itself as non-dumpable.
>
> - A container coredump server in a separate network namespace can simply
> bind to another well-know address and systemd-coredump fowards
> coredumps to the container.
>
> - Coredumps could in the future also be handled via per-user/session
> coredump servers that run only with that users privileges.
>
> The coredump server listens on the coredump socket and accepts a
> new coredump connection. It then retrieves SO_PEERPIDFD for the
> client, inspects uid/gid and hands the accepted client to the users
> own coredump handler which runs with the users privileges only
> (It must of coure pay close attention to not forward crashing suid
> binaries.).
>
> The new coredump socket will allow userspace to not have to rely on
> usermode helpers for processing coredumps and provides a safer way to
> handle them instead of relying on super privileged coredumping helpers
> that have and continue to cause significant CVEs.
>
> This will also be significantly more lightweight since no fork()+exec()
> for the usermodehelper is required for each crashing process. The
> coredump server in userspace can e.g., just keep a worker pool.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-12 10:58 ` Luca Boccassi
@ 2025-05-13 0:17 ` Kuniyuki Iwashima
2025-05-13 1:09 ` Luca Boccassi
0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-05-13 0:17 UTC (permalink / raw)
To: bluca
Cc: alexander, brauner, daan.j.demeyer, daniel, davem, david,
edumazet, horms, jack, jannh, kuba, kuniyu, lennart,
linux-fsdevel, linux-kernel, linux-security-module, me, netdev,
oleg, pabeni, viro, zbyszek
From: Luca Boccassi <bluca@debian.org>
Date: Mon, 12 May 2025 11:58:54 +0100
> On Mon, 12 May 2025 at 09:56, Christian Brauner <brauner@kernel.org> wrote:
> >
> > Coredumping currently supports two modes:
> >
> > (1) Dumping directly into a file somewhere on the filesystem.
> > (2) Dumping into a pipe connected to a usermode helper process
> > spawned as a child of the system_unbound_wq or kthreadd.
> >
> > For simplicity I'm mostly ignoring (1). There's probably still some
> > users of (1) out there but processing coredumps in this way can be
> > considered adventurous especially in the face of set*id binaries.
> >
> > The most common option should be (2) by now. It works by allowing
> > userspace to put a string into /proc/sys/kernel/core_pattern like:
> >
> > |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
> >
> > The "|" at the beginning indicates to the kernel that a pipe must be
> > used. The path following the pipe indicator is a path to a binary that
> > will be spawned as a usermode helper process. Any additional parameters
> > pass information about the task that is generating the coredump to the
> > binary that processes the coredump.
> >
> > In the example core_pattern shown above systemd-coredump is spawned as a
> > usermode helper. There's various conceptual consequences of this
> > (non-exhaustive list):
> >
> > - systemd-coredump is spawned with file descriptor number 0 (stdin)
> > connected to the read-end of the pipe. All other file descriptors are
> > closed. That specifically includes 1 (stdout) and 2 (stderr). This has
> > already caused bugs because userspace assumed that this cannot happen
> > (Whether or not this is a sane assumption is irrelevant.).
> >
> > - systemd-coredump will be spawned as a child of system_unbound_wq. So
> > it is not a child of any userspace process and specifically not a
> > child of PID 1. It cannot be waited upon and is in a weird hybrid
> > upcall which are difficult for userspace to control correctly.
> >
> > - systemd-coredump is spawned with full kernel privileges. This
> > necessitates all kinds of weird privilege dropping excercises in
> > userspace to make this safe.
> >
> > - A new usermode helper has to be spawned for each crashing process.
> >
> > This series adds a new mode:
> >
> > (3) Dumping into an abstract AF_UNIX socket.
> >
> > Userspace can set /proc/sys/kernel/core_pattern to:
> >
> > @address SO_COOKIE
> >
> > The "@" at the beginning indicates to the kernel that the abstract
> > AF_UNIX coredump socket will be used to process coredumps. The address
> > is given by @address and must be followed by the socket cookie of the
> > coredump listening socket.
> >
> > The socket cookie is used to verify the socket connection. If the
> > coredump server restarts or crashes and someone recycles the socket
> > address the kernel will detect that the address has been recycled as the
> > socket cookie will have necessarily changed and refuse to connect.
>
> This dynamic/cookie prefix makes it impossible to use this with socket
> activation units. The way systemd-coredump works is that every
> instance is an independent templated unit, spawned when there's a
> connection to the private socket. If the path was fixed, we could just
> reuse the same mechanism, it would fit very nicely with minimal
> changes.
Note this version does not use prefix. Now it requires users to
just pass the socket cookie via core_pattern so that the kernel
can verify the peer.
>
> But because you need a "server" to be permanently running, this means
> socket-based activation can no longer work, and systemd-coredump must
> switch to a persistently-running mode.
The only thing for systemd to do is assign a cookie after socket creation.
As long as systemd hold the file descriptor of the socket, you don't need
a dedicated "server" running permanently, and the fd can be passed around
to a spawned/activated process.
> This is a severe degradation of
> functionality, will continuously waste CPU/memory resources for no
> good reasons, and makes the whole thing more fragile and complex, as
> if there are any issues with this server, you start losing core files.
> And honestly I don't really see the point? Setting the pattern is a
> privileged operation anyway. systemd manages the socket with a socket
> unit and again that's privileged already.
>
> Could we drop this cookie prefix and go back to the previous version
> (v5), please? Or if there is some specific non-systemd use case in
> mind that I am not aware of, have both options, so that we can use the
> simpler and more straightforward one with systemd-coredump.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-13 0:17 ` Kuniyuki Iwashima
@ 2025-05-13 1:09 ` Luca Boccassi
2025-05-13 2:14 ` Kuniyuki Iwashima
0 siblings, 1 reply; 17+ messages in thread
From: Luca Boccassi @ 2025-05-13 1:09 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: alexander, brauner, daan.j.demeyer, daniel, davem, david,
edumazet, horms, jack, jannh, kuba, lennart, linux-fsdevel,
linux-kernel, linux-security-module, me, netdev, oleg, pabeni,
viro, zbyszek
On Tue, 13 May 2025 at 01:18, Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> From: Luca Boccassi <bluca@debian.org>
> Date: Mon, 12 May 2025 11:58:54 +0100
> > On Mon, 12 May 2025 at 09:56, Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > Coredumping currently supports two modes:
> > >
> > > (1) Dumping directly into a file somewhere on the filesystem.
> > > (2) Dumping into a pipe connected to a usermode helper process
> > > spawned as a child of the system_unbound_wq or kthreadd.
> > >
> > > For simplicity I'm mostly ignoring (1). There's probably still some
> > > users of (1) out there but processing coredumps in this way can be
> > > considered adventurous especially in the face of set*id binaries.
> > >
> > > The most common option should be (2) by now. It works by allowing
> > > userspace to put a string into /proc/sys/kernel/core_pattern like:
> > >
> > > |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
> > >
> > > The "|" at the beginning indicates to the kernel that a pipe must be
> > > used. The path following the pipe indicator is a path to a binary that
> > > will be spawned as a usermode helper process. Any additional parameters
> > > pass information about the task that is generating the coredump to the
> > > binary that processes the coredump.
> > >
> > > In the example core_pattern shown above systemd-coredump is spawned as a
> > > usermode helper. There's various conceptual consequences of this
> > > (non-exhaustive list):
> > >
> > > - systemd-coredump is spawned with file descriptor number 0 (stdin)
> > > connected to the read-end of the pipe. All other file descriptors are
> > > closed. That specifically includes 1 (stdout) and 2 (stderr). This has
> > > already caused bugs because userspace assumed that this cannot happen
> > > (Whether or not this is a sane assumption is irrelevant.).
> > >
> > > - systemd-coredump will be spawned as a child of system_unbound_wq. So
> > > it is not a child of any userspace process and specifically not a
> > > child of PID 1. It cannot be waited upon and is in a weird hybrid
> > > upcall which are difficult for userspace to control correctly.
> > >
> > > - systemd-coredump is spawned with full kernel privileges. This
> > > necessitates all kinds of weird privilege dropping excercises in
> > > userspace to make this safe.
> > >
> > > - A new usermode helper has to be spawned for each crashing process.
> > >
> > > This series adds a new mode:
> > >
> > > (3) Dumping into an abstract AF_UNIX socket.
> > >
> > > Userspace can set /proc/sys/kernel/core_pattern to:
> > >
> > > @address SO_COOKIE
> > >
> > > The "@" at the beginning indicates to the kernel that the abstract
> > > AF_UNIX coredump socket will be used to process coredumps. The address
> > > is given by @address and must be followed by the socket cookie of the
> > > coredump listening socket.
> > >
> > > The socket cookie is used to verify the socket connection. If the
> > > coredump server restarts or crashes and someone recycles the socket
> > > address the kernel will detect that the address has been recycled as the
> > > socket cookie will have necessarily changed and refuse to connect.
> >
> > This dynamic/cookie prefix makes it impossible to use this with socket
> > activation units. The way systemd-coredump works is that every
> > instance is an independent templated unit, spawned when there's a
> > connection to the private socket. If the path was fixed, we could just
> > reuse the same mechanism, it would fit very nicely with minimal
> > changes.
>
> Note this version does not use prefix. Now it requires users to
> just pass the socket cookie via core_pattern so that the kernel
> can verify the peer.
Exactly - this means the pattern cannot be static in a sysctl.d early
on boot anymore, and has to be set dynamically by <something>. This is
a severe degradation over the status quo.
> > But because you need a "server" to be permanently running, this means
> > socket-based activation can no longer work, and systemd-coredump must
> > switch to a persistently-running mode.
>
> The only thing for systemd to do is assign a cookie after socket creation.
>
> As long as systemd hold the file descriptor of the socket, you don't need
> a dedicated "server" running permanently, and the fd can be passed around
> to a spawned/activated process.
There is no such facility, a socket is just a socket and there's no
infrastructure to randomly extract random information from one and
write it to some other random file in procfs, and I don't see why we
should add some super-special-case just for this, it sounds really
messy.
Also sockets can be and in fact are routinely restarted (eg: on
package upgrades), which would invalidate this whole scheme, and
result in a very racy setup. When packages are upgraded it's one of
the most complex workflows in modern distros, and it's very likely
that things start crashing exactly at that point, and with this
workflow it would mean we'll lose core files due to the race between
restarting the socket unit and <something> updating the pattern
accordingly.
Also we very much want to be able to spawn as many core handlers at
the same time as needed, which I don't see how can work with a cookie
that has to be unique per socket.
Sorry, but this particular approach seems completely unnecessary and
over-complicated, and doesn't seem to fit very well with how modern
userspace is set up today, and I don't see what actual problem it
would solve? If you need it for some particular use case that's
absolutely fine, but please add it later as another optional mode, so
that we don't have to degrade our use cases for it. That way everyone
gets what they want, and everyone's happy.
v5 was super nice and had everything we needed to massively improve
the status quo, with easy and straightforward changes and no real
drawbacks, so it would be really great if we could just go back to
that version, please. Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-13 1:09 ` Luca Boccassi
@ 2025-05-13 2:14 ` Kuniyuki Iwashima
2025-05-13 8:56 ` Lennart Poettering
0 siblings, 1 reply; 17+ messages in thread
From: Kuniyuki Iwashima @ 2025-05-13 2:14 UTC (permalink / raw)
To: bluca
Cc: alexander, brauner, daan.j.demeyer, daniel, davem, david,
edumazet, horms, jack, jannh, kuba, kuniyu, lennart,
linux-fsdevel, linux-kernel, linux-security-module, me, netdev,
oleg, pabeni, viro, zbyszek
From: Luca Boccassi <bluca@debian.org>
Date: Tue, 13 May 2025 02:09:24 +0100
> On Tue, 13 May 2025 at 01:18, Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> >
> > From: Luca Boccassi <bluca@debian.org>
> > Date: Mon, 12 May 2025 11:58:54 +0100
> > > On Mon, 12 May 2025 at 09:56, Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > Coredumping currently supports two modes:
> > > >
> > > > (1) Dumping directly into a file somewhere on the filesystem.
> > > > (2) Dumping into a pipe connected to a usermode helper process
> > > > spawned as a child of the system_unbound_wq or kthreadd.
> > > >
> > > > For simplicity I'm mostly ignoring (1). There's probably still some
> > > > users of (1) out there but processing coredumps in this way can be
> > > > considered adventurous especially in the face of set*id binaries.
> > > >
> > > > The most common option should be (2) by now. It works by allowing
> > > > userspace to put a string into /proc/sys/kernel/core_pattern like:
> > > >
> > > > |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h
> > > >
> > > > The "|" at the beginning indicates to the kernel that a pipe must be
> > > > used. The path following the pipe indicator is a path to a binary that
> > > > will be spawned as a usermode helper process. Any additional parameters
> > > > pass information about the task that is generating the coredump to the
> > > > binary that processes the coredump.
> > > >
> > > > In the example core_pattern shown above systemd-coredump is spawned as a
> > > > usermode helper. There's various conceptual consequences of this
> > > > (non-exhaustive list):
> > > >
> > > > - systemd-coredump is spawned with file descriptor number 0 (stdin)
> > > > connected to the read-end of the pipe. All other file descriptors are
> > > > closed. That specifically includes 1 (stdout) and 2 (stderr). This has
> > > > already caused bugs because userspace assumed that this cannot happen
> > > > (Whether or not this is a sane assumption is irrelevant.).
> > > >
> > > > - systemd-coredump will be spawned as a child of system_unbound_wq. So
> > > > it is not a child of any userspace process and specifically not a
> > > > child of PID 1. It cannot be waited upon and is in a weird hybrid
> > > > upcall which are difficult for userspace to control correctly.
> > > >
> > > > - systemd-coredump is spawned with full kernel privileges. This
> > > > necessitates all kinds of weird privilege dropping excercises in
> > > > userspace to make this safe.
> > > >
> > > > - A new usermode helper has to be spawned for each crashing process.
> > > >
> > > > This series adds a new mode:
> > > >
> > > > (3) Dumping into an abstract AF_UNIX socket.
> > > >
> > > > Userspace can set /proc/sys/kernel/core_pattern to:
> > > >
> > > > @address SO_COOKIE
> > > >
> > > > The "@" at the beginning indicates to the kernel that the abstract
> > > > AF_UNIX coredump socket will be used to process coredumps. The address
> > > > is given by @address and must be followed by the socket cookie of the
> > > > coredump listening socket.
> > > >
> > > > The socket cookie is used to verify the socket connection. If the
> > > > coredump server restarts or crashes and someone recycles the socket
> > > > address the kernel will detect that the address has been recycled as the
> > > > socket cookie will have necessarily changed and refuse to connect.
> > >
> > > This dynamic/cookie prefix makes it impossible to use this with socket
> > > activation units. The way systemd-coredump works is that every
> > > instance is an independent templated unit, spawned when there's a
> > > connection to the private socket. If the path was fixed, we could just
> > > reuse the same mechanism, it would fit very nicely with minimal
> > > changes.
> >
> > Note this version does not use prefix. Now it requires users to
> > just pass the socket cookie via core_pattern so that the kernel
> > can verify the peer.
>
> Exactly - this means the pattern cannot be static in a sysctl.d early
> on boot anymore, and has to be set dynamically by <something>.
You missed the socket has to be created dynamically by <something>.
> This is
> a severe degradation over the status quo.
>
> > > But because you need a "server" to be permanently running, this means
> > > socket-based activation can no longer work, and systemd-coredump must
> > > switch to a persistently-running mode.
> >
> > The only thing for systemd to do is assign a cookie after socket creation.
> >
> > As long as systemd hold the file descriptor of the socket, you don't need
> > a dedicated "server" running permanently, and the fd can be passed around
> > to a spawned/activated process.
>
> There is no such facility, a socket is just a socket and there's no
> infrastructure to randomly extract random information from one and
> write it to some other random file in procfs,
As only one socket can be registered to core_pattern, the socket
must not be a random.
> and I don't see why we
> should add some super-special-case just for this,
Because this is a new special use case.
> it sounds really
> messy.
> Also sockets can be and in fact are routinely restarted (eg: on
> package upgrades), which would invalidate this whole scheme, and
> result in a very racy setup. When packages are upgraded it's one of
> the most complex workflows in modern distros, and it's very likely
> that things start crashing exactly at that point, and with this
> workflow it would mean we'll lose core files due to the race between
> restarting the socket unit and <something> updating the pattern
> accordingly.
Looks like you misunderstood the series.
As you need to specify the socket in core_pattern, there must be
only one socket that can receive core data, so the problem statement
is always true throughout the series.
kernel_connect() does not connect() to a random one out of sockets
that have the common prefix.
That's why the BPF was mentioned in the previous cover letter:
- Since unix_stream_connect() runs bpf programs during connect it's
possible to even redirect or multiplex coredumps to other sockets.
> Also we very much want to be able to spawn as many core handlers at
> the same time as needed, which I don't see how can work with a cookie
> that has to be unique per socket.
As said, you can just pass the fd of the coredump listener or a fd
accept()ed from the listener, depending on how you want to handle
this in userspace.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-13 2:14 ` Kuniyuki Iwashima
@ 2025-05-13 8:56 ` Lennart Poettering
2025-05-13 12:08 ` Christian Brauner
0 siblings, 1 reply; 17+ messages in thread
From: Lennart Poettering @ 2025-05-13 8:56 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: bluca, alexander, brauner, daan.j.demeyer, daniel, davem, david,
edumazet, horms, jack, jannh, kuba, linux-fsdevel, linux-kernel,
linux-security-module, me, netdev, oleg, pabeni, viro, zbyszek
On Mo, 12.05.25 19:14, Kuniyuki Iwashima (kuniyu@amazon.com) wrote:
> > > Note this version does not use prefix. Now it requires users to
> > > just pass the socket cookie via core_pattern so that the kernel
> > > can verify the peer.
> >
> > Exactly - this means the pattern cannot be static in a sysctl.d early
> > on boot anymore, and has to be set dynamically by <something>.
>
> You missed the socket has to be created dynamically by <something>.
systemd implements socket activation: the generic code in PID 1 can
bind a socket, and then generically forks off a process (or instances
of processes for connection-based sockets) once traffic is seen on
that socket. On a typical, current systemd system, PID 1 does this for
~40 sockets by default. The code to bind AF_UNIX or AF_INET/AF_INET6
sockets is entirely generic.
Currently, in the existing systemd codebase coredumping is implemented
via socket activation: the core_pattern handler binary quickly hands
off the coredump fds to an AF_UNIX socket bound that way, and the
service behind that does the heavy lifting. Our hope is that with
Christian's work we can make the kernel deliver the coredumps directly
to the socket PID1 generically binds, getting rid of one middle man.
By requiring userspace to echo the SO_COOKIE value into the
core_pattern sysctl in a special formatting, you define a bespoke
protocol: it's not just enough to bind a socket (for which the generic
code in PID1 is good enough), and to write a fixed
string into a sysctl (for which the generic code in the current
/etc/sysctl.d/ manager, i.e. systemd-sysctl, works fine). But you
suddenly are asking from userspace, that some specific tool runs at
early boot, extracts the socket cookie from PID1 somehow, and writes
that into sysctl. We'd have to come up with a new tool for that, we
can no longer use generic tools. And that's the part that Luca doesn't
like.
To a large degree I agree with Luca about this. I would much prefer
Christian's earlier proposal (i.e. to simply define some prefix of
AF_UNIX abstract namespace addresses as requiring privs to bind),
because that would enable us to do generic handling in userspace: the
existing socket binding logic in PID 1, and the existing sysctl.d
handling in the systemd suite would be good enough to set up
everything for the coredump handling.
That said, I'd take what we can get. If enforcing privs on some
abstract namespace socket address prefix is not acceptable, then we
can probably make the SO_COOKIE proposal work (Luca: we'd just hook
some small tool into ExecStartPost= of the .socket unit, and make PID1
pass the cookie in some env var or so to it; the tool would then just
echo that env var into the sysctl with the fixed prefix). In my eyes,
it's not ideal though: it would mean the sysctl data on every instance
of the system system image would necessarily deviate (because the
socket cookie is going to be different), which mgmt tools won't like
(as you cannot compare sysctl state anymore), and we'd have a weak
conflict of ownership: right now most sysctl settings are managed by
/etc/sysctl.d/, but the core_pattern suddenly wouldn't be
anymore. This will create conflicts because suddenly two components
write to the thing, and will start fighting.
Hence: I'd *much* prefer Christian's original approach as it does not
have these issues. But I'll take what I can get, we can make the
cookie thing work, but it's much uglier.
I am not sure I understand why enforcing privs on some abstract
namespace socke address prefix is such an unacceptable idea though.
Lennart
--
Lennart Poettering, Berlin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 4/9] coredump: add coredump socket
2025-05-13 8:56 ` Lennart Poettering
@ 2025-05-13 12:08 ` Christian Brauner
0 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2025-05-13 12:08 UTC (permalink / raw)
To: Lennart Poettering
Cc: Kuniyuki Iwashima, bluca, alexander, daan.j.demeyer, daniel,
davem, david, edumazet, horms, jack, jannh, kuba, linux-fsdevel,
linux-kernel, linux-security-module, me, netdev, oleg, pabeni,
viro, zbyszek
On Tue, May 13, 2025 at 10:56:03AM +0200, Lennart Poettering wrote:
> On Mo, 12.05.25 19:14, Kuniyuki Iwashima (kuniyu@amazon.com) wrote:
>
> > > > Note this version does not use prefix. Now it requires users to
> > > > just pass the socket cookie via core_pattern so that the kernel
> > > > can verify the peer.
> > >
> > > Exactly - this means the pattern cannot be static in a sysctl.d early
> > > on boot anymore, and has to be set dynamically by <something>.
> >
> > You missed the socket has to be created dynamically by <something>.
>
> systemd implements socket activation: the generic code in PID 1 can
> bind a socket, and then generically forks off a process (or instances
> of processes for connection-based sockets) once traffic is seen on
> that socket. On a typical, current systemd system, PID 1 does this for
> ~40 sockets by default. The code to bind AF_UNIX or AF_INET/AF_INET6
> sockets is entirely generic.
>
> Currently, in the existing systemd codebase coredumping is implemented
> via socket activation: the core_pattern handler binary quickly hands
> off the coredump fds to an AF_UNIX socket bound that way, and the
> service behind that does the heavy lifting. Our hope is that with
> Christian's work we can make the kernel deliver the coredumps directly
> to the socket PID1 generically binds, getting rid of one middle man.
>
> By requiring userspace to echo the SO_COOKIE value into the
> core_pattern sysctl in a special formatting, you define a bespoke
> protocol: it's not just enough to bind a socket (for which the generic
> code in PID1 is good enough), and to write a fixed
> string into a sysctl (for which the generic code in the current
> /etc/sysctl.d/ manager, i.e. systemd-sysctl, works fine). But you
> suddenly are asking from userspace, that some specific tool runs at
> early boot, extracts the socket cookie from PID1 somehow, and writes
> that into sysctl. We'd have to come up with a new tool for that, we
> can no longer use generic tools. And that's the part that Luca doesn't
> like.
>
> To a large degree I agree with Luca about this. I would much prefer
> Christian's earlier proposal (i.e. to simply define some prefix of
> AF_UNIX abstract namespace addresses as requiring privs to bind),
> because that would enable us to do generic handling in userspace: the
> existing socket binding logic in PID 1, and the existing sysctl.d
> handling in the systemd suite would be good enough to set up
> everything for the coredump handling.
>
> That said, I'd take what we can get. If enforcing privs on some
> abstract namespace socket address prefix is not acceptable, then we
> can probably make the SO_COOKIE proposal work (Luca: we'd just hook
> some small tool into ExecStartPost= of the .socket unit, and make PID1
> pass the cookie in some env var or so to it; the tool would then just
> echo that env var into the sysctl with the fixed prefix). In my eyes,
> it's not ideal though: it would mean the sysctl data on every instance
> of the system system image would necessarily deviate (because the
> socket cookie is going to be different), which mgmt tools won't like
> (as you cannot compare sysctl state anymore), and we'd have a weak
> conflict of ownership: right now most sysctl settings are managed by
> /etc/sysctl.d/, but the core_pattern suddenly wouldn't be
> anymore. This will create conflicts because suddenly two components
> write to the thing, and will start fighting.
>
> Hence: I'd *much* prefer Christian's original approach as it does not
> have these issues. But I'll take what I can get, we can make the
> cookie thing work, but it's much uglier.
>
> I am not sure I understand why enforcing privs on some abstract
> namespace socke address prefix is such an unacceptable idea though.
I prefer the prefix approach as well. It's clean, simple and is safe by
itself and elegant. And it fits into the generic socket activation and
system administration models. I mainly show-cased the cookie model as an
elaborate workaround. It can be done but it's ugly and more difficult to
use.
I do have one more idea how to solve this problem cleanly using regular
socket paths that hopefully pleases everyone.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-05-13 12:08 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
2025-05-12 8:55 ` [PATCH v6 1/9] coredump: massage format_corname() Christian Brauner
2025-05-12 8:55 ` [PATCH v6 2/9] coredump: massage do_coredump() Christian Brauner
2025-05-12 8:55 ` [PATCH v6 3/9] coredump: reflow dump helpers a little Christian Brauner
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
2025-05-12 10:58 ` Luca Boccassi
2025-05-13 0:17 ` Kuniyuki Iwashima
2025-05-13 1:09 ` Luca Boccassi
2025-05-13 2:14 ` Kuniyuki Iwashima
2025-05-13 8:56 ` Lennart Poettering
2025-05-13 12:08 ` Christian Brauner
2025-05-13 0:06 ` Kuniyuki Iwashima
2025-05-12 8:55 ` [PATCH v6 5/9] pidfs, coredump: add PIDFD_INFO_COREDUMP Christian Brauner
2025-05-12 8:55 ` [PATCH v6 6/9] coredump: show supported coredump modes Christian Brauner
2025-05-12 8:55 ` [PATCH v6 7/9] coredump: validate socket name as it is written Christian Brauner
2025-05-12 8:55 ` [PATCH v6 8/9] selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure Christian Brauner
2025-05-12 8:55 ` [PATCH v6 9/9] selftests/coredump: add tests for AF_UNIX coredumps Christian Brauner
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).