[PATCH v2 0/5] Stop TIF_NOTIFY_SIGNAL from interrupting work that can't be restarted

[PATCH v2 0/5] Stop TIF_NOTIFY_SIGNAL from interrupting work that can't be restarted

[Christian Brauner]

Ok, so I was looking into things and as usual got
side-tracked so here we are. Oleg, save me please.

TIF_NOTIFY_SIGNAL is used to kick a task in uninterruptible sleep to
return to userspace and run task work and then go back to sleep. This
mechanism works well but breaks coredumps. dump_interrupted() only
allows fatal signals to interrupt a coredump and the whole regular write
path going to actual filesystems is impervious to TIF_NOTIFY_SIGNAL as
well.

The core is that you can have quite deep callchains that end up calling
signal_pending() in both the pipe and the socket codepaths so it's like
we can just pass a flag through somehow.

For coredumps its very annoying because it means io_uring is - depending
on how much outstanding work you have - incompatible with generating
non-truncated coredumps. A process with too many file backed mappings
and io_uring requests in flight ends up losing most of the coredump.

While zap_threads() has cleared TIF_SIGPENDING for a long time, just
clearing TIF_NOTIFY_SIGNAL isn't going to work because the next
completion will just set it again.

The fun part also is that io_uring isn't actually the only case:

(1) io_uring

(2) klp_send_signals()

(3) bpf_task_work_schedule_signal()

(4) landlock's tsync

And technically, kthread_stop() and the printk kunit test set the bit
raw. So no simple way of just fixing this in one subsystem.

So, a fix for this issue has the following constraints:

(i) The places where a write is aborted are deep callchains that we
    can't reasonably parameterize. For example, anon_pipe_write(),
    unix_stream_sendmsg(), unix_stream_read_generic(),
    sk_stream_wait_memory(), or a bare wait_event_interruptible() in
    wait_for_dump_helpers(). All of them are shared with regular
    syscalls that must stay interruptible.

    IOW, the state has to be per-task and ambient.

    An LLM would call this "load bearing"...

(ii) There are multiple ways TIF_NOTIFY_SIGNAL can get raised and they
     can get set from irq context against any task. As said above we
     have at least io_uring paths (poll task_work, msg_ring, tctx exit,
     io-wq via __set_notify_signal()), bpf_task_work_schedule_signal(),
     klp_send_signals(), landlock tsync, plus kthread_stop() and the
     printk kunit stuff that set the bit raw.

     So fixing this up in the individual subsystems is doomed to fail or
     require constant audits in case some new variant shows up.

(iii) The coredump task is exiting and can't restart the work.

So here's some stuff that was considered but I think is not really
feasible:

(a) Check for task_is_coredumping(). That will end up forcing
    task_work_add() users to know about coredumps. This seems like a
    layering violation.

    I have a patch for this as well but it's ugly. It races with the
    dump starting unless the bit setter takes a lock in the io_uring
    completion. I know someone that will disagree with this approach. ;)

(b) Oleg's old suggestion iirc. Just clear the bit at coredump entry.
    That doesn't work because io_uring poll completions just raise it
    again from irq context. So you also need synchronization with the
    setter of which there are quite a few.

(c) Let the setting task defer setting the bit if the task is flagged
    and then raise it again at exit.

(d) Take TIF_NOTIFY_SIGNAL out of singal_pending() and make it opt-in at
    specific points. That breaks io_uring quite badly and forces a
    tree-wide audit.

So the amount of patches for this issue over the years is impressive. So
let me add one to the pile for the lolz.

Add PF_NO_NOTIFY_SIGNAL and helpers to raise/restore it. This is the
same approach as memalloc_nofs_save(). signal_pending() will not report
a fake pending signal via TIF_NOTIFY_SIGNAL if inside a
PF_NO_NOTIFY_SIGNAL critical section. Obviously you can't
fork()/clone3() in such a section.

Fix coredumps, smb, and pid namespace busy-reaping. Fwiw, I think
there's a few other potential users of the helpers that are left out of
this series.

