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

[PATCH 0/6] 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.

@Jens, please take a look whether the io_uring fix makes sense. That
kres thing added some patches but I found them all not very good and so
came up with other fixes mostly.

@Oleg, I'm unsure about the freezer and coredump interaction with cgroup
v2 and specifically fixing it in signal_pending(). Maybe you have a
clearer picture.

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

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (6):
      coredump: don't switch a dumper that has no files table
      fork: refuse new threads while a coredump is in progress
      coredump: hold RCU while releasing parked threads
      signal: only SIGKILL interrupts a coredumping task
      coredump: parse a snapshot of core_pattern
      io-wq: order the exit bit against worker creation task work

 fs/coredump.c                | 66 +++++++++++++++++++++++++++++---------------
 include/linux/sched/signal.h | 17 ++++++++----
 io_uring/io-wq.c             |  2 ++
 kernel/fork.c                |  4 +--
 4 files changed, 58 insertions(+), 31 deletions(-)
---
base-commit: b813a7ec026ca59a444ba0a36c2e17ac823eeed0
change-id: 20260915-work-coredump-fixes-edf98c80fe78

[PATCH 1/6] coredump: don't switch a dumper that has no files table

[Christian Brauner]

Tasks without an fdtable are skipped in coredump_close_files(). The
coredump client itself doesn't use the same check. Since
put_files_struct() doesn't tolerate NULL it will oops for such tasks
without an fdtable. The prime suspect for this behavior are vhost
workers. Skip the switch for a coredump client without a table.

Fixes: b2b36bcb13ea ("coredump: add COREDUMP_CLOSE_FILES")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 1fba3fed1a07..791a9268ed96 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -579,7 +579,11 @@ static bool coredump_close_files(struct core_state *core_state)
 	/* Use the dumper's real creds not the overridden ones. */
 	scoped_with_creds(current_real_cred()) {
 		io_uring_task_cancel();
-		switch_files_struct(current, files);
+		/* The dumper itself may be a vhost worker without a table. */
+		if (current->files)
+			switch_files_struct(current, files);
+		else
+			put_files_struct(files);
 	}
 
 	coredump_wait_inactive(core_state);

-- 
2.53.0

[PATCH 2/6] fork: refuse new threads while a coredump 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() untill all threads in the
thread-group are parked. The new thread that was created via SQPOLL
exits immediately since it got PF_SIGNALED from the parent. The problem
is that it sees signal->core_state, links itself onto core_state->tasks
and decrements "threads_remaining".

But 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.

Close that gap and refuse creating a thread while signal->core_state is
set. Both sides protect it via siglock so copy_process() and
zap_processes() are sure to see each otehr.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 10be4a0ecb3f..bd97fc7b885b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2491,8 +2491,8 @@ __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 coredump in progress terminate clone/fork */
+	if (fatal_signal_pending(current) || current->signal->core_state) {
 		retval = -EINTR;
 		goto bad_fork_core_free;
 	}

-- 
2.53.0

[PATCH 3/6] coredump: hold RCU while releasing parked threads

[Christian Brauner]

When coredump_finish() releases the threads the wakeup neither holds a
reference on the task nor is it inside rcu.

The threads don't necessarily need a wakeup to exit. If it gets
preempted or was woken for other reasons it sees ->task cleared and
exits. So only rcu keeps such a task_struct alive.

If the coredump client is preempted between the store and
wake_up_process() for longer then try_to_wake_up() takes pi_lock() in
freed memory.

Hold the rcu across the loop.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index 791a9268ed96..78ab6cb78be8 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -602,6 +602,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;

-- 
2.53.0

[PATCH 4/6] signal: only SIGKILL interrupts a coredumping task

[Christian Brauner]

The coredump client only accepts SIGKILL. I've massaged away
TIF_NOTIFY_SIGNAL in another patch series but it seems that
TIF_SIGPENDING also has some warts and causes truncated coredumps:

(1) cgroup v2 freezer isn't built on freezing. Instead,
    cgroup_freeze_task() sets JOBCTL_TRAP_FREEZE and calls
    signal_wake_up() on every task in the cgroup. That includes the
    coredump client. The coredump client isn't able to act on the trap.

    So a freeze that lands in while a coredump is written will block.
    Moving a coredumping client into a frozen cgroup has the same
    problem.

(2) retarget_shared_pending() doesn't take a coredump into account too.
    So if a sibling thread is in the middle of changing the signal mask
    or it exists with a pending signal that helpers points the signals
    to other threads.

    While it skips exiting threads it will target it at the coredump
    client as the coredump client isn't yet exiting.

So it's related to PF_NO_NOTIFY_SIGNAL which I have sitting in
kernel-7.4.signal. We should be able to fix it this time by making
signal_pending() report only SIGKILL for a task
that has PF_DUMPCORE set.

A cgroup v2 freeze now waits for the dump to finish. The PM and cgroup
v1 freezers keep aborting it through dump_interrupted().

Basically, PM should be able to interrupt the dump. cgroup v1 freezers
are legacy crap we don't care about and cgroup 2 should wait(?).

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

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 70067ccfe2ba..3a7ff3416e57 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -386,6 +386,11 @@ 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));
+}
+
 static inline int signal_pending(struct task_struct *p)
 {
 	/*
@@ -395,12 +400,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, see dump_interrupted(). */
+	if (unlikely(READ_ONCE(p->flags) & PF_DUMPCORE))
+		return __fatal_signal_pending(p);
+	return 1;
 }
 
 static inline int fatal_signal_pending(struct task_struct *p)

-- 
2.53.0

[PATCH 5/6] 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 syscal 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 pattern 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 78ab6cb78be8..20cd95786dbe 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;
@@ -1690,11 +1697,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;
 
 	/*
@@ -1706,16 +1713,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. */
@@ -1723,7 +1730,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;
@@ -1732,27 +1739,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 6/6] 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
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

Re: [PATCH 0/6] coredump & signals: an impossible affair

[Jens Axboe]

On 9/15/26 4:22 AM, Christian Brauner wrote:
> 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.
> 
> @Jens, please take a look whether the io_uring fix makes sense. That
> kres thing added some patches but I found them all not very good and so
> came up with other fixes mostly.

Both of them look fine to me, feel free to add:

Reviewed-by: Jens Axboe <axboe@kernel.dk>

if you want to carry them. Or let me know if you want me to grab them.

-- 
Jens Axboe

Re: [PATCH 2/6] fork: refuse new threads while a coredump is in progress

[Oleg Nesterov]

On 09/15, 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. That means
> copy_process() allows the creation.

Hmm, thats bad ;)

