[PATCH v3 00/17] coredump & signals: an impossible affair

[PATCH v3 00/17] coredump & signals: an impossible affair

[Christian Brauner]

Hey,

I asked Chris to look at the coredump code with kres and it found a few
bugs. I started looking as well and found a few more. Here's a fixes
series. I also used TLA+ modeling for this.

Fixes in here:

- UAF in coredump_finish(): a parked thread can be freed before it is
  woken
- only SIGKILL and the freezers interrupt a dump now, cgroup v2 included
- core_pattern is parsed from a snapshot instead of racing the sysctl
- the io-wq exit bit wasn't ordered against worker creation task work,
  exit could hang on worker_done
- shared signals are no longer retargeted to the dumper, that truncated
  cores
- a failed fork released its files under scx_fork_rwsem, deadlock
- a session leader's exit lost the SIGHUP for the foreground job
- an io-wq worker of an SQPOLL ring as the dumper deadlocks the group,
  user workers never dump now
- PTRACE_SETSIGMASK can't unmask a user worker anymore, the only way in
- exec cancels io_uring before de_thread(), nothing adds a thread after
  it
- no io threads from PF_SIGNALED or PF_POSTCOREDUMP creators, they
  broke threads_remaining
- descriptor tables are closed highest fd first again, the order the
  deferred puts had

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v3:
- Address Oleg's reviews.
- Add a couple more fixes.
- Link to v2: https://patch.msgid.link/20260917-work-coredump-fixes-v2-0-f3787fcda051@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 (18):
      Merge patch series "files: make closing files synchronous for close_range(), exec, exit"
      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
      fork: release the files of a failed fork after sched_cancel_fork()
      exit: hang up the tty before closing the files
      ptrace: refuse to change the signal mask of a user worker
      selftests/coredump: test a user worker as the coredumping thread
      selftests/coredump: expect PTRACE_SETSIGMASK to be refused on a user worker
      exec: cancel io_uring requests before de_thread()
      fork: move the coredump and exec checks into create_io_thread()
      fork: don't create io threads once PF_POSTCOREDUMP is set
      fork: use SIG_KERNEL_ONLY_MASK for the user worker signal mask
      signal: enforce the user worker signal mask in __set_task_blocked()
      fs: close files from the highest descriptor down

 fs/coredump.c                                      |  73 ++--
 fs/exec.c                                          |  11 +-
 fs/file.c                                          |  55 +--
 include/linux/sched/signal.h                       |  19 +-
 io_uring/io-wq.c                                   |   2 +
 kernel/exit.c                                      |   5 +-
 kernel/fork.c                                      |  20 +-
 kernel/ptrace.c                                    |   6 +
 kernel/signal.c                                    |  22 +
 tools/testing/selftests/coredump/.gitignore        |   2 +
 tools/testing/selftests/coredump/Makefile          |   6 +-
 .../selftests/coredump/coredump_signal_test.c      | 238 +++++++++++
 .../selftests/coredump/coredump_worker_test.c      | 447 +++++++++++++++++++++
 13 files changed, 832 insertions(+), 74 deletions(-)
---
base-commit: dadceac9d20a1c90269aafd7746f7c0c05879ad3
change-id: 20260915-work-coredump-fixes-edf98c80fe78

[PATCH v3 01/17] 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 16b331b686fb..6c0c597ec324 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -561,6 +561,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;
@@ -569,6 +571,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 v3 02/17] 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 6c0c597ec324..40eca2b85b81 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -580,12 +580,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 e039e29cd8c5..3835fbf7d80f 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -384,6 +384,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)
 {
 	/*
@@ -393,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 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

Re: [PATCH v3 02/17] signal: only SIGKILL and the freezers interrupt a coredumping task

[Oleg Nesterov]

On 09/21, Christian Brauner wrote:
>
> 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

All of them do signal_wake_up() which sets TIF_SIGPENDING
However, zap_threads() clears this flag.

With the recent changes including your PF_NO_NOTIFY_SIGNAL and
05/17 "signal: don't retarget shared signals in a dying thread group",
what else can make signal_pending() true? Apart from SIGKILL.

So perhaps we can simply do _something_ like

	--- x/fs/coredump.c
	+++ x/fs/coredump.c
	@@ -508,7 +508,8 @@ static int zap_threads(struct task_struc
		int nr = -EAGAIN;
	 
		spin_lock_irq(&tsk->sighand->siglock);
	-	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task) {
	+	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task
	+	    && !freezing(tsk) && !(tsk->jobctl & JOBCTL_TRAP_FREEZE)) {
			/* Allow SIGKILL, see prepare_signal() */
			signal->core_state = core_state;
			nr = zap_process(signal, exit_code);
	@@ -583,7 +584,7 @@ static bool dump_interrupted(void)
		 * but then we need to teach dump_write() to restart and clear
		 * TIF_SIGPENDING.
		 */
	-	return fatal_signal_pending(current) || freezing(current);
	+	return signal_pending(current);
	 }
	 
	 static void wait_for_dump_helpers(struct file *file)

