[PATCH v2 0/7] coredump & signals: an impossible affair

[PATCH v2 0/7] coredump & signals: an impossible affair

[Christian Brauner]

Hey,

I asked Chris (Mason) to look at some of the coredump code with kres and
he found a couple of things. Some of them I had already found and fixed
but there's some good stuff in here.

Other parts should be more straightforward (hopefully...).

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- Add fixes for retarget_shared_signal().
- Expand fixes for signal_pending().
- Link to v1: https://patch.msgid.link/20260915-work-coredump-fixes-v1-0-f354ca41780c@kernel.org

---
Christian Brauner (7):
      fork: refuse new threads while a coredump or an exec is in progress
      coredump: hold RCU while releasing parked threads
      signal: only SIGKILL and the freezers interrupt a coredumping task
      coredump: parse a snapshot of core_pattern
      io-wq: order the exit bit against worker creation task work
      signal: don't retarget shared signals in a dying thread group
      selftests/coredump: test shared signal retargeting during a dump

 fs/coredump.c                                      |  72 ++++---
 include/linux/sched/signal.h                       |  19 +-
 io_uring/io-wq.c                                   |   2 +
 kernel/fork.c                                      |   6 +-
 kernel/signal.c                                    |  12 ++
 tools/testing/selftests/coredump/.gitignore        |   1 +
 tools/testing/selftests/coredump/Makefile          |   4 +-
 .../selftests/coredump/coredump_signal_test.c      | 238 +++++++++++++++++++++
 8 files changed, 318 insertions(+), 36 deletions(-)
---
base-commit: b813a7ec026ca59a444ba0a36c2e17ac823eeed0
change-id: 20260915-work-coredump-fixes-edf98c80fe78

[PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

Say a SQPOLL thread is a member of a thread-group that coredumps.
The coredump code uses zap_process() and sends SIGKILL. The SQPOLL
thread uses io_sqd_handle_event() and calls get_signal(). It removes
SIGKILL from the pending set and returns. The SQPOLL thread breaks out
of the loop and drains its own task work.

Any pending io_req_task_submit() with REQ_F_FORCE_ASYNC creates a new
worker when no other worker is free. So it ends up calling
create_io_thread() from a thread whose fatal signal is gone. That means
copy_process() allows the creation. It's also possible for an exiting
io-wq worker to push new work onto the SQPOLL thread.

zap_threads() counts the number of coredumping threads. The coredump
client waits in coredump_wait_inactive() until all threads in the
thread-group are parked. The new thread exits right away because the
workqueue is going down. It inherited PF_SIGNALED from its creator so
it links itself onto core_state->tasks in coredump_task_exit(). The
problem is that it then decrements "threads_remaining" even though
zap_process() never actually counted the new thread. So the count goes
to zero too early. So either the coredump misses the thread or it dumps
a thread that is still alive.

The same race exists during exec. de_thread() zaps the other threads
the same way and counts them in signal->notify_count, and a zapped user
worker that has already dequeued its SIGKILL can still clone while
de_thread() waits for that count. And once de_thread() has cleared
signal->group_exec_task the exec'ing thread itself runs task work in
io_uring_task_cancel() and a pending create_worker_cb() creates a
thread after the group was made single-threaded.

Close all of that in copy_process(). Refuse to create a thread while:

(1) SIGNAL_GROUP_EXIT is set (set together with core_state by zap_process()
(2) signal->group_exec_task is set
(3) while current is in execve

Conditions (1) and (2) are handled with siglock help which means
copy_process() and zap_process() synchronize on it. create_io_thread()
treats the failure as a failed task creation and doesn't retry.

Fixes: 3bfe6106693b ("io-wq: fork worker threads from original task")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 kernel/fork.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10be4a0ecb3f..6cd167a2b27b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2491,8 +2491,10 @@ __latent_entropy struct task_struct *copy_process(
 		goto bad_fork_core_free;
 	}
 
-	/* Let kill terminate clone/fork in the middle */
-	if (fatal_signal_pending(current)) {
+	/* Let kill or a group exit, exec or coredump abort clone/fork */
+	if (fatal_signal_pending(current) ||
+	    (current->signal->flags & SIGNAL_GROUP_EXIT) ||
+	    current->signal->group_exec_task || current->in_execve) {
 		retval = -EINTR;
 		goto bad_fork_core_free;
 	}

-- 
2.53.0

[PATCH v2 2/7] coredump: hold RCU while releasing parked threads

[Christian Brauner]

coredump_finish() releases the parked threads by clearing ->task in
their core_thread entry and calling wake_up_process() on each of them.
It does that without holding a reference on the task and without being
inside rcu.

But calling wake_up_process(task) without rcu here isn't safe. A parked
thread doesn't need that wakeup to leave. A spurious wakeup or a
preemption after the store is enough for the task to go away.

So if the coredump client is preempted between the store and
wake_up_process() the thread can exit and be freed in the meantime and
try_to_wake_up() takes pi_lock in freed memory.

Hold rcu across the loop.

Fixes: a94e2d408eae ("coredump: kill mm->core_done")
Cc: stable@vger.kernel.org
Acked-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/coredump.c b/fs/coredump.c
index 1fba3fed1a07..5b3f3a1090d7 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -598,6 +598,8 @@ static void coredump_finish(enum coredump_state state)
 	current->signal->core_state = NULL;
 	spin_unlock_irq(&current->sighand->siglock);
 
+	/* A released thread may exit and be freed before it is woken. */
+	guard(rcu)();
 	while ((curr = next) != NULL) {
 		next = curr->next;
 		task = curr->task;
@@ -606,6 +608,7 @@ static void coredump_finish(enum coredump_state state)
 		 * ->task == NULL before we read ->next.
 		 */
 		smp_mb();
+		/* Any wakeup now lets the thread exit, rcu keeps it alive. */
 		curr->task = NULL;
 		wake_up_process(task);
 	}

-- 
2.53.0

[PATCH v2 3/7] signal: only SIGKILL and the freezers interrupt a coredumping task

[Christian Brauner]

For quite a while now coredump has allowed either SIGKILL or freezing to
interrupt an ongoing coredump. Since we seem to like it complicated
"freezing" can mean a lot of things:

(1) Power Management induced freezing
(2) cgroup v1 freezer controller induced freezing
(3) cgroup v2 freezer controller induced freezing

So (1) and (2) are handled by checking freezing() but (3) isn't.
(3) uses cgroup_freeze_task() which sets JOBCTL_TRAP_FREEZE and calls
signal_wake_up() on every task in the cgroup. The trap isn't handled by
the coredump code.

Which means the current logic is inconsistent. Power and cgroup v1
freeze and abort the coredump. With cgroup v2 it depends on where the
core goes. A dump to a file completes and the cgroup v2 freeze has to
wait for it to finish. When dumping to a pipe or socket the dump is
interrupted at the next write because of trap's TIF_SIGPENDING. Which is
it? Let's be consistent and align (1)-(3): freezing interrupts an
ongoing coredump and doesn't make it wait until the dump is done.

So make signal_pending() report only SIGKILL, freezing() and the cgroup
v2 freezer trap for a task that has PF_DUMPCORE set. freezing() lives in
linux/freezer.h and is a static branch. So keep that check a separate
helper instead of in signal_pending() itself.

Make dump_interrupted() check for JOBCTL_TRAP_FREEZE as well so a cgroup
v2 freeze keeps aborting the dump like PM and cgroup v1 do.

Fixes: 403bad72b67d ("coredump: only SIGKILL should interrupt the coredumping task")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                | 11 ++++++-----
 include/linux/sched/signal.h | 19 +++++++++++++------
 kernel/signal.c              |  8 ++++++++
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 5b3f3a1090d7..e0ade16c2f99 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -617,12 +617,13 @@ static void coredump_finish(enum coredump_state state)
 static bool dump_interrupted(void)
 {
 	/*
-	 * SIGKILL or freezing() interrupt the coredumping. Perhaps we
-	 * can do try_to_freeze() and check __fatal_signal_pending(),
-	 * but then we need to teach dump_write() to restart and clear
-	 * TIF_SIGPENDING.
+	 * SIGKILL, freezing() or the cgroup v2 freezer trap interrupt the
+	 * coredumping. Perhaps we can do try_to_freeze() and check
+	 * __fatal_signal_pending(), but then we need to teach dump_write()
+	 * to restart and clear TIF_SIGPENDING.
 	 */
-	return fatal_signal_pending(current) || freezing(current);
+	return fatal_signal_pending(current) || freezing(current) ||
+	       (READ_ONCE(current->jobctl) & JOBCTL_TRAP_FREEZE);
 }
 
 static void wait_for_dump_helpers(struct file *file)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 70067ccfe2ba..12df33db1d9d 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -386,6 +386,13 @@ static inline int task_sigpending(struct task_struct *p)
 	return unlikely(test_tsk_thread_flag(p,TIF_SIGPENDING));
 }
 
+static inline int __fatal_signal_pending(struct task_struct *p)
+{
+	return unlikely(sigismember(&p->pending.signal, SIGKILL));
+}
+
+bool coredump_signal_pending(struct task_struct *p);
+
 static inline int signal_pending(struct task_struct *p)
 {
 	/*
@@ -395,12 +402,12 @@ static inline int signal_pending(struct task_struct *p)
 	 */
 	if (unlikely(test_tsk_thread_flag(p, TIF_NOTIFY_SIGNAL)))
 		return 1;
-	return task_sigpending(p);
-}
-
-static inline int __fatal_signal_pending(struct task_struct *p)
-{
-	return unlikely(sigismember(&p->pending.signal, SIGKILL));
+	if (!task_sigpending(p))
+		return 0;
+	/* A coredumping task only stops for SIGKILL or the freezer. */
+	if (unlikely(READ_ONCE(p->flags) & PF_DUMPCORE))
+		return coredump_signal_pending(p);
+	return 1;
 }
 
 static inline int fatal_signal_pending(struct task_struct *p)
diff --git a/kernel/signal.c b/kernel/signal.c
index ec30550951ec..d8bb1f168055 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -717,6 +717,14 @@ void signal_wake_up_state(struct task_struct *t, unsigned int state)
 		kick_process(t);
 }
 
+/* Only SIGKILL or a freezer interrupt a coredump, see dump_interrupted(). */
+bool coredump_signal_pending(struct task_struct *p)
+{
+	return __fatal_signal_pending(p) || freezing(p) ||
+	       (READ_ONCE(p->jobctl) & JOBCTL_TRAP_FREEZE);
+}
+EXPORT_SYMBOL(coredump_signal_pending);
+
 static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q);
 
 static void sigqueue_free_ignored(struct task_struct *tsk, struct sigqueue *q)