But then ...

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2491,8 +2491,8 @@ __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 coredump in progress terminate clone/fork */
> +	if (fatal_signal_pending(current) || current->signal->core_state) {
>  		retval = -EINTR;
>  		goto bad_fork_core_free;

this change is not enough? Don't we have a similar race with exec?

de_thread() kills all other threads and counts them too. Shouldn't it
also check signal->group_exec_task ?

Oleg.

Re: [PATCH 3/6] coredump: hold RCU while releasing parked threads

[Oleg Nesterov]

On 09/15, Christian Brauner wrote:
>
> When coredump_finish() releases the threads the wakeup neither holds a
> reference on the task nor is it inside rcu.
>
> The threads don't necessarily need a wakeup to exit. If it gets
> preempted or was woken for other reasons it sees ->task cleared and
> exits. So only rcu keeps such a task_struct alive.
>
> If the coredump client is preempted between the store and
> wake_up_process() for longer then try_to_wake_up() takes pi_lock() in
> freed memory.
>
> Hold the rcu across the loop.
>
> Fixes: a94e2d408eae ("coredump: kill mm->core_done")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Aah, thanks...

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


But to be honest the changelog looks a bit confusing to me...

	curr->task = NULL;
	wake_up_process(task);

So wake_up_process(task) is not safe, task can exit after a spurious
wakeup if it sees ->task == NULL.

And perhaps a short comment above "curr->task = NULL;" makes sense?
The reason for guard(rcu) is not obvious at all.

Oleg.

Re: [PATCH 2/6] fork: refuse new threads while a coredump is in progress

[Christian Brauner]

On Tue, Sep 15, 2026 at 05:02:43PM +0200, Oleg Nesterov wrote:
> On 09/15, 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. That means
> > copy_process() allows the creation.
> 
> Hmm, thats bad ;)
> 
> But then ...
> 
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2491,8 +2491,8 @@ __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 coredump in progress terminate clone/fork */
> > +	if (fatal_signal_pending(current) || current->signal->core_state) {
> >  		retval = -EINTR;
> >  		goto bad_fork_core_free;
> 
> this change is not enough? Don't we have a similar race with exec?

Yeah, we do...

> de_thread() kills all other threads and counts them too. Shouldn't it
> also check signal->group_exec_task ?

not just that. We need in_execve checked too because
io_uring_task_cancel() runs the exec'ing thread's task work after
de_thread() when group_exec_task is unset.

Re: [PATCH 3/6] coredump: hold RCU while releasing parked threads

[Christian Brauner]

On Tue, Sep 15, 2026 at 05:13:25PM +0200, Oleg Nesterov wrote:
> On 09/15, Christian Brauner wrote:
> >
> > When coredump_finish() releases the threads the wakeup neither holds a
> > reference on the task nor is it inside rcu.
> >
> > The threads don't necessarily need a wakeup to exit. If it gets
> > preempted or was woken for other reasons it sees ->task cleared and
> > exits. So only rcu keeps such a task_struct alive.
> >
> > If the coredump client is preempted between the store and
> > wake_up_process() for longer then try_to_wake_up() takes pi_lock() in
> > freed memory.
> >
> > Hold the rcu across the loop.
> >
> > Fixes: a94e2d408eae ("coredump: kill mm->core_done")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> 
> Aah, thanks...
> 
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> 
> 
> But to be honest the changelog looks a bit confusing to me...
> 
> 	curr->task = NULL;
> 	wake_up_process(task);
> 
> So wake_up_process(task) is not safe, task can exit after a spurious
> wakeup if it sees ->task == NULL.
> 
> And perhaps a short comment above "curr->task = NULL;" makes sense?

Yep.

> The reason for guard(rcu) is not obvious at all.

Ok, will change.

Re: [PATCH 1/6] coredump: don't switch a dumper that has no files table

[Chris Mason]

On Tue, 15 Sep 2026 12:22:16 +0200 Christian Brauner <brauner@kernel.org> wrote:

> Tasks without an fdtable are skipped in coredump_close_files(). The
> coredump client itself doesn't use the same check. Since
> put_files_struct() doesn't tolerate NULL it will oops for such tasks
> without an fdtable. The prime suspect for this behavior are vhost
> workers. Skip the switch for a coredump client without a table.
>
> Fixes: b2b36bcb13ea ("coredump: add COREDUMP_CLOSE_FILES")
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/coredump.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 1fba3fed1a07..791a9268ed96 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -579,7 +579,11 @@ static bool coredump_close_files(struct core_state *core_state)
>  	/* Use the dumper's real creds not the overridden ones. */
>  	scoped_with_creds(current_real_cred()) {
>  		io_uring_task_cancel();
> -		switch_files_struct(current, files);
> +		/* The dumper itself may be a vhost worker without a table. */
> +		if (current->files)
> +			switch_files_struct(current, files);
> +		else
> +			put_files_struct(files);
>  	}
>
>  	coredump_wait_inactive(core_state);

When I reproduced this one, it ended up deadlocking with the fix applied.

vhost worker (the dumper)          sibling thread
=========================          ==============
get_signal() -> vfs_coredump()
  coredump_close_files()
    hands sibling a new table ---> switch_files_struct()
    coredump_wait_inactive()         put_files_struct(old table)
      waits for sibling's switch       last close of the vhost fd
                                     vhost_net_release()
                                       __vhost_worker_flush(): waits

AI suggests making the vhost thread requeue the signal for someone more
suitable?

-chris

Re: [PATCH 1/6] coredump: don't switch a dumper that has no files table

[Christian Brauner]

> When I reproduced this one, it ended up deadlocking with the fix applied.
> 
> vhost worker (the dumper)          sibling thread
> =========================          ==============
> get_signal() -> vfs_coredump()
>   coredump_close_files()
>     hands sibling a new table ---> switch_files_struct()
>     coredump_wait_inactive()         put_files_struct(old table)
>       waits for sibling's switch       last close of the vhost fd
>                                      vhost_net_release()
>                                        __vhost_worker_flush(): waits
> 
> AI suggests making the vhost thread requeue the signal for someone more
> suitable?

Afaict, this is conceptually a generic problem with letting coredump
close files for any type of user worker where the worker is the dumper
and the sibling thread closes the fd that causes it to be stopped and
waits for it to exit. So this needs thinking and we should drop coredump
closing files for now.

Re: [PATCH 1/6] coredump: don't switch a dumper that has no files table

[NeilBrown]

On Thu, 17 Sep 2026, Christian Brauner wrote:
> > When I reproduced this one, it ended up deadlocking with the fix applied.
> > 
> > vhost worker (the dumper)          sibling thread
> > =========================          ==============
> > get_signal() -> vfs_coredump()
> >   coredump_close_files()
> >     hands sibling a new table ---> switch_files_struct()
> >     coredump_wait_inactive()         put_files_struct(old table)
> >       waits for sibling's switch       last close of the vhost fd
> >                                      vhost_net_release()
> >                                        __vhost_worker_flush(): waits
> > 
> > AI suggests making the vhost thread requeue the signal for someone more
> > suitable?
> 
> Afaict, this is conceptually a generic problem with letting coredump
> close files for any type of user worker where the worker is the dumper
> and the sibling thread closes the fd that causes it to be stopped and
> waits for it to exit. So this needs thinking and we should drop coredump
> closing files for now.
> 

It appears to me that there is already infrastructure in place to handle
the generic problem.  vhost_task has a "handle_sigkill" function which
cleans up on sig kill so that siblings won't keep waiting for it.
get_signal() detects PF_USER_WORKER tasks and lets them complete
normally rather than aborting them, so that handle_sigkill can be called.

So the problem is "simply" an ordering problem. vfs_coredump() closes
files before handle_sigkill can run.
Maybe handle_sigkill() could be passed to (a version of) get_signal(),
or maybe get_signal() could indicate to the PF_USER_WORKER caller that
vfs_coredump() still needs to be run.

NeilBrown

Possibly something like this completely untested patch, though maybe
io_uring should use the new interface too.

diff --git a/include/linux/signal.h b/include/linux/signal.h
index f19816832f05..def89bac90e3 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -291,6 +291,8 @@ extern void __set_current_blocked(const sigset_t *);
 extern int show_unhandled_signals;
 
 extern bool get_signal(struct ksignal *ksig);
+extern int get_signal_nocore(struct ksignal *ksig);
+extern void get_signal_complete(struct ksignal *ksig, int signr);
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void exit_signals(struct task_struct *tsk);
 extern void kernel_sigaction(int, __sighandler_t);
diff --git a/kernel/signal.c b/kernel/signal.c
index ec30550951ec..ab8e0207f079 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2812,7 +2812,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
 	}
 }
 
