lkml.org 
[lkml]   [2015]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH v2 2/3] signal: kill the obsolete SIGNAL_UNKILLABLE check in complete_signal()
    complete_signal() checks SIGNAL_UNKILLABLE before it starts to destroy the
    thread group, today this is unnecessary and even not 100% correct.

    After the previous change we can rely on sig_task_ignored(); sig_fatal(sig)
    && SIGNAL_UNKILLABLE can only be true if we actually want to kill this task.

    And it does not look right. fatal_signal_pending() should always imply that
    the whole thread group (except ->group_exit_task if it is not NULL) is killed,
    this check breaks the rule.

    This explains WARN_ON(!JOBCTL_STOP_PENDING) in task_participate_group_stop()
    triggered by the test-case from Dmitry:

    int main()
    {
    int pid = 1;
    ptrace(PTRACE_ATTACH, pid, 0, 0);
    ptrace(PTRACE_SETOPTIONS, pid, 0, PTRACE_O_EXITKILL);
    sleep(1);
    return 0;
    }

    do_signal_stop()->signal_group_exit() returns false because SIGNAL_GROUP_EXIT
    is not set, but task_set_jobctl_pending() checks fatal_signal_pending() and
    does not set JOBCTL_STOP_PENDING.

    The test-case above needs root and (correctly) crashes the kernel, but we can
    trigger the same warning inside the container or using another test-case:

    static int init(void *arg)
    {
    for (;;)
    pause();
    }

    int main(void)
    {
    char stack[16 * 1024];

    for (;;) {
    int pid = clone(init, stack + sizeof(stack)/2,
    CLONE_NEWPID | SIGCHLD, NULL);
    assert(pid > 0);

    assert(ptrace(PTRACE_ATTACH, pid, 0, 0) == 0);
    assert(waitpid(-1, NULL, WSTOPPED) == pid);

    assert(ptrace(PTRACE_DETACH, pid, 0, SIGSTOP) == 0);
    assert(syscall(__NR_tkill, pid, SIGKILL) == 0);
    assert(pid == wait(NULL));
    }
    }

    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Signed-off-by: Oleg Nesterov <oleg@redhat.com>
    ---
    kernel/signal.c | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/kernel/signal.c b/kernel/signal.c
    index 87209e5..7e9f6fa 100644
    --- a/kernel/signal.c
    +++ b/kernel/signal.c
    @@ -914,7 +914,7 @@ static void complete_signal(int sig, struct task_struct *p, int group)
    * then start taking the whole group down immediately.
    */
    if (sig_fatal(p, sig) &&
    - !(signal->flags & (SIGNAL_UNKILLABLE | SIGNAL_GROUP_EXIT)) &&
    + !(signal->flags & SIGNAL_GROUP_EXIT) &&
    !sigismember(&t->real_blocked, sig) &&
    (sig == SIGKILL || !t->ptrace)) {
    /*
    --
    1.5.5.1


    \
     
     \ /
      Last update: 2015-11-05 20:41    [W:6.249 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site