-- 
2.53.0

[PATCH v2 4/7] coredump: parse a snapshot of core_pattern

[Christian Brauner]

This is a long-standing problem that I discussed a while back with Jann.
I didn't care enough about it to really fix it and it's from the
before-fore-times.

coredump_parse() reads core_pattern directly while it can concurrently
be modified. So it reads the first byte, figures out what mode is
wanted, then allocates the number buffer and then consumes the rest of
the core_pattern string.

Say the sysctl handler updates the core_pattern array byte by byte
(idiotic but supported). So that can lead to all kinds of insane mixups.
Say you could transform the old "|/usr/bin/helper" and the new
"/tmp/core.%p" into a usermodehelper started as "tmp/core.<pid>".

So copy what proc_do_uts_string() does and let the handler run
proc_dostring() on a copy, validate the copy and make it visible beneath
a spinlock. Then coredump_parse() can take a snapshot under the same
spinlock and parse a stable copy.

From now on, rejected patterns are never visible and we can drop the
whole rollback logic. It has the same minor defect that utsname has,
namely that two writers can race on a non-zero offset. Irrelevant imho.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index e0ade16c2f99..0e5d6e84238a 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -86,6 +86,8 @@ static int core_uses_pid;
 static unsigned int core_pipe_limit;
 static unsigned int core_sort_vma;
 static char core_pattern[CORENAME_MAX_SIZE] = "core";
+/* Taken around every copy in and out of core_pattern. */
+static DEFINE_SPINLOCK(core_pattern_lock);
 static int core_name_size = CORENAME_MAX_SIZE;
 unsigned int core_file_note_size_limit = CORE_FILE_NOTE_SIZE_DEFAULT;
 static atomic_t core_pipe_count = ATOMIC_INIT(0);
@@ -241,11 +243,16 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
 			   size_t **argv, int *argc)
 {
 	const struct cred *cred = current_cred();
-	const char *pat_ptr = core_pattern;
+	char pattern[CORENAME_MAX_SIZE];
+	const char *pat_ptr = pattern;
 	bool was_space = false;
 	int pid_in_pattern = 0;
 	int err = 0;
 
+	/* The sysctl handler may be publishing a new pattern. */
+	scoped_guard(spinlock, &core_pattern_lock)
+		strscpy(pattern, core_pattern);
+
 	cprm->mask = COREDUMP_KERNEL;
 	if (core_pipe_limit)
 		cprm->mask |= COREDUMP_WAIT;
@@ -1688,11 +1695,11 @@ void validate_coredump_safety(void)
 	}
 }
 
