No need to synchronize linux_schedtail with linux_proc_init.

p->p_emuldata is properly initialized in the time when the child can run.

Do not set p->p_emuldata to NULL when the process is exiting.
It does not make any sense and only costs 2 mutex operations.

Do not lock emul_data to unlock it on the very next line.
Comment on possible race while there.

Reparent all procs that are part of a threading group but not its leaders
to init and SIGCHLD init to finish the zombies off. This fixes zombies
left after opera's exit. [1]

There is no need to lock p_em in the linux_proc_init CLONE_THREAD
case because the process cannot change the address of the p_em->shared
because its currently running this code path.
Move assigning of em->shared outside emul_shared_lock.

Noticed by: Scott Robbins <scottro@nyc.rr.com> [1]
Submitted by: rdivacky

1 files changed, 28 insertions, 26 deletions

diff --git a/sys/compat/linux/linux_emul.c b/sys/compat/linux/linux_emul.c
index 70e6887..9240c1e2 100644
--- a/sys/compat/linux/linux_emul.c
+++ b/sys/compat/linux/linux_emul.c
@@ -114,13 +114,18 @@ linux_proc_init(struct thread *td, pid_t child, int flags)
 	if (child != 0) {
 		if (flags & CLONE_THREAD) {
 			/* lookup the parent */
-		   	EMUL_SHARED_WLOCK(&emul_shared_lock);
-			p_em = em_find(td->td_proc, EMUL_DOLOCK);
+			/* 
+			 * we dont have to lock the p_em because
+			 * its waiting for us in linux_clone so
+			 * there is no chance of it changing the
+			 * p_em->shared address
+			 */
+			p_em = em_find(td->td_proc, EMUL_DONTLOCK);
 			KASSERT(p_em != NULL, ("proc_init: parent emuldata not found for CLONE_THREAD\n"));
 			em->shared = p_em->shared;
+			EMUL_SHARED_WLOCK(&emul_shared_lock);
 			em->shared->refs++;
-		   	EMUL_SHARED_WUNLOCK(&emul_shared_lock);
-			EMUL_UNLOCK(&emul_lock);
+			EMUL_SHARED_WUNLOCK(&emul_shared_lock);
 		} else {
 			/*
 			 * handled earlier to avoid malloc(M_WAITOK) with
@@ -136,8 +141,6 @@ linux_proc_init(struct thread *td, pid_t child, int flags)
 		p = pfind(child);
 		KASSERT(p != NULL, ("process not found in proc_init\n"));
 		p->p_emuldata = em;
-		/* we might have a sleeping linux_schedtail */
-		wakeup(&p->p_emuldata);
 		PROC_UNLOCK(p);
 	} else
 		EMUL_UNLOCK(&emul_lock);
@@ -162,6 +165,17 @@ linux_proc_exit(void *arg __unused, struct proc *p)
 
 	KASSERT(em != NULL, ("proc_exit: emuldata not found.\n"));
 
+	/* reparent all procs that are not a thread leader to initproc */
+	if (em->shared->group_pid != p->p_pid) {
+	   	sx_xlock(&proctree_lock);
+	   	wakeup(initproc);
+		PROC_LOCK(p);
+		proc_reparent(p, initproc);
+		p->p_sigparent = SIGCHLD;
+		PROC_UNLOCK(p);
+	   	sx_xunlock(&proctree_lock);
+	}
+
 	child_clear_tid = em->child_clear_tid;
 
 	EMUL_UNLOCK(&emul_lock);
@@ -169,10 +183,6 @@ linux_proc_exit(void *arg __unused, struct proc *p)
 	EMUL_SHARED_WLOCK(&emul_shared_lock);
 	LIST_REMOVE(em, threads);
 
-	PROC_LOCK(p);
-	p->p_emuldata = NULL;
-	PROC_UNLOCK(p);
-
 	em->shared->refs--;
 	if (em->shared->refs == 0)
 		free(em->shared, M_LINUX);
@@ -243,12 +253,16 @@ linux_proc_exec(void *arg __unused, struct proc *p, struct image_params *imgp)
 	    && p->p_sysent == &elf_linux_sysvec)) {
 		struct linux_emuldata *em;
 
-		em = em_find(p, EMUL_DOLOCK);
+		/* 
+		 * XXX:There's a race because here we assign p->p_emuldata NULL
+		 * but the process is still counted as linux one for a short
+ 		 * time so some other process might reference it and try to
+ 		 * access its p->p_emuldata and panicing on a NULL reference.
+		 */
+		em = em_find(p, EMUL_DONTLOCK);
 
 		KASSERT(em != NULL, ("proc_exec: emuldata not found.\n"));
 
-		EMUL_UNLOCK(&emul_lock);
-
 		EMUL_SHARED_WLOCK(&emul_shared_lock);
 		LIST_REMOVE(em, threads);
 
@@ -277,22 +291,10 @@ linux_schedtail(void *arg __unused, struct proc *p)
 	if (__predict_true(p->p_sysent != &elf_linux_sysvec))
 		return;
 
-retry:
 	/* find the emuldata */
 	em = em_find(p, EMUL_DOLOCK);
 
-	if (em == NULL) {
-		/*
-		 * We might have been called before proc_init for this
-		 * process so tsleep and be woken up by it. We use
-		 * p->p_emuldata for this
-		 */
-
-		error = tsleep(&p->p_emuldata, PLOCK, "linux_schedtail", hz);
-		if (error == 0)
-			goto retry;
-		panic("no emuldata found for userreting process.\n");
-	}
+	KASSERT(em != NULL, ("linux_schedtail: emuldata not found.\n"));
 	child_set_tid = em->child_set_tid;
 	EMUL_UNLOCK(&emul_lock);