|         |    notify_signal_pipe     |   notify_signal_socket    |
|---------|---------------------------|---------------------------|
| fix     | ok                        | ok                        |
|---------|---------------------------|---------------------------|
| unfixed | 357324 of ≥ 5111808 bytes | 467336 of ≥ 5111808 bytes |

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- no_notify_signal_save() now returns the unmasked current->flags,
  current_restore_flags() masks on restore (Oleg)
- Add an irqsave-style no_notify_signal guard on top of the helpers
  and use it at all three sites
- New patch: pid_namespace: prevent TIF_NOTIFY_SIGNAL from
  interrupting the reaper (suggested by Oleg)
- selftests: unlink stale core file and socket in the fixture setup so
  a killed previous run can't fake a result; assert the exact expected
  dump size
- Add Fixes:/Cc: stable tags
- Carried Paulo's Acked-by on the smb patch over the mechanical
  conversion to the scoped guard
- Link to v1: https://patch.msgid.link/20260818-work-tif_notify_signal-v1-0-1ee1fcc5b3ff@kernel.org

---
Christian Brauner (5):
      signal: allow taks to temporarily block TIF_NOTIFY_SIGNAL
      coredump: prevent TIF_NOTIFY_SIGNAL from interrupting coredumps
      selftests/coredump: test that TIF_NOTIFY_SIGNAL doesn't truncate a coredump
      smb: prevent TIF_NOTIFY_SIGNAL from interrupting
      pid_namespace: prevent TIF_NOTIFY_SIGNAL from interrupting the reaper

 fs/coredump.c                                      |   2 +
 fs/smb/client/transport.c                          |  13 +-
 include/linux/sched.h                              |   2 +-
 include/linux/sched/signal.h                       |  27 +-
 kernel/pid_namespace.c                             |   3 +-
 tools/testing/selftests/coredump/Makefile          |   7 +-
 .../selftests/coredump/coredump_notify_signal.h    |  29 ++
 .../coredump/coredump_notify_signal_helper.c       |  46 +++
 .../coredump/coredump_notify_signal_test.c         | 245 ++++++++++++++++
 tools/testing/selftests/coredump/coredump_test.h   |   1 +
 .../selftests/coredump/coredump_test_helpers.c     | 311 +++++++++++++++++++++
 11 files changed, 672 insertions(+), 14 deletions(-)
---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260817-work-tif_notify_signal-6ab080d33693

[PATCH v2 1/5] signal: allow taks to temporarily block TIF_NOTIFY_SIGNAL

[Christian Brauner]

TIF_NOTIFY_SIGNAL is used to kick a task in uninterruptible sleep to
return to userspace and run task work and then go back to sleep. This
mechanism works well but breaks e.g., coredumps. dump_interrupted() only
allows fatal signals to interrupt a coredump and the whole regular write
path going to actual filesystems is impervious to TIF_NOTIFY_SIGNAL as
well.

Add PF_NO_NOTIFY_SIGNAL and helpers to raise and restore it. This is the
same approach as memalloc_nofs_save(). signal_pending() will not report
a fake pending signal via TIF_NOTIFY_SIGNAL if inside a
PF_NO_NOTIFY_SIGNAL section.

No functional changes.

Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 include/linux/sched.h        |  2 +-
 include/linux/sched/signal.h | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 3f100d69b053..82b43f20218a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1807,7 +1807,7 @@ extern struct pid *cad_pid;
 						 * I am cleaning dirty pages from some other bdi. */
 #define PF_KTHREAD		0x00200000	/* I am a kernel thread */
 #define PF_RANDOMIZE		0x00400000	/* Randomize virtual address space */
-#define PF__HOLE__00800000	0x00800000
+#define PF_NO_NOTIFY_SIGNAL	0x00800000	/* see no_notify_signal_save() */
 #define PF__HOLE__01000000	0x01000000
 #define PF__HOLE__02000000	0x02000000
 #define PF_NO_SETAFFINITY	0x04000000	/* Userland is not allowed to meddle with cpus_mask */
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 584ae88b435e..6164995da980 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_SCHED_SIGNAL_H
 #define _LINUX_SCHED_SIGNAL_H
 
