Re: [PATCH] coredump: set ->group_exit_code for other CLONE_VM tasks too

From: AmÃrico Wang
Date: Mon Feb 08 2010 - 05:13:22 EST


On Mon, Feb 8, 2010 at 1:16 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> User visible change.
>
> do_coredump() kills all threads which share the same ->mm but only
> the coredumping process gets the proper exit_code. Other tasks which
> share the same ->mm die "silently" and return status == 0 to parent.
>
> This is historical behaviour, not actually a bug. But I think Frank
> Heckenbach rightly dislikes the current behaviour. Simple test-case:
>
> Â Â Â Â#include <stdio.h>
> Â Â Â Â#include <unistd.h>
> Â Â Â Â#include <signal.h>
> Â Â Â Â#include <sys/wait.h>
>
> Â Â Â Âint main(void)
> Â Â Â Â{
> Â Â Â Â Â Â Â Âint stat;
>
> Â Â Â Â Â Â Â Âif (!fork()) {
> Â Â Â Â Â Â Â Â Â Â Â Âif (!vfork())
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âkill(getpid(), SIGQUIT);
> Â Â Â Â Â Â Â Â}
>
> Â Â Â Â Â Â Â Âwait(&stat);
> Â Â Â Â Â Â Â Âprintf("stat=%x\n", stat);
> Â Â Â Â Â Â Â Âreturn 0;
> Â Â Â Â}
>
> Before this patch it prints "stat=0" despite the fact the child was
> killed by SIGQUIT. After this patch the output is "stat=3" which
> obviously makes more sense.
>
> Even with this patch, only the task which originates the coredumping
> gets "|= 0x80" if the core was actually dumped, but at least the
> coredumping signal is visible to do_wait/etc.


Nice changelog!

>
> Reported-by: Frank Heckenbach <f.heckenbach@xxxxxxxxxx>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>

Acked-by: WANG Cong <xiyou.wangcong@xxxxxxxxx>

Thank you!

> ---
>
> Âfs/exec.c | Â Â8 ++++----
> Â1 file changed, 4 insertions(+), 4 deletions(-)
>
> --- V1/fs/exec.c~CD_STATUS Â Â Â2009-12-18 00:20:50.000000000 +0100
> +++ V1/fs/exec.c    Â2010-02-07 17:28:24.000000000 +0100
> @@ -1536,12 +1536,13 @@ out:
> Â Â Â Âreturn ispipe;
> Â}
>
> -static int zap_process(struct task_struct *start)
> +static int zap_process(struct task_struct *start, int exit_code)
> Â{
> Â Â Â Âstruct task_struct *t;
> Â Â Â Âint nr = 0;
>
> Â Â Â Âstart->signal->flags = SIGNAL_GROUP_EXIT;
> + Â Â Â start->signal->group_exit_code = exit_code;
> Â Â Â Âstart->signal->group_stop_count = 0;
>
> Â Â Â Ât = start;
> @@ -1566,8 +1567,7 @@ static inline int zap_threads(struct tas
> Â Â Â Âspin_lock_irq(&tsk->sighand->siglock);
> Â Â Â Âif (!signal_group_exit(tsk->signal)) {
> Â Â Â Â Â Â Â Âmm->core_state = core_state;
> - Â Â Â Â Â Â Â tsk->signal->group_exit_code = exit_code;
> - Â Â Â Â Â Â Â nr = zap_process(tsk);
> + Â Â Â Â Â Â Â nr = zap_process(tsk, exit_code);
> Â Â Â Â}
> Â Â Â Âspin_unlock_irq(&tsk->sighand->siglock);
> Â Â Â Âif (unlikely(nr < 0))
> @@ -1616,7 +1616,7 @@ static inline int zap_threads(struct tas
> Â Â Â Â Â Â Â Â Â Â Â Âif (p->mm) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âif (unlikely(p->mm == mm)) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlock_task_sighand(p, &flags);
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â nr += zap_process(p);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â nr += zap_process(p, exit_code);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunlock_task_sighand(p, &flags);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>
>
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸ ¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðà nÆàþY&—