-static inline bool check_coredump_socket(void)
+static inline bool check_coredump_socket(const char *pattern)
 {
 	const char *p;
 
-	if (core_pattern[0] != '@')
+	if (pattern[0] != '@')
 		return true;
 
 	/*
@@ -1704,16 +1711,16 @@ static inline bool check_coredump_socket(void)
 		return false;
 
 	/* Must be an absolute path... */
-	if (core_pattern[1] != '/') {
+	if (pattern[1] != '/') {
 		/* ... or the socket request protocol... */
-		if (core_pattern[1] != '@')
+		if (pattern[1] != '@')
 			return false;
 		/* ... and if so must be an absolute path. */
-		if (core_pattern[2] != '/')
+		if (pattern[2] != '/')
 			return false;
-		p = &core_pattern[2];
+		p = &pattern[2];
 	} else {
-		p = &core_pattern[1];
+		p = &pattern[1];
 	}
 
 	/* The path obviously cannot exceed UNIX_PATH_MAX. */
@@ -1721,7 +1728,7 @@ static inline bool check_coredump_socket(void)
 		return false;
 
 	/* Must not contain ".." in the path. */
-	if (name_contains_dotdot(core_pattern))
+	if (name_contains_dotdot(pattern))
 		return false;
 
 	return true;
@@ -1730,27 +1737,34 @@ static inline bool check_coredump_socket(void)
 static int proc_dostring_coredump(const struct ctl_table *table, int write,
 		  void *buffer, size_t *lenp, loff_t *ppos)
 {
+	char pattern[CORENAME_MAX_SIZE];
+	const struct ctl_table tmp = {
+		.procname	= table->procname,
+		.data		= pattern,
+		.maxlen		= sizeof(pattern),
+	};
+	bool changed = false;
 	int error;
-	ssize_t retval;
-	char old_core_pattern[CORENAME_MAX_SIZE];
-
-	if (!write)
-		return proc_dostring(table, write, buffer, lenp, ppos);
 
-	retval = strscpy(old_core_pattern, core_pattern, CORENAME_MAX_SIZE);
+	/* Work on a copy, proc_dostring() appends at *ppos. */
+	scoped_guard(spinlock, &core_pattern_lock)
+		strscpy(pattern, core_pattern);
 
-	error = proc_dostring(table, write, buffer, lenp, ppos);
-	if (error)
+	error = proc_dostring(&tmp, write, buffer, lenp, ppos);
+	if (error || !write)
 		return error;
 
-	if (!check_coredump_socket()) {
-		strscpy(core_pattern, old_core_pattern, retval + 1);
+	if (!check_coredump_socket(pattern))
 		return -EINVAL;
-	}
 
-	if (strncmp(old_core_pattern, core_pattern, CORENAME_MAX_SIZE))
+	/* Publish the validated pattern whole. */
+	scoped_guard(spinlock, &core_pattern_lock) {
+		changed = strncmp(pattern, core_pattern, CORENAME_MAX_SIZE);
+		strscpy(core_pattern, pattern);
+	}
+	if (changed)
 		validate_coredump_safety();
-	return error;
+	return 0;
 }
 
 static const unsigned int core_file_note_size_min = CORE_FILE_NOTE_SIZE_DEFAULT;

-- 
2.53.0

[PATCH v2 5/7] io-wq: order the exit bit against worker creation task work

[Christian Brauner]

io_wq_exit_workers() cancels task work for queued worker creation with
task_work_cancel_match(). That has a plain load of task->task_works.

io_queue_worker_create() adds the new entry and tests IO_WQ_BIT_EXIT to
cancel it if the workqueue is already on its way out.

That must be ordered. task_work_add() has a full barrier via cmpxchg().
But io_wq_exit_start() sets the bit with set_bit() which doesn't have
any memory ordering.

So afaict, on weakly ordered architectures the exiting task may load
task_works before the store of the bit is visible. The other side tests
the bit before the store is visible as well. So the entry remains queued
with worker_refs and the exiting task waits on worker_done indefinitely.

If the task still has rings then io_uring_del_tctx_node() provides the
barrier via test_and_set_bit() in io_wq_set_exit_on_idle(). When it
has closed all rings though that barrier is gone.

Add the barrier after the set_bit().

Fixes: 71a85387546e ("io-wq: check for wq exit after adding new worker task_work")
Cc: stable@vger.kernel.org
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 io_uring/io-wq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index 2ca223e47d41..2a980e86dd94 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -1324,6 +1324,8 @@ static bool io_task_work_match(struct callback_head *cb, void *data)
 void io_wq_exit_start(struct io_wq *wq)
 {
 	set_bit(IO_WQ_BIT_EXIT, &wq->state);
+	/* Pairs with task_work_add() in io_queue_worker_create(). */
+	smp_mb__after_atomic();
 }
 
 static void io_wq_cancel_tw_create(struct io_wq *wq)

-- 
2.53.0

[PATCH v2 6/7] signal: don't retarget shared signals in a dying thread group

[Christian Brauner]

If the calling thread has blocked a signal then
retarget_shared_pending() slingshots a shared pending signal to a
sibling thread. Today it only skips PF_EXITING threads but not a
coredumping thread.

But if SIGNAL_GROUP_EXIT is set no thread in the thread-group can
dequeue such signals anymore as get_signal() sends SIGKILL to every
thread and prepare_signal() drops any other signal.

exit_signals() also returns early on SIGNAL_GROUP_EXIT. But neither
sigprocmask() nor the mask restore in sigtimedwait() do.

Say a thread sleeps in sigtimedwait() and a sibling thread crashes. The
sleeping thread gets woken by the zap call. It restores its mask on the
way out and retargets a signal that was queued for the coredumping
process. That means TIF_SIGPENDING is set on the coredumping task which
zap_threads() had cleared. So a blocking write sees TIF_SIGPENDING and
truncates the dump.

Stop retargeting signals when SIGNAL_GROUP_EXIT is set and avoid
needlessly truncating coredumps.

Fixes: e6fa16ab9c1e ("signal: sigprocmask() should do retarget_shared_pending()")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 kernel/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index d8bb1f168055..c3bad983dd37 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3117,6 +3117,10 @@ static void retarget_shared_pending(struct task_struct *tsk, sigset_t *which)
 	sigset_t retarget;
 	struct task_struct *t;
 
+	/* Nobody dequeues them in a dying group, see get_signal(). */
+	if (tsk->signal->flags & SIGNAL_GROUP_EXIT)
+		return;
+
 	sigandsets(&retarget, &tsk->signal->shared_pending.signal, which);
 	if (sigisemptyset(&retarget))
 		return;

-- 
2.53.0

[PATCH v2 7/7] selftests/coredump: test shared signal retargeting during a dump

[Christian Brauner]

A shared signal that is still queued when a thread crashes gets
retargeted to the dumper when a zapped sibling restores its signal mask
on the way out of sigtimedwait(). That sets TIF_SIGPENDING on the dumper
and cuts the core short at the next full pipe or socket buffer. Cover
that:

- one sibling sleeps in sigtimedwait() for SIGUSR1

- a vfork() child queues SIGUSR1 for the main thread while it sleeps
  killably and then the SIGSEGV that is dequeued first

- the coredump server holds its read back so the dumper blocks on the
  full socket buffer

Check that the core covers every segment with and without the queued
signal.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/coredump/.gitignore        |   1 +
 tools/testing/selftests/coredump/Makefile          |   4 +-
 .../selftests/coredump/coredump_signal_test.c      | 238 +++++++++++++++++++++
 3 files changed, 242 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/coredump/.gitignore b/tools/testing/selftests/coredump/.gitignore
index 097f52db0be9..c198f2ca5872 100644
--- a/tools/testing/selftests/coredump/.gitignore
+++ b/tools/testing/selftests/coredump/.gitignore
@@ -2,3 +2,4 @@
 stackdump_test
 coredump_socket_test
 coredump_socket_protocol_test
+coredump_signal_test
diff --git a/tools/testing/selftests/coredump/Makefile b/tools/testing/selftests/coredump/Makefile
index dc8489d40618..bfb53f512d8e 100644
--- a/tools/testing/selftests/coredump/Makefile
+++ b/tools/testing/selftests/coredump/Makefile
@@ -4,7 +4,8 @@ CFLAGS += -Wall -O0 -g $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
 TEST_GEN_PROGS := stackdump_test \
 		  coredump_socket_test \
 		  coredump_socket_protocol_test \
-		  coredump_close_files_test
+		  coredump_close_files_test \
+		  coredump_signal_test
 TEST_FILES := stackdump
 
 include ../lib.mk
@@ -13,3 +14,4 @@ $(OUTPUT)/stackdump_test: coredump_test_helpers.c
 $(OUTPUT)/coredump_socket_test: coredump_test_helpers.c
 $(OUTPUT)/coredump_socket_protocol_test: coredump_test_helpers.c
 $(OUTPUT)/coredump_close_files_test: coredump_test_helpers.c
+$(OUTPUT)/coredump_signal_test: coredump_test_helpers.c
diff --git a/tools/testing/selftests/coredump/coredump_signal_test.c b/tools/testing/selftests/coredump/coredump_signal_test.c
new file mode 100644
index 000000000000..fdf48b144781
--- /dev/null
+++ b/tools/testing/selftests/coredump/coredump_signal_test.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <fcntl.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "coredump_test.h"
+
+/* Big enough to fill the socket buffer many times over. */
+#define CRASH_MAPPING_SIZE (32 * 1024 * 1024)
+
+FIXTURE_SETUP(coredump)
+{
+	FILE *file;
+	int ret;
+
+	self->pid_coredump_server = -ESRCH;
+	self->fd_tmpfs_detached = -1;
+	file = fopen("/proc/sys/kernel/core_pattern", "r");
+	ASSERT_NE(NULL, file);
+
+	ret = fread(self->original_core_pattern, 1, sizeof(self->original_core_pattern), file);
+	ASSERT_TRUE(ret || feof(file));
+	ASSERT_LT(ret, sizeof(self->original_core_pattern));
+
+	self->original_core_pattern[ret] = '\0';
+
+	ret = fclose(file);
+	ASSERT_EQ(0, ret);
+}
+
+FIXTURE_TEARDOWN(coredump)
+{
+	const char *reason;
+	FILE *file;
+	int ret, status;
+
+	if (self->pid_coredump_server > 0) {
+		kill(self->pid_coredump_server, SIGTERM);
+		waitpid(self->pid_coredump_server, &status, 0);
+	}
+	unlink("/tmp/coredump.file");
+	unlink("/tmp/coredump.socket");
+
+	file = fopen("/proc/sys/kernel/core_pattern", "w");
+	if (!file) {
+		reason = "Unable to open core_pattern";
+		goto fail;
+	}
+
+	ret = fprintf(file, "%s", self->original_core_pattern);
+	if (ret < 0) {
+		reason = "Unable to write to core_pattern";
+		goto fail;
+	}
+
+	ret = fclose(file);
+	if (ret) {
+		reason = "Unable to close core_pattern";
+		goto fail;
+	}
+
+	return;
+fail:
+	/* This should never happen */
+	fprintf(stderr, "Failed to cleanup coredump test: %s\n", reason);
+}
+
+static volatile int waiter_ready;
+
+static void usr1_handler(int sig)
+{
+}
+
+/* Sleeps in sigtimedwait() with SIGUSR1 unblocked only inside the kernel. */
+static void *sigwaiter(void *arg)
+{
+	sigset_t set;
+	siginfo_t info;
+
+	sigemptyset(&set);
+	sigaddset(&set, SIGUSR1);
+	__atomic_store_n(&waiter_ready, 1, __ATOMIC_RELEASE);
+	for (;;)
+		sigtimedwait(&set, &info, NULL);
+	return NULL;
+}
+
+/*
+ * Crash with a shared SIGUSR1 still queued for this thread. The vfork()
+ * child queues it while we sleep killably and then the SIGSEGV that is
+ * dequeued first. The zap wakes the sibling out of sigtimedwait() and its
+ * mask restore retargets SIGUSR1 to the dumper.
+ */
+static void crashing_child_retarget(bool queue_shared)
+{
+	sigset_t all, old;
+	pthread_t thread;
+	pid_t pid, tid;
+	char *p;
+
+	p = mmap(NULL, CRASH_MAPPING_SIZE, PROT_READ | PROT_WRITE,
+		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (p == MAP_FAILED)
+		_exit(EXIT_FAILURE);
+	memset(p, 0x5a, CRASH_MAPPING_SIZE);
+
+	signal(SIGUSR1, usr1_handler);
+	sigfillset(&all);
+	pthread_sigmask(SIG_BLOCK, &all, &old);
+	/* One waiter only, retarget stops at the first thread not blocking it. */
+	if (pthread_create(&thread, NULL, sigwaiter, NULL))
+		_exit(EXIT_FAILURE);
+	pthread_sigmask(SIG_SETMASK, &old, NULL);
+	while (!__atomic_load_n(&waiter_ready, __ATOMIC_ACQUIRE))
+		usleep(1000);
+	usleep(50 * 1000);
+
+	pid = getpid();
+	tid = syscall(SYS_gettid);
+	if (vfork() == 0) {
+		if (queue_shared)
+			syscall(SYS_kill, pid, SIGUSR1);
+		syscall(SYS_tgkill, pid, tid, SIGSEGV);
+		syscall(SYS_exit, 0);
+	}
+
+	/* Not reached, the pending SIGSEGV dumps core. */
+	for (;;)
+		pause();
+}
+
+/*
+ * Dump the crashing child into /tmp/coredump.file through a server that
+ * holds the read back so the dumper blocks on the full socket buffer.
+ */
+static void run_coredump(struct __test_metadata *const _metadata,
+			 FIXTURE_DATA(coredump) *self, bool queue_shared)
+{
+	pid_t pid, pid_coredump_server;
+	int ipc_sockets[2];
+	int status;
+	char c;
+
+	unlink("/tmp/coredump.file");
+	unlink("/tmp/coredump.socket");
+	ASSERT_TRUE(set_core_pattern("@/tmp/coredump.socket"));
+	ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets), 0);
+
+	pid_coredump_server = fork();
+	ASSERT_GE(pid_coredump_server, 0);
+	if (pid_coredump_server == 0) {
+		int fd_server = -1, fd_coredump = -1, fd_core_file = -1;
+		int exit_code = EXIT_FAILURE;
+
+		close(ipc_sockets[0]);
+
+		fd_server = create_and_listen_unix_socket("/tmp/coredump.socket");
+		if (fd_server < 0)
+			goto out;
+
+		if (write_nointr(ipc_sockets[1], "1", 1) < 0)
+			goto out;
+		close(ipc_sockets[1]);
+
+		fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+		if (fd_coredump < 0) {
+			fprintf(stderr, "%s: accept4 failed: %m\n", __func__);
+			goto out;
+		}
+
+		/* Let the dumper run into the full socket buffer first. */
+		sleep(1);
+
+		fd_core_file = creat("/tmp/coredump.file", 0644);
+		if (fd_core_file < 0) {
+			fprintf(stderr, "%s: creat failed: %m\n", __func__);
+			goto out;
+		}
+
+		if (recv_coredump_bytes(fd_coredump, fd_core_file) < 0)
+			goto out;
+
+		exit_code = EXIT_SUCCESS;
+out:
+		if (fd_core_file >= 0)
+			close(fd_core_file);
+		if (fd_coredump >= 0)
+			close(fd_coredump);
+		if (fd_server >= 0)
+			close(fd_server);
+		_exit(exit_code);
+	}
+	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_retarget(queue_shared);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_EQ(WTERMSIG(status), SIGSEGV);
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+}
+
+static void check_coredump_complete(struct __test_metadata *const _metadata)
+{
+	int fd;
+
+	fd = open("/tmp/coredump.file", O_RDONLY | O_CLOEXEC);
+	ASSERT_GE(fd, 0);
+	ASSERT_TRUE(check_coredump_extent(fd));
+	close(fd);
+}
+
+TEST_F(coredump, retarget_shared_pending)
+{
+	run_coredump(_metadata, self, false);
+	check_coredump_complete(_metadata);
+
+	run_coredump(_metadata, self, true);
+	check_coredump_complete(_metadata);
+}
+
+TEST_HARNESS_MAIN

-- 
2.53.0

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Bradley Morgan]