-bool get_signal(struct ksignal *ksig)
+int get_signal_nocore(struct ksignal *ksig)
 {
 	struct sighand_struct *sighand = current->sighand;
 	struct signal_struct *signal = current->signal;
@@ -2823,10 +2823,10 @@ bool get_signal(struct ksignal *ksig)
 		task_work_run();
 
 	if (!task_sigpending(current))
-		return false;
+		return 0;
 
 	if (unlikely(uprobe_deny_signal()))
-		return false;
+		return 0;
 
 	/*
 	 * Do this once, we can't return to user-mode if freezing() == T.
@@ -3021,21 +3021,6 @@ bool get_signal(struct ksignal *ksig)
 		 */
 		current->flags |= PF_SIGNALED;
 
-		if (sig_kernel_coredump(signr)) {
-			if (print_fatal_signals)
-				print_fatal_signal(signr);
-			proc_coredump_connector(current);
-			/*
-			 * If it was able to dump core, this kills all
-			 * other threads in the group and synchronizes with
-			 * their demise.  If we lost the race with another
-			 * thread getting here, it set group_exit_code
-			 * first and our do_group_exit call below will use
-			 * that value and ignore the one we pass it.
-			 */
-			vfs_coredump(&ksig->info);
-		}
-
 		/*
 		 * PF_USER_WORKER threads will catch and exit on fatal signals
 		 * themselves. They have cleanup that must be performed, so we
@@ -3058,6 +3043,38 @@ bool get_signal(struct ksignal *ksig)
 	if (signr && !(ksig->ka.sa.sa_flags & SA_EXPOSE_TAGBITS))
 		hide_si_addr_tag_bits(ksig);
 out:
+	return signr;
+}
+
+void get_signal_complete(struct ksignal *ksig, int signr)
+{
+	if (!(current->flags & PF_SIGNALED))
+		return;
+
+	if (sig_kernel_coredump(signr)) {
+		if (print_fatal_signals)
+			print_fatal_signal(signr);
+		proc_coredump_connector(current);
+		/*
+		 * If it was able to dump core, this kills all
+		 * other threads in the group and synchronizes with
+		 * their demise.  If we lost the race with another
+		 * thread getting here, it set group_exit_code
+		 * first and our do_group_exit call below will use
+		 * that value and ignore the one we pass it.
+		 */
+		vfs_coredump(&ksig->info);
+	}
+	do_group_exit(signr);
+	/* NOTREACHED */
+}
+
+bool get_signal(struct ksignal *ksig)
+{
+	int signr = get_signal_nocore(ksig);
+
+	if (signr)
+		get_signal_complete(ksig, signr);
 	return signr > 0;
 }
 
diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
index 3717885a5992..8bdf5b55f419 100644
--- a/kernel/vhost_task.c
+++ b/kernel/vhost_task.c
@@ -27,14 +27,15 @@ struct vhost_task {
 static int vhost_task_fn(void *data)
 {
 	struct vhost_task *vtsk = data;
+	struct ksignal ksig;
+	int signr = 0;
 
 	for (;;) {
 		bool did_work;
 
 		if (signal_pending(current)) {
-			struct ksignal ksig;
-
-			if (get_signal(&ksig))
+			signr = get_signal_nocore(&ksig);
+			if (signr)
 				break;
 		}
 
@@ -64,6 +65,8 @@ static int vhost_task_fn(void *data)
 	mutex_unlock(&vtsk->exit_mutex);
 	complete(&vtsk->exited);
 
+	if (signr)
+		get_signal_complete(&ksig, signr);
 	do_exit(0);
 }
 

user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table

[Christian Brauner]

On Thu, Sep 17, 2026 at 11:51:14AM +1000, NeilBrown wrote:
> On Thu, 17 Sep 2026, Christian Brauner wrote:
> > > When I reproduced this one, it ended up deadlocking with the fix applied.
> > > 
> > > vhost worker (the dumper)          sibling thread
> > > =========================          ==============
> > > get_signal() -> vfs_coredump()
> > >   coredump_close_files()
> > >     hands sibling a new table ---> switch_files_struct()
> > >     coredump_wait_inactive()         put_files_struct(old table)
> > >       waits for sibling's switch       last close of the vhost fd
> > >                                      vhost_net_release()
> > >                                        __vhost_worker_flush(): waits
> > > 
> > > AI suggests making the vhost thread requeue the signal for someone more
> > > suitable?
> > 
> > Afaict, this is conceptually a generic problem with letting coredump
> > close files for any type of user worker where the worker is the dumper
> > and the sibling thread closes the fd that causes it to be stopped and
> > waits for it to exit. So this needs thinking and we should drop coredump
> > closing files for now.
> > 
> 
> It appears to me that there is already infrastructure in place to handle
> the generic problem.  vhost_task has a "handle_sigkill" function which
> cleans up on sig kill so that siblings won't keep waiting for it.
> get_signal() detects PF_USER_WORKER tasks and lets them complete
> normally rather than aborting them, so that handle_sigkill can be called.
> 
> So the problem is "simply" an ordering problem. vfs_coredump() closes
> files before handle_sigkill can run.
> Maybe handle_sigkill() could be passed to (a version of) get_signal(),
> or maybe get_signal() could indicate to the PF_USER_WORKER caller that
> vfs_coredump() still needs to be run.

Oh good, it's worse. This isn't specific to this patchset. This class of
bugs I mentioned is reproducible on current mainline with SQPOLL:

  io-wq worker (the dumper)               SQPOLL thread
  =========================               =============
  io_wq_worker()                          io_sq_thread()
    get_signal()                            io_sqd_handle_event()
      vfs_coredump()
        coredump_wait()
          zap_threads()
            zap_process(): SIGKILL ---->      get_signal(): SIGKILL, returns
          wait_for_completion_state(        io_uring_cancel_generic(true, sqd)
            &core_state->startup)             io_uring_clean_tctx()
          waits for every thread to             io_wq_put_and_exit()
          reach coredump_task_exit()              io_wq_exit_workers()
                                                    wait_for_completion(
                                                      &wq->worker_done)
                                                    waits for the worker's
                                                    io_worker_exit(), which
                                                    runs after get_signal()

Here's a selftest to reproduce it:

From 3099604cb701a7a5242986653add8c52d4b6d2c6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Thu, 17 Sep 2026 10:20:49 +0200
Subject: [PATCH] selftests/coredump: test a user worker as the coredumping
 thread

A tracer can clear the signal mask of an io-wq worker or an SQPOLL
thread with PTRACE_SETSIGMASK and inject a coredump signal. The worker
then runs vfs_coredump() itself. Cover that:

- a child keeps a ring, an idle io-wq worker and with SQPOLL the SQPOLL
  thread alive

- seize the chosen thread, stop it with SIGSTOP, clear its mask and
  detach with SIGSEGV

- require the thread group to be gone in bounded time, by the dump or
  by SIGKILL

The io-wq worker of an SQPOLL ring fails: the SQPOLL thread waits for
its workers to exit before it parks, the dumping worker waits for the
SQPOLL thread to park and the group is stuck in D state.

This reproduces the following hang when SQPOLL becomes the coredumper:

  io-wq worker (the dumper)               SQPOLL thread
  =========================               =============
  io_wq_worker()                          io_sq_thread()
    get_signal()                            io_sqd_handle_event()
      vfs_coredump()
        coredump_wait()
          zap_threads()
            zap_process(): SIGKILL ---->      get_signal(): SIGKILL, returns
          wait_for_completion_state(        io_uring_cancel_generic(true, sqd)
            &core_state->startup)             io_uring_clean_tctx()
          waits for every thread to             io_wq_put_and_exit()
          reach coredump_task_exit()              io_wq_exit_workers()
                                                    wait_for_completion(
                                                      &wq->worker_done)
                                                    waits for the worker's
                                                    io_worker_exit(), which
                                                    runs after get_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_worker_test.c | 427 ++++++++++++++++++
 3 files changed, 431 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/coredump/coredump_worker_test.c

diff --git a/tools/testing/selftests/coredump/.gitignore b/tools/testing/selftests/coredump/.gitignore
index c198f2ca5872..e32f6e9006f6 100644
--- a/tools/testing/selftests/coredump/.gitignore
+++ b/tools/testing/selftests/coredump/.gitignore
@@ -3,3 +3,4 @@ stackdump_test
 coredump_socket_test
 coredump_socket_protocol_test
 coredump_signal_test
+coredump_worker_test
diff --git a/tools/testing/selftests/coredump/Makefile b/tools/testing/selftests/coredump/Makefile
index bfb53f512d8e..9459ba3692c1 100644
--- a/tools/testing/selftests/coredump/Makefile
+++ b/tools/testing/selftests/coredump/Makefile
@@ -5,7 +5,8 @@ TEST_GEN_PROGS := stackdump_test \
 		  coredump_socket_test \
 		  coredump_socket_protocol_test \
 		  coredump_close_files_test \
-		  coredump_signal_test
+		  coredump_signal_test \
+		  coredump_worker_test
 TEST_FILES := stackdump
 
 include ../lib.mk
@@ -15,3 +16,4 @@ $(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
+$(OUTPUT)/coredump_worker_test: coredump_test_helpers.c
diff --git a/tools/testing/selftests/coredump/coredump_worker_test.c b/tools/testing/selftests/coredump/coredump_worker_test.c
new file mode 100644
index 000000000000..81cadde0e752
--- /dev/null
+++ b/tools/testing/selftests/coredump/coredump_worker_test.c
@@ -0,0 +1,427 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * A user worker as the coredumping thread.
+ *
+ * io-wq workers and SQPOLL threads are threads of the process that never
+ * return to userspace. They block every signal but SIGKILL and SIGSTOP,
+ * but a tracer can replace that mask with PTRACE_SETSIGMASK and inject a
+ * coredump signal. get_signal() then runs vfs_coredump() in the worker.
+ * The worker's own exit bookkeeping runs only after the dump, so a
+ * zapped sibling that waits for it in its exit path deadlocks with the
+ * dumper and the whole thread group is stuck in D state.
+ *
+ * Inject SIGSEGV into a chosen thread and require that the thread group
+ * is gone in bounded time, either because the dump completed or because
+ * SIGKILL still works. A failure leaves the stuck process behind.
+ */
+#include <ctype.h>
+#include <dirent.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <sys/ptrace.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <linux/io_uring.h>
+
+#include "coredump_test.h"
+
+#ifndef PTRACE_SETSIGMASK
+#define PTRACE_SETSIGMASK 0x420b
+#endif
+
+/* The dump of the tiny child takes well under a second. */
+#define EXIT_TIMEOUT_MS 5000
+
+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;
+
+	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);
+}
+
+/* A raw ring, no liburing. */
+struct uring {
+	int fd;
+	struct io_uring_params params;
+	void *sq;
+	size_t sq_len;
+	struct io_uring_sqe *sqes;
+	size_t sqes_len;
+	unsigned int *sq_tail, *sq_mask, *sq_array;
+	unsigned int *cq_head, *cq_tail, *cq_mask;
+	struct io_uring_cqe *cqes;
+};
+
+static int uring_setup(struct uring *r, unsigned int flags)
+{
+	size_t cq_len;
+
+	memset(r, 0, sizeof(*r));
+	r->params.flags = flags;
+	if (flags & IORING_SETUP_SQPOLL)
+		r->params.sq_thread_idle = 2000;
+	r->fd = syscall(__NR_io_uring_setup, 8, &r->params);
+	if (r->fd < 0)
+		return -1;
+	if (!(r->params.features & IORING_FEAT_SINGLE_MMAP))
+		return -1;
+
+	r->sq_len = r->params.sq_off.array + r->params.sq_entries * sizeof(unsigned int);
+	cq_len = r->params.cq_off.cqes + r->params.cq_entries * sizeof(struct io_uring_cqe);
+	if (cq_len > r->sq_len)
+		r->sq_len = cq_len;
+	r->sq = mmap(NULL, r->sq_len, PROT_READ | PROT_WRITE,
+		     MAP_SHARED | MAP_POPULATE, r->fd, IORING_OFF_SQ_RING);
+	if (r->sq == MAP_FAILED)
+		return -1;
+	r->sqes_len = r->params.sq_entries * sizeof(struct io_uring_sqe);
+	r->sqes = mmap(NULL, r->sqes_len, PROT_READ | PROT_WRITE,
+		       MAP_SHARED | MAP_POPULATE, r->fd, IORING_OFF_SQES);
+	if (r->sqes == MAP_FAILED)
+		return -1;
+
+	r->sq_tail = r->sq + r->params.sq_off.tail;
+	r->sq_mask = r->sq + r->params.sq_off.ring_mask;
+	r->sq_array = r->sq + r->params.sq_off.array;
+	r->cq_head = r->sq + r->params.cq_off.head;
+	r->cq_tail = r->sq + r->params.cq_off.tail;
+	r->cq_mask = r->sq + r->params.cq_off.ring_mask;
+	r->cqes = r->sq + r->params.cq_off.cqes;
+	return 0;
+}
+
+/* Submit one sqe, wait for its completion and return the result. */
+static int uring_submit_wait(struct uring *r, const struct io_uring_sqe *sqe)
+{
+	unsigned int tail = *r->sq_tail, idx = tail & *r->sq_mask;
+	unsigned int flags = IORING_ENTER_GETEVENTS;
+	int i;
+
+	r->sqes[idx] = *sqe;
+	r->sq_array[idx] = idx;
+	__atomic_store_n(r->sq_tail, tail + 1, __ATOMIC_RELEASE);
+
+	if (r->params.flags & IORING_SETUP_SQPOLL)
+		flags |= IORING_ENTER_SQ_WAKEUP;
+
+	for (i = 0; i < 100; i++) {
+		if (syscall(__NR_io_uring_enter, r->fd, 1, 1, flags, NULL, 0) < 0 &&
+		    errno != EINTR)
+			return -1;
+		if (__atomic_load_n(r->cq_tail, __ATOMIC_ACQUIRE) != *r->cq_head) {
+			unsigned int head = *r->cq_head;
+			int res = r->cqes[head & *r->cq_mask].res;
+
+			__atomic_store_n(r->cq_head, head + 1, __ATOMIC_RELEASE);
+			return res;
+		}
+		flags &= ~IORING_ENTER_SQ_WAKEUP;
+		usleep(10 * 1000);
+	}
+	return -1;
+}
+
+static bool uring_available(unsigned int flags)
+{
+	struct io_uring_params params = { .flags = flags };
+	int fd;
+
+	fd = syscall(__NR_io_uring_setup, 2, &params);
+	if (fd < 0)
+		return false;
+	close(fd);
+	return true;
+}
+
+/*
+ * Keep a ring, an idle io-wq worker and with SQPOLL an SQPOLL thread
+ * alive. The last worker of a ring never exits on its idle timeout.
+ */
+static void worker_child(bool sqpoll, int fd_ipc)
+{
+	struct rlimit rl = { RLIM_INFINITY, RLIM_INFINITY };
+	struct io_uring_sqe sqe = {};
+	static char buf[64];
+	struct uring ring;
+	int memfd;
+
+	if (setrlimit(RLIMIT_CORE, &rl))
+		_exit(EXIT_FAILURE);
+
+	memfd = memfd_create("coredump_worker", 0);
+	if (memfd < 0 || write(memfd, "hello", 5) != 5)
+		_exit(EXIT_FAILURE);
+
+	if (uring_setup(&ring, sqpoll ? IORING_SETUP_SQPOLL : 0))
+		_exit(EXIT_FAILURE);
+
+	/* IOSQE_ASYNC forces the read through io-wq so a worker appears. */
+	sqe.opcode = IORING_OP_READ;
+	sqe.fd = memfd;
+	sqe.addr = (__u64)(uintptr_t)buf;
+	sqe.len = sizeof(buf);
+	sqe.flags = IOSQE_ASYNC;
+	if (uring_submit_wait(&ring, &sqe) != 5)
+		_exit(EXIT_FAILURE);
+
+	if (write_nointr(fd_ipc, "1", 1) != 1)
+		_exit(EXIT_FAILURE);
+	close(fd_ipc);
+
+	for (;;)
+		pause();
+}
+
+/* Find the thread of @pid whose comm starts with @prefix. */
+static pid_t find_thread(pid_t pid, const char *prefix)
+{
+	char path[64], comm[64];
+	pid_t tid = -1;
+	struct dirent *de;
+	ssize_t bytes;
+	DIR *dir;
+	int fd;
+
+	snprintf(path, sizeof(path), "/proc/%d/task", pid);
+	dir = opendir(path);
+	if (!dir)
+		return -1;
+
+	while (tid < 0 && (de = readdir(dir))) {
+		if (!isdigit(de->d_name[0]))
+			continue;
+		snprintf(path, sizeof(path), "/proc/%d/task/%s/comm", pid, de->d_name);
+		fd = open(path, O_RDONLY | O_CLOEXEC);
+		if (fd < 0)
+			continue;
+		bytes = read(fd, comm, sizeof(comm) - 1);
+		close(fd);
+		if (bytes <= 0)
+			continue;
+		comm[bytes] = '\0';
+		if (!strncmp(comm, prefix, strlen(prefix)))
+			tid = atoi(de->d_name);
+	}
+	closedir(dir);
+	return tid;
+}
+
+/*
+ * Attach, stop the thread with SIGSTOP, drop the signal mask that
+ * copy_process() gave it and resume it with SIGSEGV instead.
+ */
+static bool inject_coredump_signal(pid_t pid, pid_t tid)
+{
+	__u64 mask = 0;
+	int status;
+
+	if (ptrace(PTRACE_SEIZE, tid, NULL, NULL))
+		return false;
+	if (syscall(SYS_tgkill, pid, tid, SIGSTOP))
+		return false;
+	if (waitpid(tid, &status, __WALL) != tid)
+		return false;
+	if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGSTOP)
+		return false;
+	if (ptrace(PTRACE_SETSIGMASK, tid, sizeof(mask), &mask))
+		return false;
+	return !ptrace(PTRACE_DETACH, tid, NULL, (void *)(long)SIGSEGV);
+}
+
+/* Reap @pid within @timeout_ms, -1 when it is still there. */
+static int wait_exit(pid_t pid, int *status, int timeout_ms)
+{
+	int i;
+
+	for (i = 0; i < timeout_ms / 10; i++) {
+		pid_t ret = waitpid(pid, status, WNOHANG);
+
+		if (ret == pid)
+			return 0;
+		if (ret < 0)
+			return -1;
+		usleep(10 * 1000);
+	}
+	return -1;
+}
+
+static void log_threads(struct __test_metadata *const _metadata, pid_t pid)
+{
+	char path[64], line[256], comm[64] = {};
+	struct dirent *de;
+	DIR *dir;
+	FILE *f;
+
+	snprintf(path, sizeof(path), "/proc/%d/task", pid);
+	dir = opendir(path);
+	if (!dir)
+		return;
+	while ((de = readdir(dir))) {
+		if (!isdigit(de->d_name[0]))
+			continue;
+		snprintf(path, sizeof(path), "/proc/%d/task/%s/status", pid, de->d_name);
+		f = fopen(path, "r");
+		if (!f)
+			continue;
+		while (fgets(line, sizeof(line), f)) {
+			line[strcspn(line, "\n")] = '\0';
+			if (!strncmp(line, "Name:", 5))
+				snprintf(comm, sizeof(comm), "%s", line + 6);
+			else if (!strncmp(line, "State:", 6))
+				TH_LOG("tid %s (%s) %s", de->d_name, comm, line + 7);
+		}
+		fclose(f);
+	}
+	closedir(dir);
+}
+
+enum dumper {
+	DUMPER_MAIN,
+	DUMPER_WORKER,
+	DUMPER_SQPOLL,
+};
+
+static void run_dumper(struct __test_metadata *const _metadata, bool sqpoll,
+		       enum dumper dumper)
+{
+	bool killed = false;
+	char path[64], c;
+	int ipc[2], status, fd;
+	pid_t pid, tid;
+
+	ASSERT_TRUE(set_core_pattern("/tmp/coredump.file.%p"));
+	ASSERT_EQ(pipe2(ipc, O_CLOEXEC), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		close(ipc[0]);
+		worker_child(sqpoll, ipc[1]);
+	}
+	close(ipc[1]);
+	ASSERT_EQ(read_nointr(ipc[0], &c, 1), 1);
+	close(ipc[0]);
+
+	switch (dumper) {
+	case DUMPER_MAIN:
+		tid = pid;
+		break;
+	case DUMPER_WORKER:
+		tid = find_thread(pid, "iou-wrk-");
+		break;
+	case DUMPER_SQPOLL:
+		tid = find_thread(pid, "iou-sqp-");
+		break;
+	}
+	ASSERT_GT(tid, 0);
+	ASSERT_TRUE(inject_coredump_signal(pid, tid));
+
+	if (wait_exit(pid, &status, EXIT_TIMEOUT_MS)) {
+		/* No dump. Whatever happened, SIGKILL must still work. */
+		log_threads(_metadata, pid);
+		kill(pid, SIGKILL);
+		killed = true;
+		ASSERT_EQ(wait_exit(pid, &status, EXIT_TIMEOUT_MS), 0) {
+			TH_LOG("thread group %d is stuck after SIGSEGV to tid %d",
+			       pid, tid);
+		}
+	}
+
+	ASSERT_TRUE(WIFSIGNALED(status));
+	if (killed) {
+		TH_LOG("tid %d did not dump, the group was killed instead", tid);
+		ASSERT_EQ(WTERMSIG(status), SIGKILL);
+		return;
+	}
+	ASSERT_EQ(WTERMSIG(status), SIGSEGV);
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	snprintf(path, sizeof(path), "/tmp/coredump.file.%d", pid);
+	fd = open(path, O_RDONLY | O_CLOEXEC);
+	unlink(path);
+	ASSERT_GE(fd, 0);
+	ASSERT_TRUE(check_coredump_extent(fd));
+	close(fd);
+}
+
+/* The mechanics: an injected SIGSEGV into a normal thread dumps core. */
+TEST_F(coredump, main_thread_dumper)
+{
+	if (!uring_available(0))
+		SKIP(return, "io_uring is not available");
+	run_dumper(_metadata, false, DUMPER_MAIN);
+}
+
+TEST_F(coredump, plain_worker_dumper)
+{
+	if (!uring_available(0))
+		SKIP(return, "io_uring is not available");
+	run_dumper(_metadata, false, DUMPER_WORKER);
+}
+
+TEST_F(coredump, sqpoll_thread_dumper)
+{
+	if (!uring_available(IORING_SETUP_SQPOLL))
+		SKIP(return, "io_uring SQPOLL is not available");
+	run_dumper(_metadata, true, DUMPER_SQPOLL);
+}
+
+/*
+ * The SQPOLL thread leaves its loop on the zap and waits for its io-wq
+ * workers to exit before it parks. The dumping worker never does.
+ */
+TEST_F(coredump, sqpoll_worker_dumper)
+{
+	if (!uring_available(IORING_SETUP_SQPOLL))
+		SKIP(return, "io_uring SQPOLL is not available");
+	run_dumper(_metadata, true, DUMPER_WORKER);
+}
+
+TEST_HARNESS_MAIN
-- 
2.53.0

