linux-kernel - Re: [PATCH v2] kernel/fork: fix CLONE_CHILD

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]

Date:   Wed, 24 Aug 2016 10:10:24 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>,
        Roland McGrath <roland@...k.frob.com>,
        Andreas Schwab <schwab@...e.com>,
        William Preston <wpreston@...e.com>
Subject: Re: [PATCH v2] kernel/fork: fix CLONE_CHILD_CLEARTID regression in
 nscd

On Tue 23-08-16 18:32:34, Oleg Nesterov wrote:
> On 08/23, Michal Hocko wrote:
> >
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -913,14 +913,11 @@ void mm_release(struct task_struct *tsk, struct mm_struct *mm)
> >  	deactivate_mm(tsk, mm);
> >  
> >  	/*
> > -	 * If we're exiting normally, clear a user-space tid field if
> > -	 * requested.  We leave this alone when dying by signal, to leave
> > -	 * the value intact in a core dump, and to save the unnecessary
> > -	 * trouble, say, a killed vfork parent shouldn't touch this mm.
> > -	 * Userland only wants this done for a sys_exit.
> > +	 * Signal userspace if we're not exiting with a core dump
> > +	 * or a killed vfork parent which shouldn't touch this mm.
> 
> Well. ACK, but the comment looks wrong...
> 
> The "killed vfork parent ..." part should be removed, as you pointed
> out this is no longer true.
> 
> OTOH, to me it would be better to not remove the "leave the value
> intact in a core dump" part, otherwise the " we're not exiting with
> a core dump" looks pointless because SIGNAL_GROUP_COREDUMP is self-
> documenting.

Sounds better?
diff --git a/kernel/fork.c b/kernel/fork.c
index b89f0eb99f0a..ddde5849df81 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -914,7 +914,8 @@ void mm_release(struct task_struct *tsk, struct mm_struct *mm)
 
 	/*
 	 * Signal userspace if we're not exiting with a core dump
-	 * or a killed vfork parent which shouldn't touch this mm.
+	 * because we want to leave the value intact for debugging
+	 * purposes.
 	 */
 	if (tsk->clear_child_tid) {
 		if (!(tsk->signal->flags & SIGNAL_GROUP_COREDUMP) &&
-- 
Michal Hocko
SUSE Labs