On 02/12, Eric W. Biederman wrote: > > > Here I was trying for the simple minimal change and I hit this landmine. > > Which leaves me with the question of what should be semantics of signal > > handling after exit.
Yes, currently it is undefined. Even signal_pending() is random. > > I think from dim memory of previous conversations the desired semantics > > look like: > > a) Ignore all signal state except for SIGKILL. > > b) Letting SIGKILL wake up the process should be sufficient. signal_wake_up(true) to make fatal_signal_pending() == T, I think. > Oleg any ideas on how to make PTRACE_EVENT_EXIT reliably killable? My answer is very simple: PTRACE_EVENT_EXIT must not stop if the tracee was killed by the "real" SIGKILL (not by group_exit/etc), that is all. But this is another user-visible change, it can equally confuse, say, strace (albeit not too much iiuc). But this needs another discussion. > diff --git a/kernel/signal.c b/kernel/signal.c > index 99fa8ff06fd9..a1f154dca73c 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -2544,6 +2544,9 @@ bool get_signal(struct ksignal *ksig) > } > > fatal: > + /* No more signals can be pending past this point */ > + sigdelset(¤t->pending.signal, SIGKILL); Well, this is very confusing. In fact, this is not really correct. Say, we should not remove the pending SIGKILL if we are going to call do_coredump(). This is possible if ptrace_signal() was called, or after is_current_pgrp_orphaned() returns false. > + clear_tsk_thread_flag(current, TIF_SIGPENDING); I don't understand this change, it looks irrelevant. Possibly makes sense, but this connects to "semantics of signal handling after exit". OK, we need a minimal incremental fix for now. I'd suggest to replace ksig->info.si_signo = signr = SIGKILL; if (signal_group_exit(signal)) goto fatal; added by this patch with if (__fatal_signal_pending(current)) { ksig->info.si_signo = signr = SIGKILL; sigdelset(¤t->pending.signal, SIGKILL); goto fatal; } __fatal_signal_pending() is cheaper and looks more understandable. Oleg.