?

At first glance the change in dump_interrupted() makes sense anyway.
For example, dumping to pipe can't tolerate signal_pendin() == true.

What do you think?

> Fixes: 403bad72b67d ("coredump: only SIGKILL should interrupt the coredumping task")

Well, I think we should blame 76f969e8948d ("cgroup: cgroup v2 freezer")

Oleg.

Re: [PATCH v3 02/17] signal: only SIGKILL and the freezers interrupt a coredumping task

[Christian Brauner]

On Tue, Sep 22, 2026 at 02:55:41PM +0200, Oleg Nesterov wrote:
> On 09/21, Christian Brauner wrote:
> >
> > 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
> 
> All of them do signal_wake_up() which sets TIF_SIGPENDING
> However, zap_threads() clears this flag.
> 
> With the recent changes including your PF_NO_NOTIFY_SIGNAL and
> 05/17 "signal: don't retarget shared signals in a dying thread group",
> what else can make signal_pending() true? Apart from SIGKILL.
> 
> So perhaps we can simply do _something_ like
> 
> 	--- x/fs/coredump.c
> 	+++ x/fs/coredump.c
> 	@@ -508,7 +508,8 @@ static int zap_threads(struct task_struc
> 		int nr = -EAGAIN;
> 	 
> 		spin_lock_irq(&tsk->sighand->siglock);
> 	-	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task) {
> 	+	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task
> 	+	    && !freezing(tsk) && !(tsk->jobctl & JOBCTL_TRAP_FREEZE)) {
> 			/* Allow SIGKILL, see prepare_signal() */
> 			signal->core_state = core_state;
> 			nr = zap_process(signal, exit_code);
> 	@@ -583,7 +584,7 @@ static bool dump_interrupted(void)
> 		 * but then we need to teach dump_write() to restart and clear
> 		 * TIF_SIGPENDING.
> 		 */
> 	-	return fatal_signal_pending(current) || freezing(current);
> 	+	return signal_pending(current);
> 	 }
> 	 
> 	 static void wait_for_dump_helpers(struct file *file)
> 
> ?
> 
> At first glance the change in dump_interrupted() makes sense anyway.
> For example, dumping to pipe can't tolerate signal_pendin() == true.
> 
> What do you think?

Yes. Here's what I have in the tree.

Subject: [PATCH v4 02/17] signal: only SIGKILL and the freezers interrupt a
 coredumping task

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.

Fixes: 76f969e8948d ("cgroup: cgroup v2 freezer")
Cc: stable@vger.kernel.org
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/arJ6zZuI59BJnXpl@redhat.com
Link: https://patch.msgid.link/20260921-work-coredump-fixes-v3-2-8e4adb1619e6@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v4:
- Gate the dump in zap_threads() on freezing() and JOBCTL_TRAP_FREEZE and
  let dump_interrupted() test the flag, as suggested by Oleg.
- Drop the signal_pending() hook and coredump_signal_pending().
- Blame the cgroup v2 freezer commit as well.
- Link to v3: https://patch.msgid.link/20260921-work-coredump-fixes-v3-2-8e4adb1619e6@kernel.org

 fs/coredump.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index d88a65e37ee1..af09a4660341 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -508,7 +508,9 @@ static int zap_threads(struct task_struct *tsk,
 	int nr = -EAGAIN;
 
 	spin_lock_irq(&tsk->sighand->siglock);
-	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task) {
+	/* A freeze requested before the dump would be lost with TIF_SIGPENDING. */
+	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task &&
+	    !freezing(tsk) && !(tsk->jobctl & JOBCTL_TRAP_FREEZE)) {
 		/* Allow SIGKILL, see prepare_signal() */
 		signal->core_state = core_state;
 		nr = zap_process(signal, exit_code);
@@ -580,13 +582,8 @@ static void coredump_finish(bool core_dumped)
 
 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.
-	 */
-	return fatal_signal_pending(current) || freezing(current);
+	/* Only SIGKILL and the freezers set it after zap_threads(). */
+	return task_sigpending(current);
 }
 
 static void wait_for_dump_helpers(struct file *file)