Re: user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table

[Christian Brauner]

On Thu, Sep 17, 2026 at 10:49:36AM +0200, Christian Brauner wrote:
> On Thu, Sep 17, 2026 at 11:51:14AM +1000, NeilBrown wrote:
> > On Thu, 17 Sep 2026, Christian Brauner wrote:
> > > > When I reproduced this one, it ended up deadlocking with the fix applied.
> > > > 
> > > > vhost worker (the dumper)          sibling thread
> > > > =========================          ==============
> > > > get_signal() -> vfs_coredump()
> > > >   coredump_close_files()
> > > >     hands sibling a new table ---> switch_files_struct()
> > > >     coredump_wait_inactive()         put_files_struct(old table)
> > > >       waits for sibling's switch       last close of the vhost fd
> > > >                                      vhost_net_release()
> > > >                                        __vhost_worker_flush(): waits
> > > > 
> > > > AI suggests making the vhost thread requeue the signal for someone more
> > > > suitable?
> > > 
> > > Afaict, this is conceptually a generic problem with letting coredump
> > > close files for any type of user worker where the worker is the dumper
> > > and the sibling thread closes the fd that causes it to be stopped and
> > > waits for it to exit. So this needs thinking and we should drop coredump
> > > closing files for now.
> > > 
> > 
> > It appears to me that there is already infrastructure in place to handle
> > the generic problem.  vhost_task has a "handle_sigkill" function which
> > cleans up on sig kill so that siblings won't keep waiting for it.
> > get_signal() detects PF_USER_WORKER tasks and lets them complete
> > normally rather than aborting them, so that handle_sigkill can be called.
> > 
> > So the problem is "simply" an ordering problem. vfs_coredump() closes
> > files before handle_sigkill can run.
> > Maybe handle_sigkill() could be passed to (a version of) get_signal(),
> > or maybe get_signal() could indicate to the PF_USER_WORKER caller that
> > vfs_coredump() still needs to be run.
> 
> Oh good, it's worse. This isn't specific to this patchset. This class of
> bugs I mentioned is reproducible on current mainline with SQPOLL:
> 
>   io-wq worker (the dumper)               SQPOLL thread
>   =========================               =============
>   io_wq_worker()                          io_sq_thread()
>     get_signal()                            io_sqd_handle_event()
>       vfs_coredump()
>         coredump_wait()
>           zap_threads()
>             zap_process(): SIGKILL ---->      get_signal(): SIGKILL, returns
>           wait_for_completion_state(        io_uring_cancel_generic(true, sqd)
>             &core_state->startup)             io_uring_clean_tctx()
>           waits for every thread to             io_wq_put_and_exit()
>           reach coredump_task_exit()              io_wq_exit_workers()
>                                                     wait_for_completion(
>                                                       &wq->worker_done)
>                                                     waits for the worker's
>                                                     io_worker_exit(), which
>                                                     runs after get_signal()

