lkml.org 
[lkml]   [2010]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: kernel panic on kill(0, SIGTERM) with PGID == 0
    sorry for delay, vacation.

    On 05/04, Mathias Krause wrote:
    >
    > Hi Oleg, Hi Eric,
    >
    > I stumbled across a nasty bug related to the special init I'm using
    > (cinit) and a process trying to kill it's process group. That always ends
    > in a kernel NULL pointer dereference. git bisect brought me to that
    > commit:
    >
    > | commit 430c623121ea88ca80595c99fdc63b7f8a803ae5
    > | Author: Oleg Nesterov <oleg@tv-sign.ru>
    > | Date: Fri Feb 8 04:19:11 2008 -0800
    > |
    > | start the global /sbin/init with 0,0 special pids
    > |
    > | As Eric pointed out, there is no problem with init starting with sid == pgid
    > | == 0, and this was historical linux behavior changed in 2.6.18.
    > |
    > | Remove kernel_init()->__set_special_pids(), this is unneeded and complicates
    > | the rules for sys_setsid().
    > |
    > | This change and the previous change in daemonize() mean that /sbin/init does
    > | not need the special "session != 1" hack in sys_setsid() any longer. We can'
    > | remove this check yet, we should cleanup copy_process(CLONE_NEWPID) first, s
    > | update the comment only.
    > |
    > | Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
    > | Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
    > | Cc: Pavel Emelyanov <xemul@openvz.org>
    > | Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    > | Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    >
    > Well, it actually is a problem for my setup. If neither init nor any of
    > the programs init starts ever change the PGID, all will live in the
    > process group 0. That's bad when some of the started programs tries to
    > kill its process group. That will in fact kill _all_ processes. So far so
    > bad.

    sorry again, I'll try to comment this later...

    And I think this should be discussed on lkml, cc'ed.

    > But it even gets worser because process group 0 contains some
    > special processes, like swapper (PID: 0). Normally swapper will never be
    > reachable for userland because PID 0 is handled special by kill(2) but
    > killing the current process group while having a PGID of 0 will also try
    > to kill those special processes like swapper. This ends in the following
    > kernel null pointer deref:
    >
    > [ 3.595820] BUG: unable to handle kernel NULL pointer dereference at 000003a8

    Thanks Mathias.

    I think this should be fixed anyway. Could you try the patch below?

    In any case swapper should be immune to signals, and its ->thread_group
    should be properly initiallized (the patch does only this).

    > [ 3.595820] [<c012b45b>] __group_send_sig_info+0x7b/0xa0
    > [ 3.595820] [<c012b5bd>] group_send_sig_info+0x5d/0x80
    > [ 3.595820] [<c012b628>] __kill_pgrp_info+0x48/0x70
    > [ 3.595820] [<c012b679>] kill_pgrp_info+0x29/0x40

    Looks like, you kernel is old. Any chance you can also test the recent
    kernel?

    > May be a minor bug, because it can be work around by calling setpgid(0,0)
    > in init

    setpgid(0,0) just moves the caller's pgrp from PGID 0, that is why it
    helps.

    > but I think it should be fixed, anyway.

    Completely agreed.

    > A reproducer is attached. It contains a substitute for init that triggers
    > the bug.

    Thanks.

    I didn't try it, but it looks overcomplicated to trigger this bug, or
    I missed something? Afaics, init could be just

    int main(void)
    {
    kill(0, SIGGKILL);
    }

    No?

    Oleg.

    We should also change INIT_SIGHAND, but _hopefully_ this is enough
    to fix the crash.

    --- x/include/linux/init_task.h
    +++ x/include/linux/init_task.h
    @@ -172,6 +172,7 @@ extern struct cred init_cred;
    [PIDTYPE_PGID] = INIT_PID_LINK(PIDTYPE_PGID), \
    [PIDTYPE_SID] = INIT_PID_LINK(PIDTYPE_SID), \
    }, \
    + .thread_group = LIST_HEAD_INIT(tsk.thread_group), \
    .dirties = INIT_PROP_LOCAL_SINGLE(dirties), \
    INIT_IDS \
    INIT_PERF_EVENTS(tsk) \



    \
     
     \ /
      Last update: 2010-05-09 20:49    [W:2.780 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site