signal: Remove the helper signal_group_exit

This helper is misleading.  It tests for an ongoing exec as well as
the process having received a fatal signal.

Sometimes it is appropriate to treat an on-going exec differently than
a process that is shutting down due to a fatal signal.  In particular
taking the fast path out of exit_signals instead of retargeting
signals is not appropriate during exec, and not changing the the exit
code in do_group_exit during exec.

Removing the helper makes it more obvious what is going on as both
cases must be coded for explicitly.

While removing the helper fix the two cases where I have observed
using signal_group_exit resulted in the wrong result.

In exit_signals only test for SIGNAL_GROUP_EXIT so that signals are
retargetted during an exec.

In do_group_exit use 0 as the exit code during an exec as de_thread
does not set group_exit_code.  As best as I can determine
group_exit_code has been is set to 0 most of the time during
de_thread.  During a thread group stop group_exit_code is set to the
stop signal and when the thread group receives SIGCONT group_exit_code
is reset to 0.

Link: https://lkml.kernel.org/r/20211213225350.27481-8-ebiederm@xmission.com
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

diff --git a/fs/coredump.c b/fs/coredump.c
index c92ffc0bf2c2837467679411d824cff02a6f3270..7dece20b162b38fc6215f3ae38a4f64d62179d50 100644 (file)
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -372,11 +372,12 @@ static int zap_process(struct task_struct *start, int exit_code)
 static int zap_threads(struct task_struct *tsk,
                        struct core_state *core_state, int exit_code)
 {
+       struct signal_struct *signal = tsk->signal;
        int nr = -EAGAIN;
 
        spin_lock_irq(&tsk->sighand->siglock);
-       if (!signal_group_exit(tsk->signal)) {
-               tsk->signal->core_state = core_state;
+       if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task) {
+               signal->core_state = core_state;
                nr = zap_process(tsk, exit_code);
                clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
                tsk->flags |= PF_DUMPCORE;

diff --git a/fs/exec.c b/fs/exec.c
index 9d2925811011657904bd8068ab69cff4a135c586..82db656ca709d6f1d4ef982181086c1a62b94a2c 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1045,7 +1045,7 @@ static int de_thread(struct task_struct *tsk)
         * Kill all other threads in the thread group.
         */
        spin_lock_irq(lock);
-       if (signal_group_exit(sig)) {
+       if ((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task) {
                /*
                 * Another group action in progress, just
                 * return so that the signal is processed.

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index d3248aba5183d48be6b1ebff95a1708059c5b16f..b6ecb9fc4cd2d76123aa69c00995844875e689ea 100644 (file)
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -271,13 +271,6 @@ static inline void signal_set_stop_flags(struct signal_struct *sig,
        sig->flags = (sig->flags & ~SIGNAL_STOP_MASK) | flags;
 }
 
-/* If true, all threads except ->group_exec_task have pending SIGKILL */
-static inline int signal_group_exit(const struct signal_struct *sig)
-{
-       return  (sig->flags & SIGNAL_GROUP_EXIT) ||
-               (sig->group_exec_task != NULL);
-}
-
 extern void flush_signals(struct task_struct *);
 extern void ignore_signals(struct task_struct *);
 extern void flush_signal_handlers(struct task_struct *, int force_default);

diff --git a/kernel/exit.c b/kernel/exit.c
index b05578abbf261a75ecdb252947ab523ed1457008..861cfb1e2f779a4e76925bc2909c77e6b9ad75a0 100644 (file)
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -914,15 +914,19 @@ do_group_exit(int exit_code)
 
        BUG_ON(exit_code & 0x80); /* core dumps don't get here */
 
-       if (signal_group_exit(sig))
+       if (sig->flags & SIGNAL_GROUP_EXIT)
                exit_code = sig->group_exit_code;
+       else if (sig->group_exec_task)
+               exit_code = 0;
        else if (!thread_group_empty(current)) {
                struct sighand_struct *const sighand = current->sighand;
 
                spin_lock_irq(&sighand->siglock);
-               if (signal_group_exit(sig))
+               if (sig->flags & SIGNAL_GROUP_EXIT)
                        /* Another thread got here before we took the lock.  */
                        exit_code = sig->group_exit_code;
+               else if (sig->group_exec_task)
+                       exit_code = 0;
                else {
                        sig->group_exit_code = exit_code;
                        sig->flags = SIGNAL_GROUP_EXIT;

diff --git a/kernel/signal.c b/kernel/signal.c
index bae231bc2f4a284bf8796e9795561b2846e07398..167b8e196a79c4ed6d5debf8f20febf83e9f0b0f 100644 (file)
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2386,7 +2386,8 @@ static bool do_signal_stop(int signr)
                WARN_ON_ONCE(signr & ~JOBCTL_STOP_SIGMASK);
 
                if (!likely(current->jobctl & JOBCTL_STOP_DEQUEUED) ||
-                   unlikely(signal_group_exit(sig)))
+                   unlikely(sig->flags & SIGNAL_GROUP_EXIT) ||
+                   unlikely(sig->group_exec_task))
                        return false;
                /*
                 * There is no group stop already in progress.  We must
@@ -2693,7 +2694,8 @@ relock:
                enum pid_type type;
 
                /* Has this task already been marked for death? */
-               if (signal_group_exit(signal)) {
+               if ((signal->flags & SIGNAL_GROUP_EXIT) ||
+                    signal->group_exec_task) {
                        ksig->info.si_signo = signr = SIGKILL;
                        sigdelset(&current->pending.signal, SIGKILL);
                        trace_signal_deliver(SIGKILL, SEND_SIG_NOINFO,
@@ -2949,7 +2951,7 @@ void exit_signals(struct task_struct *tsk)
         */
        cgroup_threadgroup_change_begin(tsk);
 
-       if (thread_group_empty(tsk) || signal_group_exit(tsk->signal)) {
+       if (thread_group_empty(tsk) || (tsk->signal->flags & SIGNAL_GROUP_EXIT)) {
                tsk->flags |= PF_EXITING;
                cgroup_threadgroup_change_end(tsk);
                return;