+#include <linux/cleanup.h>
 #include <linux/rculist.h>
 #include <linux/signal.h>
 #include <linux/sched.h>
@@ -384,14 +385,36 @@ static inline int task_sigpending(struct task_struct *p)
 	return unlikely(test_tsk_thread_flag(p,TIF_SIGPENDING));
 }
 
+/* Prevent TIF_NOTIFY_SIGNAL from interrupting this task. */
+static inline unsigned int no_notify_signal_save(void)
+{
+	unsigned int flags = current->flags;
+
+	current->flags |= PF_NO_NOTIFY_SIGNAL;
+	return flags;
+}
+
+/* Restore the previous PF_NO_NOTIFY_SIGNAL state. */
+static inline void no_notify_signal_restore(unsigned int flags)
+{
+	current_restore_flags(flags, PF_NO_NOTIFY_SIGNAL);
+}
+
+DEFINE_LOCK_GUARD_0(no_notify_signal,
+		    _T->flags = no_notify_signal_save(),
+		    no_notify_signal_restore(_T->flags),
+		    unsigned int flags)
+
 static inline int signal_pending(struct task_struct *p)
 {
 	/*
 	 * TIF_NOTIFY_SIGNAL isn't really a signal, but it requires the same
 	 * behavior in terms of ensuring that we break out of wait loops
-	 * so that notify signal callbacks can be processed.
+	 * so that notify signal callbacks can be processed. Not for a task
+	 * that asked not to be interrupted by it, see no_notify_signal_save().
 	 */
-	if (unlikely(test_tsk_thread_flag(p, TIF_NOTIFY_SIGNAL)))
+	if (unlikely(test_tsk_thread_flag(p, TIF_NOTIFY_SIGNAL)) &&
+	    likely(!(READ_ONCE(p->flags) & PF_NO_NOTIFY_SIGNAL)))
 		return 1;
 	return task_sigpending(p);
 }

-- 
2.53.0

[PATCH v2 2/5] coredump: prevent TIF_NOTIFY_SIGNAL from interrupting coredumps

[Christian Brauner]

TIF_NOTIFY_SIGNAL is used to kick a task in uninterruptible sleep to
return to userspace and run task work and then go back to sleep. This
mechanism works well but breaks coredumps. dump_interrupted() only
allows fatal signals to interrupt a coredump and the whole regular write
path going to actual filesystems is impervious to TIF_NOTIFY_SIGNAL as
well.

However, both the usermodehelper pipe and the coredump socket will bail
early on TIF_NOTIFY_SIGNAL. This affects the following codepaths:

- coredump_sock_recv() -> unix_stream_read_generic()
  The request/ack handshake is abandoned before any coredump data is
  sent.

- anon_pipe_write() returning -ERESTARTSYS
  Once the pipe is full this truncates the coredump.

- unix_stream_sendmsg() returning -ERESTARTSYS
  Once the send buffer is full this truncates the coredump.

- coredump_sock_wait() -> __kernel_read()
  This reports a failure that didn't happen.

- wait_for_dump_helpers() -> wait_event_interruptible()
  This stops waiting for the coredump helpers.

Such truncation is entirely invisible to userspace and all uapi bits
still indicate that a successful coredump happened. A crashing process
with a bunch of file backed mappings and io_uring thrown in loses most
of the coredump data. If the NT_FILE note goes past PAGE_SIZE mappings
it's gonzo.

