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

[Oleg Nesterov <oleg@redhat.com>]

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.

Oleg.

On 07/03, Eric W. Biederman wrote:
>
> Oleg's recent patchset tweaking how force_sig_info works has inspired me
> to finally push through and update the signal handling to have proper
> short circuit deliver for coredump signals.  Everything is just simpler
> when coredumps are not such a large special case.
>
> What makes this tricky is coredumps have had their own process
> shoot-down logic similar to but separate and different from everything
> else in the kernel.  The bulk of this set of changes is merging the
> process shoot-down logic that is used for signals and the logic for
> coredumps.  So the same process shoot-down logic can be shared.
>
> With the shoot-down logic sorted the rest is quite straight forward.
>
> Oleg when reviewing the first version of this set of changes noticed
> that dequeue_exit_signal did not properly handle thread local signal
> that trigger a coredump.  To resolve this I have added a few more
> cleanups so that I can detect a fatal signal as it is being enqueued
> and place it in the shared signal queue.
>
> One of those cleanups is a rewrite of detecting if a signal can be
> delivered immediately when sent aka short circuit delivery.  The
> processing of signals that will be ignored and signals that will cause a
> process to exit without returning to userspace (such as SIGKILL) are
> both enhanced.
>
> This set of changes is against v7.2-rc1
>
>  fs/coredump.c                          | 161 +++++++++++++++++----------------
>  include/linux/coredump.h               |   4 +
>  include/linux/sched/signal.h           |   2 +
>  include/linux/sched/task.h             |   1 -
>  include/linux/signal_types.h           |   3 -
>  include/uapi/asm-generic/signal-defs.h |   1 -
>  kernel/exit.c                          |  41 ++-------
>  kernel/signal.c                        | 119 +++++++++++++++---------
>  mm/oom_kill.c                          |   2 +-
>  9 files changed, 171 insertions(+), 163 deletions(-)
>
> Eric W. Biederman (14):
>       signal: Generalize posixtimer_queue_sigqueue into enqueue_signal
>       signal: Factor out sig_blocked from sig_ignored
>       signal: More accurate ignoring of signals based on sig_can_short_circuit
>       signal: Use sig_can_short_circuit to improve fatal signal delivery
>       signal: Compute the exit_code in get_signal
>       signal: In get_signal call do_exit when it is unnecessary to shoot down threads
>       signal: Bring down all threads when handling a non-coredump fatal signal
>       signal: Move stopping for the coredump from do_exit into get_signal
>       signal: Move audit_core_dumps from do_coredump into get_signal
>       coredump: In zap_threads complete startup if there is no need to wait
>       signal: Use the thread killing in get_signal for coredumps
>       exit: Make do_group_exit static
>       signal: Dequeue fatal signals
>       signal: Short circuit deliver coredump signals
>
>  fs/coredump.c                | 153 ++++++++++++++-------------
>  include/linux/coredump.h     |   4 +
>  include/linux/sched/signal.h |   2 +
>  include/linux/sched/task.h   |   1 -
>  kernel/exit.c                |  41 ++------
>  kernel/signal.c              | 246 +++++++++++++++++++++++++++++--------------
>  mm/oom_kill.c                |   2 +-
>  7 files changed, 260 insertions(+), 189 deletions(-)
>