From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752397AbbJEQZm (ORCPT ); Mon, 5 Oct 2015 12:25:42 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:33103 "EHLO mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751722AbbJEQZl (ORCPT ); Mon, 5 Oct 2015 12:25:41 -0400 Date: Mon, 5 Oct 2015 18:25:38 +0200 From: Michal Hocko To: Oleg Nesterov Cc: Andrew Morton , David Rientjes , Kyle Walker , Stanislav Kozina , Tetsuo Handa , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] coredump: ensure all coredumping tasks have Message-ID: <20151005162538.GE7023@dhcp22.suse.cz> References: <20150929155446.GA15095@redhat.com> <20150929155502.GA15109@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150929155502.GA15109@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The subject is truncated. It is missing SIGNAL_GROUP_COREDUMP On Tue 29-09-15 17:55:02, Oleg Nesterov wrote: > task_will_free_mem() is wrong in many ways, and in particular the > SIGNAL_GROUP_COREDUMP check is not reliable: a task can participate > in the coredumping without SIGNAL_GROUP_COREDUMP bit set. > > change zap_threads() paths to always set SIGNAL_GROUP_COREDUMP even > if other CLONE_VM processes can't react to SIGKILL. Fortunately, at > least oom-kill case if fine; it kills all tasks sharing the same mm, > so it should also kill the process which actually dumps the core. Yes I do not think it will make too much difference for the oom killer but it is much better to handle all the processes sharing the mm the same way. > > The change in prepare_signal() is not strictly necessary, it just > ensures that the patch does not bring another subtle behavioural > change. But it reminds us that this SIGNAL_GROUP_EXIT/COREDUMP case > needs more changes. > > Signed-off-by: Oleg Nesterov Acked-by: Michal Hocko > --- > fs/coredump.c | 12 ++++++------ > kernel/signal.c | 2 +- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/coredump.c b/fs/coredump.c > index 53d7d46..4fed8d0 100644 > --- a/fs/coredump.c > +++ b/fs/coredump.c > @@ -282,11 +282,13 @@ out: > return ispipe; > } > > -static int zap_process(struct task_struct *start, int exit_code) > +static int zap_process(struct task_struct *start, int exit_code, int flags) > { > struct task_struct *t; > int nr = 0; > > + /* ignore all signals except SIGKILL, see prepare_signal() */ > + start->signal->flags = SIGNAL_GROUP_COREDUMP | flags; > start->signal->group_exit_code = exit_code; > start->signal->group_stop_count = 0; > > @@ -313,10 +315,8 @@ static int zap_threads(struct task_struct *tsk, struct mm_struct *mm, > spin_lock_irq(&tsk->sighand->siglock); > if (!signal_group_exit(tsk->signal)) { > mm->core_state = core_state; > - nr = zap_process(tsk, exit_code); > tsk->signal->group_exit_task = tsk; > - /* ignore all signals except SIGKILL, see prepare_signal() */ > - tsk->signal->flags = SIGNAL_GROUP_COREDUMP; > + nr = zap_process(tsk, exit_code, 0); > clear_tsk_thread_flag(tsk, TIF_SIGPENDING); > } > spin_unlock_irq(&tsk->sighand->siglock); > @@ -367,8 +367,8 @@ static int zap_threads(struct task_struct *tsk, struct mm_struct *mm, > if (p->mm) { > if (unlikely(p->mm == mm)) { > lock_task_sighand(p, &flags); > - nr += zap_process(p, exit_code); > - p->signal->flags = SIGNAL_GROUP_EXIT; > + nr += zap_process(p, exit_code, > + SIGNAL_GROUP_EXIT); > unlock_task_sighand(p, &flags); > } > break; > diff --git a/kernel/signal.c b/kernel/signal.c > index f2cbd4e..c0b01fe 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -788,7 +788,7 @@ 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_COREDUMP) > + if (!(signal->flags & SIGNAL_GROUP_EXIT)) > return sig == SIGKILL; > /* > * The process is in the middle of dying, nothing to do. > -- > 2.4.3 -- Michal Hocko SUSE Labs