On 17 September 2026 10:17:28 BST, Christian Brauner <brauner@kernel.org>
wrote:
>Say a SQPOLL thread is a member of a thread-group that coredumps.
>The coredump code uses zap_process() and sends SIGKILL. The SQPOLL
>thread uses io_sqd_handle_event() and calls get_signal(). It removes
>SIGKILL from the pending set and returns. The SQPOLL thread breaks out
>of the loop and drains its own task work.
>
>Any pending io_req_task_submit() with REQ_F_FORCE_ASYNC creates a new
>worker when no other worker is free. So it ends up calling
>create_io_thread() from a thread whose fatal signal is gone. That means
>copy_process() allows the creation. It's also possible for an exiting
>io-wq worker to push new work onto the SQPOLL thread.
>
>zap_threads() counts the number of coredumping threads. The coredump
>client waits in coredump_wait_inactive() until all threads in the
>thread-group are parked. The new thread exits right away because the
>workqueue is going down. It inherited PF_SIGNALED from its creator so
>it links itself onto core_state->tasks in coredump_task_exit(). The
>problem is that it then decrements "threads_remaining" even though
>zap_process() never actually counted the new thread. So the count goes
>to zero too early. So either the coredump misses the thread or it dumps
>a thread that is still alive.
>
>The same race exists during exec. de_thread() zaps the other threads
>the same way and counts them in signal->notify_count, and a zapped user
>worker that has already dequeued its SIGKILL can still clone while
>de_thread() waits for that count. And once de_thread() has cleared
>signal->group_exec_task the exec'ing thread itself runs task work in
>io_uring_task_cancel() and a pending create_worker_cb() creates a
>thread after the group was made single-threaded.
>
>Close all of that in copy_process(). Refuse to create a thread while:
>
>(1) SIGNAL_GROUP_EXIT is set (set together with core_state by
>zap_process()
>(2) signal->group_exec_task is set
>(3) while current is in execve
>
>Conditions (1) and (2) are handled with siglock help which means
>copy_process() and zap_process() synchronize on it. create_io_thread()
>treats the failure as a failed task creation and doesn't retry.
>
>Fixes: 3bfe6106693b ("io-wq: fork worker threads from original task")
>Cc: stable@vger.kernel.org
>Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
>---
> kernel/fork.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/kernel/fork.c b/kernel/fork.c
>index 10be4a0ecb3f..6cd167a2b27b 100644
>--- a/kernel/fork.c
>+++ b/kernel/fork.c
>@@ -2491,8 +2491,10 @@ __latent_entropy struct task_struct *copy_process(
> 		goto bad_fork_core_free;
> 	}
> 
>-	/* Let kill terminate clone/fork in the middle */
>-	if (fatal_signal_pending(current)) {
>+	/* Let kill or a group exit, exec or coredump abort clone/fork */
>+	if (fatal_signal_pending(current) ||
>+	    (current->signal->flags & SIGNAL_GROUP_EXIT) ||
>+	   

Hi! This LGTM, thanks for the patch. 

Reviewed-by: Bradley Morgan <brads@mainlining.org>

 

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Oleg Nesterov]

On 09/17, Christian Brauner wrote:
>
> Say a SQPOLL thread is a member of a thread-group that coredumps.
> The coredump code uses zap_process() and sends SIGKILL. The SQPOLL
> thread uses io_sqd_handle_event() and calls get_signal(). It removes
> SIGKILL from the pending set and returns. The SQPOLL thread breaks out
> of the loop and drains its own task work.
>
> Any pending io_req_task_submit() with REQ_F_FORCE_ASYNC creates a new
> worker when no other worker is free. So it ends up calling
> create_io_thread() from a thread whose fatal signal is gone.

Oh.. Can we fix this in the io_uring/ code somehow? The very fact that
copy_process() can be called after get_signal() returns SIGKILL looks
very wrong to me. See below.

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2491,8 +2491,10 @@ __latent_entropy struct task_struct *copy_process(
>  		goto bad_fork_core_free;
>  	}
>
> -	/* Let kill terminate clone/fork in the middle */
> -	if (fatal_signal_pending(current)) {
> +	/* Let kill or a group exit, exec or coredump abort clone/fork */
> +	if (fatal_signal_pending(current) ||
> +	    (current->signal->flags & SIGNAL_GROUP_EXIT) ||
> +	    current->signal->group_exec_task || current->in_execve) {

Well, the comment doesn't explain why should we care about exec or coredump,
if we forget about the problem above fatal_signal_pending() must be true.

At least, can we move these additional checks into create_io_thread() ?
To not uglify copy_process()...

Hmm... get_signal() sets PF_SIGNALED before it checks PF_USER_WORKER, so
perhaps something like below can work?

And perhaps io_should_retry_thread() should check PF_SIGNALED too?

Oleg.

--- x/kernel/fork.c
+++ x/kernel/fork.c
@@ -2700,6 +2700,9 @@ struct task_struct *create_io_thread(int
 		.user_worker	= 1,
 	};
 
+	if (current->flags && PF_SIGNALED)
+		return -EINTR;
+
 	return copy_process(NULL, 0, node, &args);
 }
 

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Fri, Sep 18, 2026 at 12:40:45PM +0200, Oleg Nesterov wrote:
> On 09/17, Christian Brauner wrote:
> >
> > Say a SQPOLL thread is a member of a thread-group that coredumps.
> > The coredump code uses zap_process() and sends SIGKILL. The SQPOLL
> > thread uses io_sqd_handle_event() and calls get_signal(). It removes
> > SIGKILL from the pending set and returns. The SQPOLL thread breaks out
> > of the loop and drains its own task work.
> >
> > Any pending io_req_task_submit() with REQ_F_FORCE_ASYNC creates a new
> > worker when no other worker is free. So it ends up calling
> > create_io_thread() from a thread whose fatal signal is gone.
> 
> Oh.. Can we fix this in the io_uring/ code somehow? The very fact that
> copy_process() can be called after get_signal() returns SIGKILL looks
> very wrong to me. See below.

Yeah, I agree but the io_uring solution I came up with all where a bit
involved...

> 
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2491,8 +2491,10 @@ __latent_entropy struct task_struct *copy_process(
> >  		goto bad_fork_core_free;
> >  	}
> >
> > -	/* Let kill terminate clone/fork in the middle */
> > -	if (fatal_signal_pending(current)) {
> > +	/* Let kill or a group exit, exec or coredump abort clone/fork */
> > +	if (fatal_signal_pending(current) ||
> > +	    (current->signal->flags & SIGNAL_GROUP_EXIT) ||
> > +	    current->signal->group_exec_task || current->in_execve) {
> 
> Well, the comment doesn't explain why should we care about exec or coredump,
> if we forget about the problem above fatal_signal_pending() must be true.
> 
> At least, can we move these additional checks into create_io_thread() ?
> To not uglify copy_process()...
> 
> Hmm... get_signal() sets PF_SIGNALED before it checks PF_USER_WORKER, so
> perhaps something like below can work?
> 
> And perhaps io_should_retry_thread() should check PF_SIGNALED too?

Hm, I like this idea...
Let's see.

> 
> Oleg.
> 
> --- x/kernel/fork.c
> +++ x/kernel/fork.c
> @@ -2700,6 +2700,9 @@ struct task_struct *create_io_thread(int
>  		.user_worker	= 1,
>  	};
>  
> +	if (current->flags && PF_SIGNALED)
> +		return -EINTR;
> +
>  	return copy_process(NULL, 0, node, &args);
>  }
>  
> 

-- 

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Fri, Sep 18, 2026 at 02:24:07PM +0200, Christian Brauner wrote:
> On Fri, Sep 18, 2026 at 12:40:45PM +0200, Oleg Nesterov wrote:
> > On 09/17, Christian Brauner wrote:
> > >
> > > Say a SQPOLL thread is a member of a thread-group that coredumps.
> > > The coredump code uses zap_process() and sends SIGKILL. The SQPOLL
> > > thread uses io_sqd_handle_event() and calls get_signal(). It removes
> > > SIGKILL from the pending set and returns. The SQPOLL thread breaks out
> > > of the loop and drains its own task work.
> > >
> > > Any pending io_req_task_submit() with REQ_F_FORCE_ASYNC creates a new
> > > worker when no other worker is free. So it ends up calling
> > > create_io_thread() from a thread whose fatal signal is gone.
> > 
> > Oh.. Can we fix this in the io_uring/ code somehow? The very fact that
> > copy_process() can be called after get_signal() returns SIGKILL looks
> > very wrong to me. See below.
> 
> Yeah, I agree but the io_uring solution I came up with all where a bit
> involved...
> 
> > 
> > > --- a/kernel/fork.c
> > > +++ b/kernel/fork.c
> > > @@ -2491,8 +2491,10 @@ __latent_entropy struct task_struct *copy_process(
> > >  		goto bad_fork_core_free;
> > >  	}
> > >
> > > -	/* Let kill terminate clone/fork in the middle */
> > > -	if (fatal_signal_pending(current)) {
> > > +	/* Let kill or a group exit, exec or coredump abort clone/fork */
> > > +	if (fatal_signal_pending(current) ||
> > > +	    (current->signal->flags & SIGNAL_GROUP_EXIT) ||
> > > +	    current->signal->group_exec_task || current->in_execve) {
> > 
> > Well, the comment doesn't explain why should we care about exec or coredump,
> > if we forget about the problem above fatal_signal_pending() must be true.
> > 
> > At least, can we move these additional checks into create_io_thread() ?
> > To not uglify copy_process()...
> > 
> > Hmm... get_signal() sets PF_SIGNALED before it checks PF_USER_WORKER, so
> > perhaps something like below can work?
> > 
> > And perhaps io_should_retry_thread() should check PF_SIGNALED too?
> 
> Hm, I like this idea...
> Let's see.

I think this works but they also need to check ->in_execve as the
exec'ing thread is obviously never signaled:

@@ -2709,6 +2707,10 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
                .user_worker    = 1,
        };

+       /* A creator past its fatal signal or in execve gets no thread. */
+       if ((current->flags & PF_SIGNALED) || current->in_execve)
+               return ERR_PTR(-EINTR);
+
        return copy_process(NULL, 0, node, &args);
 }

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Oleg Nesterov]

On 09/18, Christian Brauner wrote:
>
> I think this works but they also need to check ->in_execve as the
> exec'ing thread is obviously never signaled:
>
> @@ -2709,6 +2707,10 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
>                 .user_worker    = 1,
>         };
>
> +       /* A creator past its fatal signal or in execve gets no thread. */
> +       if ((current->flags & PF_SIGNALED) || current->in_execve)
> +               return ERR_PTR(-EINTR);

Ah, I forgot to mention...

Can we shift io_uring_task_cancel() up, after setting bprm->point_of_no_return
but before de_thread() ?

I know nothing about io_uring, not sure this would be enough...

Oleg.

Re: [PATCH v2 6/7] signal: don't retarget shared signals in a dying thread group

[Oleg Nesterov]

On 09/17, Christian Brauner wrote:
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3117,6 +3117,10 @@ static void retarget_shared_pending(struct task_struct *tsk, sigset_t *which)
>  	sigset_t retarget;
>  	struct task_struct *t;
>
> +	/* Nobody dequeues them in a dying group, see get_signal(). */
> +	if (tsk->signal->flags & SIGNAL_GROUP_EXIT)
> +		return;

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Fri, Sep 18, 2026 at 03:41:42PM +0200, Oleg Nesterov wrote:
> On 09/18, Christian Brauner wrote:
> >
> > I think this works but they also need to check ->in_execve as the
> > exec'ing thread is obviously never signaled:
> >
> > @@ -2709,6 +2707,10 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
> >                 .user_worker    = 1,
> >         };
> >
> > +       /* A creator past its fatal signal or in execve gets no thread. */
> > +       if ((current->flags & PF_SIGNALED) || current->in_execve)
> > +               return ERR_PTR(-EINTR);
> 
> Ah, I forgot to mention...
> 
> Can we shift io_uring_task_cancel() up, after setting bprm->point_of_no_return
> but before de_thread() ?
> 
> I know nothing about io_uring, not sure this would be enough...

From my reading of this code it works.
The cancel runs task work and any create_worker_cb() queued before the
exec can still create an io-wq worker. If that happens before
de_thread(), that worker just becomes a sibling that de_thread() kills
and waits for. After the cancel nothing new can be queued.

Re: [PATCH v2 5/7] io-wq: order the exit bit against worker creation task work

[Oleg Nesterov]

On 09/17, Christian Brauner wrote:
>
> --- a/io_uring/io-wq.c
> +++ b/io_uring/io-wq.c
> @@ -1324,6 +1324,8 @@ static bool io_task_work_match(struct callback_head *cb, void *data)
>  void io_wq_exit_start(struct io_wq *wq)
>  {
>  	set_bit(IO_WQ_BIT_EXIT, &wq->state);
> +	/* Pairs with task_work_add() in io_queue_worker_create(). */
> +	smp_mb__after_atomic();
>  }

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Fri, Sep 18, 2026 at 04:29:41PM +0200, Christian Brauner wrote:
> On Fri, Sep 18, 2026 at 03:41:42PM +0200, Oleg Nesterov wrote:
> > On 09/18, Christian Brauner wrote:
> > >
> > > I think this works but they also need to check ->in_execve as the
> > > exec'ing thread is obviously never signaled:
> > >
> > > @@ -2709,6 +2707,10 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
> > >                 .user_worker    = 1,
> > >         };
> > >
> > > +       /* A creator past its fatal signal or in execve gets no thread. */
> > > +       if ((current->flags & PF_SIGNALED) || current->in_execve)
> > > +               return ERR_PTR(-EINTR);
> > 
> > Ah, I forgot to mention...
> > 
> > Can we shift io_uring_task_cancel() up, after setting bprm->point_of_no_return
> > but before de_thread() ?
> > 
> > I know nothing about io_uring, not sure this would be enough...
> 
> From my reading of this code it works.
> The cancel runs task work and any create_worker_cb() queued before the
> exec can still create an io-wq worker. If that happens before
> de_thread(), that worker just becomes a sibling that de_thread() kills
> and waits for. After the cancel nothing new can be queued.

