linux-kernel - Re: [PATCH] coredump: set ->group_exit_code for other CLONE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2375c9f91002080213n6de036br7690e14294ede27b@mail.gmail.com>
Date:	Mon, 8 Feb 2010 18:13:15 +0800
From:	Américo Wang <xiyou.wangcong@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Frank Heckenbach <f.heckenbach@...soft.de>,
	Neil Horman <nhorman@...driver.com>,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coredump: set ->group_exit_code for other CLONE_VM tasks 
	too

On Mon, Feb 8, 2010 at 1:16 AM, Oleg Nesterov <oleg@...hat.com> 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@...soft.de>
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>

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

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;
>
>