So I thought a bit about this yesterday and had some brief discussions
bout this as well. I think we should kill this whole bug class by
ensuring that PF_USER_WORKERs never participate in a coredump. So
something like:

diff --git a/kernel/signal.c b/kernel/signal.c
index ec30550951ec..e3c271cf1314 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3021,6 +3021,21 @@ bool get_signal(struct ksignal *ksig)
 		 */
 		current->flags |= PF_SIGNALED;
 
+		/*
+		 * PF_USER_WORKER threads will catch and exit on fatal signals
+		 * themselves. They have cleanup that must be performed, so we
+		 * cannot call do_exit() on their behalf. Note that ksig won't
+		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
+		 *
+		 * They must not dump core either. The dumper waits for every
+		 * other thread in the group to exit, and a sibling's exit path
+		 * may in turn wait for this worker's own exit, which only runs
+		 * after get_signal() returns: io_sq_thread() ends up in
+		 * io_wq_exit_workers() and waits there for its io-wq workers.
+		 */
+		if (current->flags & PF_USER_WORKER)
+			goto out;
+
 		if (sig_kernel_coredump(signr)) {
 			if (print_fatal_signals)
 				print_fatal_signal(signr);
@@ -3036,15 +3051,6 @@ bool get_signal(struct ksignal *ksig)
 			vfs_coredump(&ksig->info);
 		}
 