Right, one more thing to think about with this...

zap_process() skips every thread that has PF_POSTCOREDUMP set.
do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
io_uring_files_cancel() right after that.

That runs task work, create_worker_cb() runs and creates a new io-wq
worker. If a thread started a coredump rgith before that worker gets
created from a thread with PF_POSTCOREDUMP set which zap_processes()
doesn't see. So two ways of fixing this:

(1) mask off PF_POSTCOREDUMP in copy_process() -> probably the wrong
    place
(2) key on PF_SIGNALED | PF_POSTCOREDUMP

I think (2) is probably correct:

diff --git a/kernel/fork.c b/kernel/fork.c
index a28fd3976cc0..94a652d933c8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2707,8 +2707,8 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
                .user_worker    = 1,
        };

-       /* A creator past its fatal signal gets no thread. */
-       if (current->flags & PF_SIGNALED)
+       /* A creator past its fatal signal or its coredump point gets no thread. */
+       if (current->flags & (PF_SIGNALED | PF_POSTCOREDUMP))
                return ERR_PTR(-EINTR);

        return copy_process(NULL, 0, node, &args);

Better ideas?

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Fri, Sep 18, 2026 at 05:00:50PM +0200, Christian Brauner wrote:
> On Fri, Sep 18, 2026 at 04:29:41PM +0200, Christian Brauner wrote:
> > On Fri, Sep 18, 2026 at 03:41:42PM +0200, Oleg Nesterov wrote:
> > > On 09/18, Christian Brauner wrote:
> > > >
> > > > I think this works but they also need to check ->in_execve as the
> > > > exec'ing thread is obviously never signaled:
> > > >
> > > > @@ -2709,6 +2707,10 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
> > > >                 .user_worker    = 1,
> > > >         };
> > > >
> > > > +       /* A creator past its fatal signal or in execve gets no thread. */
> > > > +       if ((current->flags & PF_SIGNALED) || current->in_execve)
> > > > +               return ERR_PTR(-EINTR);
> > > 
> > > Ah, I forgot to mention...
> > > 
> > > Can we shift io_uring_task_cancel() up, after setting bprm->point_of_no_return
> > > but before de_thread() ?
> > > 
> > > I know nothing about io_uring, not sure this would be enough...
> > 
> > From my reading of this code it works.
> > The cancel runs task work and any create_worker_cb() queued before the
> > exec can still create an io-wq worker. If that happens before
> > de_thread(), that worker just becomes a sibling that de_thread() kills
> > and waits for. After the cancel nothing new can be queued.
> 
> Right, one more thing to think about with this...
> 
> zap_process() skips every thread that has PF_POSTCOREDUMP set.
> do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
> io_uring_files_cancel() right after that.
> 
> That runs task work, create_worker_cb() runs and creates a new io-wq
> worker. If a thread started a coredump rgith before that worker gets
> created from a thread with PF_POSTCOREDUMP set which zap_processes()
> doesn't see. So two ways of fixing this:
> 
> (1) mask off PF_POSTCOREDUMP in copy_process() -> probably the wrong
>     place
> (2) key on PF_SIGNALED | PF_POSTCOREDUMP
> 
> I think (2) is probably correct:
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index a28fd3976cc0..94a652d933c8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2707,8 +2707,8 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
>                 .user_worker    = 1,
>         };
> 
> -       /* A creator past its fatal signal gets no thread. */
> -       if (current->flags & PF_SIGNALED)
> +       /* A creator past its fatal signal or its coredump point gets no thread. */
> +       if (current->flags & (PF_SIGNALED | PF_POSTCOREDUMP))
>                 return ERR_PTR(-EINTR);
> 
>         return copy_process(NULL, 0, node, &args);
> 
> Better ideas?