TIF_NOTIFY_SIGNAL is sent by io_uring for the common case. And it uses
poll without sleeping so a completion callback runs task_work_add() from
interrupt context against the task that submitted the request. This is
the task that is running the coredump. Since that task hasn't set
work_exited (it hasn't exited yet after all) TIF_NOTIFY_SIGNAL keeps
reappearing.

A coredumping task doesn't return to userspace. The task work is run at
exit. So interrupting it doesn't buy anything and just loses the
coredump which is quite valuable.

Note that this isn't specific to io_uring. There's also
klp_send_signals(), bpf_task_work_schedule_signal(), landlock's tsync
and then  technically, kthread_stop() and the printk kunit test set the
bit raw.

Fixes: 12db8b690010 ("entry: Add support for TIF_NOTIFY_SIGNAL")
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 ac3cd74808c6..7c0f276c3e0d 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1191,6 +1191,8 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
 	if (coredump_wait(siginfo->si_signo, &core_state) < 0)
 		return;
 
+	/* Task work must not cut the dump short, see signal_pending(). */
+	guard(no_notify_signal)();
 	scoped_with_creds(cred)
 		do_coredump(&cn, &cprm, &argv, &argc, binfmt);
 	coredump_cleanup(&cn, &cprm);

-- 
2.53.0

[PATCH v2 5/5] pid_namespace: prevent TIF_NOTIFY_SIGNAL from interrupting the reaper

[Christian Brauner]

Since commit 7fea700e04bd ("zap_pid_ns_processes: clear
TIF_NOTIFY_SIGNAL along with TIF_SIGPENDING") zap_pid_ns_processes()
clears TIF_NOTIFY_SIGNAL before every kernel_wait4() call. Otherwise
kernel_wait4() doesn't sleep and returns -ERESTARTSYS if
signal_pending() is true and the reaper busy-waits.

But clearing TIF_NOTIFY_SIGNAL doesn't fix it. It can get raised again.
For example, klp_send_signals() sets it raw against any task that hasn't
transitioned yet. And it keeps sending it until the livepatch finishes.

The sleep loop waiting for pid_allocated to drop has the same problem
and it doesn't even clear anything. So a schedule() in
TASK_INTERRUPTIBLE returns immediately.

Use a no_notify_signal guard. signal_pending() will ignore
TIF_NOTIFY_SIGNAL and the pid namespace reaper sleeps until a child
exits or free_pid() wakes it.

Fixes: 7fea700e04bd ("zap_pid_ns_processes: clear TIF_NOTIFY_SIGNAL along with TIF_SIGPENDING")
Cc: stable@vger.kernel.org
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 kernel/pid_namespace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index d36afc58ee1d..8bc9edb40b78 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -238,9 +238,10 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	 * kernel_wait4() will also block until our children traced from the
 	 * parent namespace are detached and become EXIT_DEAD.
 	 */
+	/* Task work must not busy-loop the reaper, see signal_pending(). */
+	guard(no_notify_signal)();
 	do {
 		clear_thread_flag(TIF_SIGPENDING);
-		clear_thread_flag(TIF_NOTIFY_SIGNAL);
 		rc = kernel_wait4(-1, NULL, __WALL, NULL);
 	} while (rc != -ECHILD);
 

-- 
2.53.0

Re: [PATCH v2 0/5] Stop TIF_NOTIFY_SIGNAL from interrupting work that can't be restarted

[Oleg Nesterov]

On 08/24, Christian Brauner wrote:
>
> Christian Brauner (5):
>       signal: allow taks to temporarily block TIF_NOTIFY_SIGNAL
>       coredump: prevent TIF_NOTIFY_SIGNAL from interrupting coredumps
>       selftests/coredump: test that TIF_NOTIFY_SIGNAL doesn't truncate a coredump
>       smb: prevent TIF_NOTIFY_SIGNAL from interrupting
>       pid_namespace: prevent TIF_NOTIFY_SIGNAL from interrupting the reaper

For 1-2 and 4-5 feel free to add

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

-------------------------------------------------------------------------------
sigprocmask(SIG_BLOCK) in fs/smb/client/transport.c:__smb_send_rqst() looks
a bit suspicious. Even if we forget about TIF_NOTIFY_SIGNAL, sigprocmask()
doesn't necessarily clear TIF_SIGPENDING, nor it can protect from (say)
do_signal_stop()->signal_wake_up() from a sub-thread. But this is offtopic.

Oleg.