[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 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

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

[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

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

[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 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