I guess we could move io_uring_files_cancel() before
synchronize_group_exit() but that feels sketchy.

Re: [PATCH v2 4/7] coredump: parse a snapshot of core_pattern

[Oleg Nesterov]

On 09/17, Christian Brauner wrote:

> +	/* Publish the validated pattern whole. */
> +	scoped_guard(spinlock, &core_pattern_lock) {
> +		changed = strncmp(pattern, core_pattern, CORENAME_MAX_SIZE);
> +		strscpy(core_pattern, pattern);

Perhaps
		if (changed)
			strscpy(core_pattern, pattern);

will look more consistent... but this is purely cosmetic.

Reviewed-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Oleg Nesterov]

On 09/18, Christian Brauner wrote:
>
> zap_process() skips every thread that has PF_POSTCOREDUMP set.
> do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
> io_uring_files_cancel() right after that.
>
> That runs task work, create_worker_cb() runs and creates a new io-wq
> worker. If a thread started a coredump rgith before that worker gets
> created from a thread with PF_POSTCOREDUMP set which zap_processes()
> doesn't see. So two ways of fixing this:

Christian, I got lost ;)

TBH, I don't understand why the new io worker is really bad in this case.

But lets forget about coredump/zap_process. With the change below the
io_uring_files_cancel() paths will no longer be able to create a new
worker, even if coredump is not in progress.

