Messages in this thread Patch in this message | | | Date | Sun, 9 May 2010 20:45:10 +0200 | From | Oleg Nesterov <> | Subject | Re: 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) \
| |