-		/*
-		 * PF_USER_WORKER threads will catch and exit on fatal signals
-		 * themselves. They have cleanup that must be performed, so we
-		 * cannot call do_exit() on their behalf. Note that ksig won't
-		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
-		 */
-		if (current->flags & PF_USER_WORKER)
-			goto out;
-
 		/*
 		 * Death signals, no core dump.
 		 */

Afaict the main consequence of this is for a thread-group with a bunch
of spawned workers where the thread-group leader exits prematurely
before all workers: any worker that takes a coredump signal via ptrace()
shenanigans won't create a coredump anymore. I think that's entirely
acceptable.

Re: user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table

[Christian Brauner]

On Fri, Sep 18, 2026 at 02:28:58PM +0200, Christian Brauner wrote:
> On Thu, Sep 17, 2026 at 10:49:36AM +0200, Christian Brauner wrote:
> > On Thu, Sep 17, 2026 at 11:51:14AM +1000, NeilBrown wrote:
> > > On Thu, 17 Sep 2026, Christian Brauner wrote:
> > > > > When I reproduced this one, it ended up deadlocking with the fix applied.
> > > > > 
> > > > > vhost worker (the dumper)          sibling thread
> > > > > =========================          ==============
> > > > > get_signal() -> vfs_coredump()
> > > > >   coredump_close_files()
> > > > >     hands sibling a new table ---> switch_files_struct()
> > > > >     coredump_wait_inactive()         put_files_struct(old table)
> > > > >       waits for sibling's switch       last close of the vhost fd
> > > > >                                      vhost_net_release()
> > > > >                                        __vhost_worker_flush(): waits
> > > > > 
> > > > > AI suggests making the vhost thread requeue the signal for someone more
> > > > > suitable?
> > > > 
> > > > Afaict, this is conceptually a generic problem with letting coredump
> > > > close files for any type of user worker where the worker is the dumper
> > > > and the sibling thread closes the fd that causes it to be stopped and
> > > > waits for it to exit. So this needs thinking and we should drop coredump
> > > > closing files for now.
> > > > 
> > > 
> > > It appears to me that there is already infrastructure in place to handle
> > > the generic problem.  vhost_task has a "handle_sigkill" function which
> > > cleans up on sig kill so that siblings won't keep waiting for it.
> > > get_signal() detects PF_USER_WORKER tasks and lets them complete
> > > normally rather than aborting them, so that handle_sigkill can be called.
> > > 
> > > So the problem is "simply" an ordering problem. vfs_coredump() closes
> > > files before handle_sigkill can run.
> > > Maybe handle_sigkill() could be passed to (a version of) get_signal(),
> > > or maybe get_signal() could indicate to the PF_USER_WORKER caller that
> > > vfs_coredump() still needs to be run.
> > 
> > Oh good, it's worse. This isn't specific to this patchset. This class of
> > bugs I mentioned is reproducible on current mainline with SQPOLL:
> > 
> >   io-wq worker (the dumper)               SQPOLL thread
> >   =========================               =============
> >   io_wq_worker()                          io_sq_thread()
> >     get_signal()                            io_sqd_handle_event()
> >       vfs_coredump()
> >         coredump_wait()
> >           zap_threads()
> >             zap_process(): SIGKILL ---->      get_signal(): SIGKILL, returns
> >           wait_for_completion_state(        io_uring_cancel_generic(true, sqd)
> >             &core_state->startup)             io_uring_clean_tctx()
> >           waits for every thread to             io_wq_put_and_exit()
> >           reach coredump_task_exit()              io_wq_exit_workers()
> >                                                     wait_for_completion(
> >                                                       &wq->worker_done)
> >                                                     waits for the worker's
> >                                                     io_worker_exit(), which
> >                                                     runs after get_signal()
> 
> So I thought a bit about this yesterday and had some brief discussions
> bout this as well. I think we should kill this whole bug class by
> ensuring that PF_USER_WORKERs never participate in a coredump. So
> something like:
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index ec30550951ec..e3c271cf1314 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3021,6 +3021,21 @@ bool get_signal(struct ksignal *ksig)
>  		 */
>  		current->flags |= PF_SIGNALED;
>  
> +		/*
> +		 * PF_USER_WORKER threads will catch and exit on fatal signals
> +		 * themselves. They have cleanup that must be performed, so we
> +		 * cannot call do_exit() on their behalf. Note that ksig won't
> +		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
> +		 *
> +		 * They must not dump core either. The dumper waits for every
> +		 * other thread in the group to exit, and a sibling's exit path
> +		 * may in turn wait for this worker's own exit, which only runs
> +		 * after get_signal() returns: io_sq_thread() ends up in
> +		 * io_wq_exit_workers() and waits there for its io-wq workers.
> +		 */
> +		if (current->flags & PF_USER_WORKER)
> +			goto out;
> +
>  		if (sig_kernel_coredump(signr)) {
>  			if (print_fatal_signals)
>  				print_fatal_signal(signr);
> @@ -3036,15 +3051,6 @@ bool get_signal(struct ksignal *ksig)
>  			vfs_coredump(&ksig->info);
>  		}
>  
> -		/*
> -		 * PF_USER_WORKER threads will catch and exit on fatal signals
> -		 * themselves. They have cleanup that must be performed, so we
> -		 * cannot call do_exit() on their behalf. Note that ksig won't
> -		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
> -		 */
> -		if (current->flags & PF_USER_WORKER)
> -			goto out;
> -
>  		/*
>  		 * Death signals, no core dump.
>  		 */
> 
> Afaict the main consequence of this is for a thread-group with a bunch
> of spawned workers where the thread-group leader exits prematurely
> before all workers: any worker that takes a coredump signal via ptrace()
> shenanigans won't create a coredump anymore. I think that's entirely
> acceptable.