Is it correct?

Oleg.

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2707,8 +2707,8 @@ struct task_struct *create_io_thread(int (*fn)(void *), void *arg, int node)
>                 .user_worker    = 1,
>         };
>
> -       /* A creator past its fatal signal gets no thread. */
> -       if (current->flags & PF_SIGNALED)
> +       /* A creator past its fatal signal or its coredump point gets no thread. */
> +       if (current->flags & (PF_SIGNALED | PF_POSTCOREDUMP))
>                 return ERR_PTR(-EINTR);

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Christian Brauner]

On Mon, Sep 21, 2026 at 12:32:36PM +0200, Oleg Nesterov wrote:
> On 09/18, Christian Brauner wrote:
> >
> > zap_process() skips every thread that has PF_POSTCOREDUMP set.
> > do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
> > io_uring_files_cancel() right after that.
> >
> > That runs task work, create_worker_cb() runs and creates a new io-wq
> > worker. If a thread started a coredump rgith before that worker gets
> > created from a thread with PF_POSTCOREDUMP set which zap_processes()
> > doesn't see. So two ways of fixing this:
> 
> Christian, I got lost ;)
> 
> TBH, I don't understand why the new io worker is really bad in this case.

zap_processes() only counts threads it sends SIGKILL too. But every
thread that later finds ->core_state decrements that count. An io-wq
worker created from the exiting thread isn't counted and exits right
away because the io-wq exit bit is already set. So it decrements without
having been counted.

