Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 1 Sep 2012 11:21:57 +0000 (UTC)
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r239989 - head/sys/kern
Message-ID:  <201209011121.q81BLvxV000218@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: pjd
Date: Sat Sep  1 11:21:56 2012
New Revision: 239989
URL: http://svn.freebsd.org/changeset/base/239989

Log:
  Fix panic in procdesc that can be triggered in the following scenario:
  
  1. Process A pdfork(2)s process B.
  2. Process A passes process descriptor of B to unrelated process C.
  3. Hit CTRL+C to terminate process A. Process B is also terminated
     with SIGINT.
  4. init(8) collects status of process B.
  5. Process C closes process descriptor associated with process B.
  
  When we have such order of events, init(8), by collecting status of
  process B, will call procdesc_reap(). This function sets pd_proc to NULL.
  
  Now when process C calls close on this process descriptor,
  procdesc_close() is called. Unfortunately procdesc_close() assumes that
  pd_proc points at a valid proc structure, but it was set to NULL earlier,
  so the kernel panics.
  
  The patch also adds setting 'p->p_procdesc' to NULL in procdesc_reap(),
  which I think should be done.
  
  MFC after:	1 week

Modified:
  head/sys/kern/sys_procdesc.c

Modified: head/sys/kern/sys_procdesc.c
==============================================================================
--- head/sys/kern/sys_procdesc.c	Sat Sep  1 10:56:15 2012	(r239988)
+++ head/sys/kern/sys_procdesc.c	Sat Sep  1 11:21:56 2012	(r239989)
@@ -333,6 +333,7 @@ procdesc_reap(struct proc *p)
 
 	pd = p->p_procdesc;
 	pd->pd_proc = NULL;
+	p->p_procdesc = NULL;
 	procdesc_free(pd);
 }
 
@@ -358,14 +359,20 @@ procdesc_close(struct file *fp, struct t
 	pd->pd_flags |= PDF_CLOSED;
 	PROCDESC_UNLOCK(pd);
 	p = pd->pd_proc;
-	PROC_LOCK(p);
-	if (p->p_state == PRS_ZOMBIE) {
+	if (p == NULL) {
+		/*
+		 * This is the case where process' exit status was already
+		 * 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, NULL);
 	} else {
@@ -376,6 +383,7 @@ procdesc_close(struct file *fp, struct t
 		 * 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);

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