lkml.org 
[lkml]   [2021]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 1/8] signal: Make SIGKILL during coredumps an explicit special case
    Simplify the code that allows SIGKILL during coredumps to terminate
    the coredump. As far as I can tell I have avoided breaking it
    by dumb luck.

    Historically with all of the other threads stopping in exit_mm the
    wants_signal loop in complete_signal would find the dumper task and
    then complete_signal would wake the dumper task with signal_wake_up.

    After moving the coredump_task_exit above the setting of PF_EXITING in
    commit 92307383082d ("coredump: Don't perform any cleanups before
    dumping core") wants_signal will consider all of the threads in a
    multi-threaded process for waking up, not just the core dumping task.

    Luckily complete_signal short circuits SIGKILL during a coredump marks
    every thread with SIGKILL and signal_wake_up. This code is arguably
    buggy however as it tries to skip creating a group exit when is already
    present, and it fails that a coredump is in progress.

    Ever since commit 06af8679449d ("coredump: Limit what can interrupt
    coredumps") was added dump_interrupted needs not just TIF_SIGPENDING
    set on the dumper task but also SIGKILL set in it's pending bitmap.
    This means that if the code is ever fixed not to short-circuit and
    kill a process after it has already been killed the special case
    for SIGKILL during a coredump will be broken.

    Sort all of this out by making the coredump special case more special,
    and perform all of the work in prepare_signal and leave the rest of
    the signal delivery path out of it.

    In prepare_signal when the process coredumping is sent SIGKILL find
    the task performing the coredump and use sigaddset and signal_wake_up
    to ensure that task reports fatal_signal_pending.

    Return false from prepare_signal to tell the rest of the signal
    delivery path to ignore the signal.

    Update wait_for_dump_helpers to perform a wait_event_killable wait
    so that if signal_pending gets set spuriously the wait will not
    be interrupted unless fatal_signal_pending is true.

    I have tested this and verified I did not break SIGKILL during
    coredumps by accident (before or after this change). I actually
    thought I had and I had to figure out what I had misread that kept
    SIGKILL during coredumps working.

    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    ---
    fs/coredump.c | 4 ++--
    kernel/signal.c | 11 +++++++++--
    2 files changed, 11 insertions(+), 4 deletions(-)

    diff --git a/fs/coredump.c b/fs/coredump.c
    index a6b3c196cdef..7b91fb32dbb8 100644
    --- a/fs/coredump.c
    +++ b/fs/coredump.c
    @@ -448,7 +448,7 @@ static void coredump_finish(bool core_dumped)
    static bool dump_interrupted(void)
    {
    /*
    - * SIGKILL or freezing() interrupt the coredumping. Perhaps we
    + * SIGKILL or freezing() interrupted the coredumping. Perhaps we
    * can do try_to_freeze() and check __fatal_signal_pending(),
    * but then we need to teach dump_write() to restart and clear
    * TIF_SIGPENDING.
    @@ -471,7 +471,7 @@ static void wait_for_dump_helpers(struct file *file)
    * We actually want wait_event_freezable() but then we need
    * to clear TIF_SIGPENDING and improve dump_interrupted().
    */
    - wait_event_interruptible(pipe->rd_wait, pipe->readers == 1);
    + wait_event_killable(pipe->rd_wait, pipe->readers == 1);

    pipe_lock(pipe);
    pipe->readers--;
    diff --git a/kernel/signal.c b/kernel/signal.c
    index 8272cac5f429..7e305a8ec7c2 100644
    --- a/kernel/signal.c
    +++ b/kernel/signal.c
    @@ -907,8 +907,15 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
    sigset_t flush;

    if (signal->flags & (SIGNAL_GROUP_EXIT | SIGNAL_GROUP_COREDUMP)) {
    - if (!(signal->flags & SIGNAL_GROUP_EXIT))
    - return sig == SIGKILL;
    + struct core_state *core_state = signal->core_state;
    + if (core_state) {
    + if (sig == SIGKILL) {
    + struct task_struct *dumper = core_state->dumper.task;
    + sigaddset(&dumper->pending.signal, SIGKILL);
    + signal_wake_up(dumper, 1);
    + }
    + return false;
    + }
    /*
    * The process is in the middle of dying, nothing to do.
    */
    --
    2.29.2
    \
     
     \ /
      Last update: 2021-12-13 23:54    [W:4.142 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site