Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 6 Apr 2014 20:00:42 +0000 (UTC)
From:      Ed Schouten <ed@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r264200 - head/sys/kern
Message-ID:  <201404062000.s36K0gOv060915@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ed
Date: Sun Apr  6 20:00:42 2014
New Revision: 264200
URL: http://svnweb.freebsd.org/changeset/base/264200

Log:
  Nit: fix locking of p->p_state in procdesc_close().
  
  According to <sys/proc.h>, this field needs to be locked with either the
  p_mtx or the p_slock. In this case the damage was quite small. Instead
  of being reaped, the process would just be reparented to init, so it
  could be reaped from there.

Modified:
  head/sys/kern/sys_procdesc.c

Modified: head/sys/kern/sys_procdesc.c
==============================================================================
--- head/sys/kern/sys_procdesc.c	Sun Apr  6 19:51:57 2014	(r264199)
+++ head/sys/kern/sys_procdesc.c	Sun Apr  6 20:00:42 2014	(r264200)
@@ -364,39 +364,41 @@ procdesc_close(struct file *fp, struct t
 		 * collected and procdesc_reap() was already called.
 		 */
 		sx_xunlock(&proctree_lock);
-	} else if (p->p_state == PRS_ZOMBIE) {
-		/*
-		 * If the process is already dead and just awaiting reaping,
-		 * do that now.  This will release the process's reference to
-		 * the process descriptor when it calls back into
-		 * procdesc_reap().
-		 */
-		PROC_LOCK(p);
-		PROC_SLOCK(p);
-		proc_reap(curthread, p, NULL, 0);
 	} else {
-		/*
-		 * If the process is not yet dead, we need to kill it, but we
-		 * can't wait around synchronously for it to go away, as that
-		 * path leads to madness (and deadlocks).  First, detach the
-		 * process from its descriptor so that its exit status will
-		 * be reported normally.
-		 */
 		PROC_LOCK(p);
-		pd->pd_proc = NULL;
-		p->p_procdesc = NULL;
-		procdesc_free(pd);
-
-		/*
-		 * Next, reparent it to init(8) so that there's someone to
-		 * pick up the pieces; finally, terminate with prejudice.
-		 */
-		p->p_sigparent = SIGCHLD;
-		proc_reparent(p, initproc);
-		if ((pd->pd_flags & PDF_DAEMON) == 0)
-			kern_psignal(p, SIGKILL);
-		PROC_UNLOCK(p);
-		sx_xunlock(&proctree_lock);
+		if (p->p_state == PRS_ZOMBIE) {
+			/*
+			 * If the process is already dead and just awaiting
+			 * reaping, do that now.  This will release the
+			 * process's reference to the process descriptor when it
+			 * calls back into procdesc_reap().
+			 */
+			PROC_SLOCK(p);
+			proc_reap(curthread, p, NULL, 0);
+		} else {
+			/*
+			 * If the process is not yet dead, we need to kill it,
+			 * but we can't wait around synchronously for it to go
+			 * away, as that path leads to madness (and deadlocks).
+			 * First, detach the process from its descriptor so that
+			 * its exit status will be reported normally.
+			 */
+			pd->pd_proc = NULL;
+			p->p_procdesc = NULL;
+			procdesc_free(pd);
+
+			/*
+			 * Next, reparent it to init(8) so that there's someone
+			 * to pick up the pieces; finally, terminate with
+			 * prejudice.
+			 */
+			p->p_sigparent = SIGCHLD;
+			proc_reparent(p, initproc);
+			if ((pd->pd_flags & PDF_DAEMON) == 0)
+				kern_psignal(p, SIGKILL);
+			PROC_UNLOCK(p);
+			sx_xunlock(&proctree_lock);
+		}
 	}
 
 	/*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201404062000.s36K0gOv060915>