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

[Eric W. Biederman]

Oleg Nesterov <oleg@xxxxxxxxxx> writes:

> On 07/07, Oleg Nesterov wrote:
>>
>> 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.
>>
>> 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.
>
> Hmm. And it seems that V2 doesn't fix the problem I noticed in V1: the dumper
> thread can be wrong.
>
> The first thread which calls get_signal() -> dequeue_exit_signal() will
> initiate the coredumping, not necessary the faulting thread. This looks
> very wrong to me.

The problem was that dequeue_exit_signal would read the exit signal
from the per thread queue.

I solved that by always placing the exit signal on the shared_pending
queue (even when it would normally not be on the shared_pending queue).

With the signal on the shared_pending queue any thread can initiate the
core dump without problems.  Or am I missing something where it still
matters?

I can see where it would be nice in the coredump to know which thread
triggered things, but I don't remember anything  actually caring.

I have just skimmed through the code looking.   The only thing
I can see that would actually be effected are "%i" and "%I" in
the coredump pattern.

The only other effect I can see is that the order of the threads notes
as written into the coredump with fill_note_info will be different.
Perhaps a debugger takes that as a signal to report which thread has
died?  If something actually cares that is worth fixing.  Do you know
of anything that actually cares?

Eric