NetBSD Problem Report #50298

From kre@munnari.OZ.AU  Sun Oct  4 13:21:09 2015
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A9971A5B2E
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  4 Oct 2015 13:21:09 +0000 (UTC)
Message-Id: <201510041319.t94DJYvi028886@andromeda.noi.kre.to>
Date: Sun, 4 Oct 2015 20:19:34 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@gnats.NetBSD.org
Subject: Questionable code in kern_exec.c
X-Send-Pr-Version: 3.95

>Number:         50298
>Category:       kern
>Synopsis:       Questionable code in kern_exec.c
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    pgoyette
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 04 13:25:00 +0000 2015
>Closed-Date:    Sat Nov 07 22:16:51 +0000 2015
>Last-Modified:  Sat Nov 07 22:16:51 +0000 2015
>Originator:     Robert Elz
>Release:        NetBSD 7.99.21 (current as of 2015-10-01)
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD andromeda.noi.kre.to 7.99.21 NetBSD 7.99.21 (VBOX64-1.1-20150829) #3: Sun Aug 30 07:16:17 ICT 2015 kre@andromeda.noi.kre.to:/home/kre/src/current-kernel/usr/src/sys/arch/amd64/compile/VBOX64 amd64
Architecture: x86_64
Machine: amd64
>Description:
	While looking into Paul Goyette's unkillable zombie
	problem (and p_nstopchild in particular), I came across
	the folowing code in kern_exec.c (in execve_runproc()) ...

        if (p->p_sflag & PS_STOPEXEC) {
                ksiginfoq_t kq;

                KERNEL_UNLOCK_ALL(l, &l->l_biglocks);
                p->p_pptr->p_nstopchild++;
                p->p_pptr->p_waited = 0;
		/* .... */

	I doubt this is in any way related to the problem
	under investigation, but it looks obviously wrong.

	The p_waited that is cleared should be p->p_waited
	not the flag in the parent process.

	That is, p_nstopchild (in the parent) is a count of
	its child processes on which a wait() should succeed.
	A child that is stopped (rather than dead) as is the
	case here, should be wait()'d for exactly once.
	p_waited (in the child) determines whether that
	has happened or not, for the qhile in question
	(when p_waited is set to 1, p->p_pptr->p_nstopchild
	gets decremented by one).

	Clearing the parent's p_waited flag (without possibly
	adjusting its parent's p_nstopchild) is wrong.

	Incrementing our parent's p_nstopchild without
	verifying (or forcing) our p_waited to 0 is also
	wrong (if p->p_waited != 0) then this process can
	never be waited for.

>How-To-Repeat:
	By inspection.   I'm not aware of any way to actually
	force a problem to occur here (which doesn't mean there
	isn't one).

>Fix:
	s/p->p_pptr->p_waited/p->p_waited/

	(probably).

	If someone can explain why the code as it is now is
	actually correct, please just close this PR.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->pgoyette
Responsible-Changed-By: pgoyette@NetBSD.org
Responsible-Changed-When: Sun, 11 Oct 2015 23:30:45 +0000
Responsible-Changed-Why:
I have the fix.


From: "Paul Goyette" <pgoyette@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50298 CVS commit: src/sys/kern
Date: Tue, 13 Oct 2015 00:24:35 +0000

 Module Name:	src
 Committed By:	pgoyette
 Date:		Tue Oct 13 00:24:35 UTC 2015

 Modified Files:
 	src/sys/kern: kern_exec.c

 Log Message:
 In execve_runproc(), update the p_waited entry for the process being
 moved to SSTOP state, not for its parent.  (It is correct to update
 the parent's p_nstopchild count.)  If the value is not already zero,
 it could prevent its parent from waiting for the process.

 Fixes PR kern/50298

 Pullups will be requested for:

        NetBSD-7, -6, -6-0, -6-1, -5, -5-0, -5-1, and -5-2


 To generate a diff of this commit:
 cvs rdiff -u -r1.418 -r1.419 src/sys/kern/kern_exec.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->pending-pullups
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Tue, 13 Oct 2015 01:34:14 +0000
State-Changed-Why:
Committed to head, pending-pullups


State-Changed-From-To: pending-pullups->closed
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Sat, 07 Nov 2015 22:16:51 +0000
State-Changed-Why:
Fixed, pullups completed.


>Unformatted:

Home	PR Database Search