Re: [PATCH v2 00/14] Short circuit delivery for coredump signals

[Eric W. Biederman]

Oleg Nesterov <oleg@xxxxxxxxxx> writes:

> Eric, so far I applied the whole series, and I don't see how it can
> solve one of the problems I tried to fix in my series.
>
> Again. A task T has a pending and blocked SIGSYS. si_code = SI_USER.
> (although the latter is not strictly necessary)
>
> force_sig_seccomp(force_coredump => true) sent to T unblocks SIGSYS
> and sets SA_IMMUTABLE.
>
> However, __send_signal_locked() will bypass enqueue_signal() (so it
> won't set SIGNAL_GROUP_EXIT) because legacy_queue() is true.

This very much causes the synchronous signal not to be treated as
synchronous and that is a real problem.

I want to say more but don't have time now.  Hopefully tomorrow.


> T calls get_signal(). Now. it can dequeue another synchronous signal.
> If that signal has a handler and its sa_mask includes SIGSYS, the task
> can return to userspace and survive.
>
> No?
>
> Oh... And SIGKILL still can be lost, and I still think this is not
> good.

Two things on that score.  According to my notes this fixes the much
more problematic issue that coredumps of processes using io_uring have
problems.  With this change io_uring processes act just like everything
else so those problems go away.  This should allow effectively reverting
commit 06af8679449d ("coredump: Limit what can interrupt coredumps").

If we want SIGKILL to be logically processed before coredump signal we
can do:

 kernel/signal.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index bf45ebeb7940..048520dfcef4 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -951,6 +951,16 @@ static bool prepare_signal(int sig, struct task_struct *p,
 	if (signal->flags & SIGNAL_GROUP_EXIT) {
 		if (signal->core_state)
 			return sig == SIGKILL;
+		else if ((signal->flags & SIGNAL_EXIT_DEQUEUE) &&
+			 (sig == SIGKILL) &&
+			 sig_kernel_coredump(signal->group_exit_code)) {
+			/* Let a SIGKILL before a coredump begins preempt
+			 * a coredump signal that is about to happen.
+			 */
+			signalfd_notify(p, SIGKILL);
+			signal->group_exit_code = SIGKILL;
+			sigaddset(&signal->shared_pending.signal, SIGKILL);
+		}
 		/*
 		 * The process is in the middle of dying, drop the signal.
 		 */
-- 
2.41.0


Eric