-- 
2.53.0

[PATCH v3 04/17] 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>
Acked-by: Oleg Nesterov <oleg@redhat.com>
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 v3 05/17] 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
Acked-by: Oleg Nesterov <oleg@redhat.com>
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 v3 09/17] ptrace: refuse to change the signal mask of a user worker

[Christian Brauner]

A user worker is created with every signal other than SIGKILL and
SIGSTOP blocked. It never runs user code and that mask is what keeps
get_signal() from dequeuing anything else for it. complete_signal()
never picks a thread that blocks the signal, tkill() queues on the
worker but nothing dequeues it. ptrace_signal() requeues an injected
signal that the tracee blocks.

PTRACE_SETSIGMASK is the only way to change that mask from the outside.
A tracer that clears it makes the worker eligible for every signal.
Whatever the worker then dequeues it can only act on by leaving.

Refuse PTRACE_SETSIGMASK for a user worker with -EPERM, the way
ptrace_attach() refuses a kernel thread. A worker is guaranteed to only
ever dequeue SIGKILL or SIGSTOP and an injected signal stays pending on
it, which is what already happens when the mask isn't touched.

Taking the request and quietly leaving the mask alone was the other
option, the way set_current_blocked() keeps SIGKILL and SIGSTOP
unblocked whatever userspace asks for. But then a tracer gets success
back with nothing changed and no way to tell that apart from a mask that
took effect.