> But lets forget about coredump/zap_process. With the change below the
> io_uring_files_cancel() paths will no longer be able to create a new
> worker, even if coredump is not in progress.
> 
> Is it correct?

Yes.

Re: [PATCH v2 1/7] fork: refuse new threads while a coredump or an exec is in progress

[Oleg Nesterov]

On 09/21, Christian Brauner wrote:
>
> On Mon, Sep 21, 2026 at 12:32:36PM +0200, Oleg Nesterov wrote:
> > On 09/18, Christian Brauner wrote:
> > >
> > > zap_process() skips every thread that has PF_POSTCOREDUMP set.
> > > do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
> > > io_uring_files_cancel() right after that.
> > >
> > > That runs task work, create_worker_cb() runs and creates a new io-wq
> > > worker. If a thread started a coredump rgith before that worker gets
> > > created from a thread with PF_POSTCOREDUMP set which zap_processes()
> > > doesn't see. So two ways of fixing this:
> >
> > Christian, I got lost ;)
> >
> > TBH, I don't understand why the new io worker is really bad in this case.
>
> zap_processes() only counts threads it sends SIGKILL too. But every
> thread that later finds ->core_state decrements that count. An io-wq
> worker created from the exiting thread isn't counted and exits right
> away because the io-wq exit bit is already set. So it decrements without
> having been counted.

Aaah, I forgot that the new worker can exit too and notice
signal->core_state != NULL in synchronize_group_exit(). Thanks.

> > But lets forget about coredump/zap_process. With the change below the
> > io_uring_files_cancel() paths will no longer be able to create a new
> > worker, even if coredump is not in progress.
> >
> > Is it correct?
>
> Yes.

Great. Then I think your change is fine.

Oleg.