So to put some details on it after some research:

io-wq workers die with their owner's exit() and vhost workers with the
files table. But SQPOLL threads don't necessarily. So if a premature
thread-group leader exits then an SQPOLL thread can be left. The SQPOLL
thread pins the fdtable through CLOSE_FILES, the fdtable pins the io
ring, the rings pins the thread.

So here's how the behavior would change with my proposed patch:

|                case                |                           mainline                           |  PF_USER_WORKERS don't dump             
|────────────────────────────────────|──────────────────────────────────────────────────────────────|───────────────────────────────
| leader gone, no signal             | zombie leader + iou-sqp, not reapable, lingers until SIGKILL | same                          
|────────────────────────────────────|──────────────────────────────────────────────────────────────|───────────────────────────────
| leader gone, SIGSEGV into iou-sqp  | killed by signal 11, core written                            | exited with status 0, no core 
|────────────────────────────────────|──────────────────────────────────────────────────────────────|───────────────────────────────
| leader gone, SIGTERM into iou-sqp  | exited with status 0                                         | exited with status 0          
|────────────────────────────────────|──────────────────────────────────────────────────────────────|───────────────────────────────
| leader alive, SIGSEGV into iou-sqp | whole group killed by 11, core written                       | process survives, worker gone 
|────────────────────────────────────|──────────────────────────────────────────────────────────────|───────────────────────────────

Please note that only ptrace() can be used to inject a coredump signal
into such SQPOLL threads.

TL;DR: with PF_USER_WORKERS not being eligible from writing coredumps
any coredump signal sent to them is ignored. They just exit.

The alternative to this is akin to what Neil proposed where the coredump
signal is retargeted towards a non-PF_USER_WORKER thread.

One should note that PF_USER_WORKERs very much intentionally blog
everything apart from SIGKILL and SIGSTOP so being able to change their
signal mask via ptrace() is questionable in the first place.

In fact, the more I think about it I keep thinking we should do both:

(1) PF_USER_WORKERs can't become a dumper
(2) ptrace() should not be able to alter the signal mask for a PF_USER_WORKER

Re: user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table

[Oleg Nesterov]

On 09/18, Christian Brauner wrote:
>
> So I thought a bit about this yesterday and had some brief discussions
> bout this as well. I think we should kill this whole bug class by
> ensuring that PF_USER_WORKERs never participate in a coredump.

Agreed,

> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3021,6 +3021,21 @@ bool get_signal(struct ksignal *ksig)
>  		 */
>  		current->flags |= PF_SIGNALED;
>
> +		/*
> +		 * PF_USER_WORKER threads will catch and exit on fatal signals
> +		 * themselves. They have cleanup that must be performed, so we
> +		 * cannot call do_exit() on their behalf. Note that ksig won't
> +		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
> +		 *
> +		 * They must not dump core either. The dumper waits for every
> +		 * other thread in the group to exit, and a sibling's exit path
> +		 * may in turn wait for this worker's own exit, which only runs
> +		 * after get_signal() returns: io_sq_thread() ends up in
> +		 * io_wq_exit_workers() and waits there for its io-wq workers.
> +		 */
> +		if (current->flags & PF_USER_WORKER)
> +			goto out;

This probably makes sense anyway... but see below.

Perhaps it makes sense to change ptrace(PTRACE_SETSIGMASK) to fail if
child->flags & PF_USER_WORKER ?

Currently get_signal() from PF_USER_WORKER can only return SIGKILL,
I think we should keep this rule.



I don't think your change can fix all problems. Suppose we have a main
thread T and a PF_USER_WORKER sub-thread W.

Some signal, say, SIGHUP has a handler. Debugger unblocks SIGHUP for W.

Now, kill(SIGHUP, T) can choose W as a target, complete_signal() can
choose any thread if wants_signal(t) is true. W will "drop" this signal,
its signal handler won't be called.


In the longer term it would be nice to rework this logic somehow to not
rely on ->blocked...

Oleg.

Re: user workers as coredumpers [Re: [PATCH 1/6] coredump: don't switch a dumper that has no files] table

[Christian Brauner]

On Sun, Sep 20, 2026 at 05:15:09PM +0200, Oleg Nesterov wrote:
> On 09/18, Christian Brauner wrote:
> >
> > So I thought a bit about this yesterday and had some brief discussions
> > bout this as well. I think we should kill this whole bug class by
> > ensuring that PF_USER_WORKERs never participate in a coredump.
> 
> Agreed,
> 
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -3021,6 +3021,21 @@ bool get_signal(struct ksignal *ksig)
> >  		 */
> >  		current->flags |= PF_SIGNALED;
> >
> > +		/*
> > +		 * PF_USER_WORKER threads will catch and exit on fatal signals
> > +		 * themselves. They have cleanup that must be performed, so we
> > +		 * cannot call do_exit() on their behalf. Note that ksig won't
> > +		 * be properly initialized, PF_USER_WORKER's shouldn't use it.
> > +		 *
> > +		 * They must not dump core either. The dumper waits for every
> > +		 * other thread in the group to exit, and a sibling's exit path
> > +		 * may in turn wait for this worker's own exit, which only runs
> > +		 * after get_signal() returns: io_sq_thread() ends up in
> > +		 * io_wq_exit_workers() and waits there for its io-wq workers.
> > +		 */
> > +		if (current->flags & PF_USER_WORKER)
> > +			goto out;
> 
> This probably makes sense anyway... but see below.
> 
> Perhaps it makes sense to change ptrace(PTRACE_SETSIGMASK) to fail if
> child->flags & PF_USER_WORKER ?

Yes, see my other mail.

> Currently get_signal() from PF_USER_WORKER can only return SIGKILL,
> I think we should keep this rule.

Yes, agreed.

> I don't think your change can fix all problems. Suppose we have a main
> thread T and a PF_USER_WORKER sub-thread W.
> 
> Some signal, say, SIGHUP has a handler. Debugger unblocks SIGHUP for W.
> 
> Now, kill(SIGHUP, T) can choose W as a target, complete_signal() can
> choose any thread if wants_signal(t) is true. W will "drop" this signal,
> its signal handler won't be called.
> 
> 
> In the longer term it would be nice to rework this logic somehow to not
> rely on ->blocked...

Agreed.