Fixes: e8b33b8cfafc ("Revert "kernel: treat PF_IO_WORKER like PF_KTHREAD for ptrace/signals"")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 kernel/ptrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index d041645d9d17..4e9822a87aab 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -1227,6 +1227,12 @@ int ptrace_request(struct task_struct *child, long request,
 	case PTRACE_SETSIGMASK: {
 		sigset_t new_set;
 
+		/* A user worker only ever takes SIGKILL and SIGSTOP. */
+		if (child->flags & PF_USER_WORKER) {
+			ret = -EPERM;
+			break;
+		}
+
 		if (addr != sizeof(sigset_t)) {
 			ret = -EINVAL;
 			break;

-- 
2.53.0

Re: [PATCH v3 09/17] ptrace: refuse to change the signal mask of a user worker

[Oleg Nesterov]

On 09/21, Christian Brauner wrote:
>
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -1227,6 +1227,12 @@ int ptrace_request(struct task_struct *child, long request,
>  	case PTRACE_SETSIGMASK: {
>  		sigset_t new_set;
>
> +		/* A user worker only ever takes SIGKILL and SIGSTOP. */
> +		if (child->flags & PF_USER_WORKER) {
> +			ret = -EPERM;
> +			break;
> +		}

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

[PATCH v3 12/17] exec: cancel io_uring requests before de_thread()

[Christian Brauner]

begin_new_exec() cancels the exec'ing thread's io_uring requests after
de_thread() made it the only thread in the thread-group. Now
io_uring_task_cancel() runs task work after. If it runs a
create_worker_cb() item then a io-wq queued on the thread before the
exec creates a new io-wq worker thread.

Said thread joins the thread-group which de_thread() just took down. The
io-iwq sticks around until it notices that its workqueue is dying. But
code after de_thread() relies on being single-threaded.

Move io_uring_task_cancel() before de_thread() but behind the point of
no return. Any worker created during task work run from
io_uring_task_cancel() is just a sibling thread that de_thread() will
take down and reap.

Once io_uring_task_cancel() returned the thread has neither a task
context nor a workqueue left so nothing can add a thread behind
de_thread()'s back anymore.

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/aq0_lvHDViIpX_Mu@redhat.com
Fixes: 3bfe6106693b ("io-wq: fork worker threads from original task")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/exec.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 075a744421e1..334e6d358c56 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1149,16 +1149,19 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 */
 	bprm->point_of_no_return = true;
 
+	/*
+	 * Cancel any io_uring activity across execve. This runs task work
+	 * that may still create an io-wq worker, so do it while de_thread()
+	 * can still zap it.
+	 */
+	io_uring_task_cancel();
+
 	/* Make this the only thread in the thread group */
 	retval = de_thread(me);
 	if (retval)
 		goto out;
 	/* see the comment in check_unsafe_exec() */
 	current->fs->in_exec = 0;
-	/*
-	 * Cancel any io_uring activity across execve
-	 */
-	io_uring_task_cancel();
 
 	/* Ensure the files table is not shared. */
 	retval = unshare_fd(CLONE_FILES, &files);

-- 
2.53.0

Re: [PATCH v3 12/17] exec: cancel io_uring requests before de_thread()

[Oleg Nesterov]

On 09/21, Christian Brauner wrote:
>
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1149,16 +1149,19 @@ int begin_new_exec(struct linux_binprm * bprm)
>  	 */
>  	bprm->point_of_no_return = true;
>  
> +	/*
> +	 * Cancel any io_uring activity across execve. This runs task work
> +	 * that may still create an io-wq worker, so do it while de_thread()
> +	 * can still zap it.
> +	 */
> +	io_uring_task_cancel();

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

Re: [PATCH v3 12/17] exec: cancel io_uring requests before de_thread()

[Jens Axboe]

On 9/21/26 7:45 AM, Christian Brauner wrote:
> begin_new_exec() cancels the exec'ing thread's io_uring requests after
> de_thread() made it the only thread in the thread-group. Now
> io_uring_task_cancel() runs task work after. If it runs a
> create_worker_cb() item then a io-wq queued on the thread before the
> exec creates a new io-wq worker thread.
> 
> Said thread joins the thread-group which de_thread() just took down. The
> io-iwq sticks around until it notices that its workqueue is dying. But
> code after de_thread() relies on being single-threaded.
> 
> Move io_uring_task_cancel() before de_thread() but behind the point of
> no return. Any worker created during task work run from
> io_uring_task_cancel() is just a sibling thread that de_thread() will
> take down and reap.
> 
> Once io_uring_task_cancel() returned the thread has neither a task
> context nor a workqueue left so nothing can add a thread behind
> de_thread()'s back anymore.

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

-- 
Jens Axboe

[PATCH v3 14/17] fork: don't create io threads once PF_POSTCOREDUMP is set

[Christian Brauner]

zap_process() skips every thread that already has PF_POSTCOREDUMP set.
Such a thread is past synchronize_group_exit(), so a coredump can't
catch it anymore. It isn't counted in core_state->threads_remaining and
it isn't sent SIGKILL.

do_exit() sets PF_POSTCOREDUMP in synchronize_group_exit() and calls
io_uring_files_cancel() right after that. The cancellation runs task
work and a create_worker_cb() that io-wq queued before the exit creates
a new io-wq worker from there. If another thread started a coredump in
the meantime that worker joins a thread-group which is already being
dumped. zap_process() never saw it so it was never counted. But when it
exits it sees signal->core_state in synchronize_group_exit() and
decrements threads_remaining like any other thread.

So the count reaches zero one thread early and the dumper leaves
coredump_wait_inactive() while a thread it counted is still running.

The task has no fatal signal pending because zap_process() deliberately
didn't send it one, and it never went through get_signal() so it doesn't
have PF_SIGNALED either.

Refuse to create an io thread when the creator has PF_POSTCOREDUMP.
That costs nothing. io_uring_files_cancel() raises IO_WQ_BIT_EXIT
before it runs any task work, so a worker created from there only ever
gets to exit again, and io_should_retry_thread() doesn't retry -EINTR.

Clearing PF_POSTCOREDUMP for the new thread in copy_process() was the
other option. It makes the worker a thread like any other, but it only
helps while the dump hasn't started. A worker born after zap_process()
has run is invisible to it whatever its flags say and still decrements
threads_remaining on the way out. Refusing to create it covers both, and
then no task is ever born with the flag, so there is nothing to clear.

Moving io_uring_files_cancel() ahead of synchronize_group_exit() closes
the same window. It runs the cancellation, and whatever that can block
on, before the thread announces itself to the dumper. A thread that
blocks there never reaches coredump_task_exit() at all, so the dumper
ends up waiting for a thread that never parks. Refusing the creation
leaves the cancellation where it is. But it's very ugly to run io_uring
work even before we did all the generic exit work.

Fixes: 92307383082d ("coredump:  Don't perform any cleanups before dumping core")
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 ede9f02bef47..fd2829b0a8b6 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2706,8 +2706,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);

-- 
2.53.0

Re: [PATCH v3 14/17] fork: don't create io threads once PF_POSTCOREDUMP is set

[Oleg Nesterov]

On 09/21, Christian Brauner wrote:
>
> Refuse to create an io thread when the creator has PF_POSTCOREDUMP.
> That costs nothing. io_uring_files_cancel() raises IO_WQ_BIT_EXIT
> before it runs any task work, so a worker created from there only ever
> gets to exit again, and io_should_retry_thread() doesn't retry -EINTR.

Thanks for this note in the changelog,

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2706,8 +2706,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);

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