Bug 144194 - [linux] [patch] linuxulator: 2 exec bug fixes
Summary: [linux] [patch] linuxulator: 2 exec bug fixes
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 1.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: Alexander Leidinger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-22 01:50 UTC by Gleb Kurtsou
Modified: 2011-03-02 10:12 UTC (History)
0 users

See Also:


Attachments
file.shar (7.27 KB, text/plain)
2010-02-22 01:50 UTC, Gleb Kurtsou
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gleb Kurtsou freebsd_committer freebsd_triage 2010-02-22 01:50:02 UTC
1. After calling exec() in multithreaded linux program threads are not destroyed and continue running. They get killed after program being executed finishes

2. linux_exit_group doesn't return correct exit code when called from not from group leader. Which happens regularly using sun jvm. I've changed exit1() to allow process_exit event handler to change p->p_xstat, just like NetBSD does.

There's another PR for this bug: kern/141439
But in that PR doesn't kill group leader, it works because sun jvm calls exit_group once again. Expected behavior for exit_group is not to return to userspace.

Submitting it as a single PR because second patch (linux-exit-group-status-code.patch.txt) relays on the first one

Fix: Patch attached with submission follows:
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-02-22 17:09:17 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-emulation

Over to maintainer(s).
Comment 2 Dmitry Chagin freebsd_committer freebsd_triage 2010-07-05 22:43:55 UTC
Responsible Changed
From-To: freebsd-emulation->dchagin

Grab
Comment 3 dfilter service freebsd_committer freebsd_triage 2010-11-22 09:07:08 UTC
Author: netchild
Date: Mon Nov 22 09:06:59 2010
New Revision: 215664
URL: http://svn.freebsd.org/changeset/base/215664

Log:
  By using the 32-bit Linux version of Sun's Java Development Kit 1.6
  on FreeBSD (amd64), invocations of "javac" (or "java") eventually
  end with the output of "Killed" and exit code 137.
  
  This is caused by:
  1. After calling exec() in multithreaded linux program threads are not
     destroyed and continue running. They get killed after program being
     executed finishes.
  
  2. linux_exit_group doesn't return correct exit code when called not
     from group leader. Which happens regularly using sun jvm.
  
  The submitters fix this in a similar way to how NetBSD handles this.
  
  I took the PRs away from dchagin, who seems to be out of touch of
  this since a while (no response from him).
  
  The patches committed here are from [2], with some little modifications
  from me to the style.
  
  PR:		141439 [1], 144194 [2]
  Submitted by:	Stefan Schmidt <stefan.schmidt@stadtbuch.de>, gk
  Reviewed by:	rdivacky (in april 2010)
  MFC after:	5 days

Modified:
  head/sys/compat/linux/linux_emul.c
  head/sys/compat/linux/linux_emul.h
  head/sys/compat/linux/linux_misc.c
  head/sys/kern/kern_exit.c

Modified: head/sys/compat/linux/linux_emul.c
==============================================================================
--- head/sys/compat/linux/linux_emul.c	Mon Nov 22 09:04:29 2010	(r215663)
+++ head/sys/compat/linux/linux_emul.c	Mon Nov 22 09:06:59 2010	(r215664)
@@ -155,7 +155,7 @@ void
 linux_proc_exit(void *arg __unused, struct proc *p)
 {
 	struct linux_emuldata *em;
-	int error;
+	int error, shared_flags, shared_xstat;
 	struct thread *td = FIRST_THREAD_IN_PROC(p);
 	int *child_clear_tid;
 	struct proc *q, *nq;
@@ -187,6 +187,8 @@ linux_proc_exit(void *arg __unused, stru
 	}
 
 	EMUL_SHARED_WLOCK(&emul_shared_lock);
+	shared_flags = em->shared->flags;
+	shared_xstat = em->shared->xstat;
 	LIST_REMOVE(em, threads);
 
 	em->shared->refs--;
@@ -196,6 +198,12 @@ linux_proc_exit(void *arg __unused, stru
 	} else	
 		EMUL_SHARED_WUNLOCK(&emul_shared_lock);
 
+	if ((shared_flags & EMUL_SHARED_HASXSTAT) != 0) {
+		PROC_LOCK(p);
+		p->p_xstat = shared_xstat;
+		PROC_UNLOCK(p);
+	}
+
 	if (child_clear_tid != NULL) {
 		struct linux_sys_futex_args cup;
 		int null = 0;
@@ -257,6 +265,9 @@ linux_proc_exec(void *arg __unused, stru
 	if (__predict_false(imgp->sysent == &elf_linux_sysvec
 	    && p->p_sysent != &elf_linux_sysvec))
 		linux_proc_init(FIRST_THREAD_IN_PROC(p), p->p_pid, 0);
+	if (__predict_false(p->p_sysent == &elf_linux_sysvec))
+		/* Kill threads regardless of imgp->sysent value */
+		linux_kill_threads(FIRST_THREAD_IN_PROC(p), SIGKILL);
 	if (__predict_false(imgp->sysent != &elf_linux_sysvec
 	    && p->p_sysent == &elf_linux_sysvec)) {
 		struct linux_emuldata *em;
@@ -334,3 +345,29 @@ linux_set_tid_address(struct thread *td,
 	EMUL_UNLOCK(&emul_lock);
 	return 0;
 }
+
+void
+linux_kill_threads(struct thread *td, int sig)
+{
+	struct linux_emuldata *em, *td_em, *tmp_em;
+	struct proc *sp;
+
+	td_em = em_find(td->td_proc, EMUL_DONTLOCK);
+
+	KASSERT(td_em != NULL, ("linux_kill_threads: emuldata not found.\n"));
+
+	EMUL_SHARED_RLOCK(&emul_shared_lock);
+	LIST_FOREACH_SAFE(em, &td_em->shared->threads, threads, tmp_em) {
+		if (em->pid == td_em->pid)
+			continue;
+
+		sp = pfind(em->pid);
+		if ((sp->p_flag & P_WEXIT) == 0)
+			psignal(sp, sig);
+		PROC_UNLOCK(sp);
+#ifdef DEBUG
+		printf(LMSG("linux_kill_threads: kill PID %d\n"), em->pid);
+#endif
+	}
+	EMUL_SHARED_RUNLOCK(&emul_shared_lock);
+}

Modified: head/sys/compat/linux/linux_emul.h
==============================================================================
--- head/sys/compat/linux/linux_emul.h	Mon Nov 22 09:04:29 2010	(r215663)
+++ head/sys/compat/linux/linux_emul.h	Mon Nov 22 09:06:59 2010	(r215664)
@@ -31,8 +31,12 @@
 #ifndef _LINUX_EMUL_H_
 #define	_LINUX_EMUL_H_
 
+#define EMUL_SHARED_HASXSTAT	0x01
+
 struct linux_emuldata_shared {
 	int	refs;
+	int	flags;
+	int	xstat;
 	pid_t	group_pid;
 
 	LIST_HEAD(, linux_emuldata) threads; /* head of list of linux threads */
@@ -76,6 +80,7 @@ int	linux_proc_init(struct thread *, pid
 void	linux_proc_exit(void *, struct proc *);
 void	linux_schedtail(void *, struct proc *);
 void	linux_proc_exec(void *, struct proc *, struct image_params *);
+void	linux_kill_threads(struct thread *, int);
 
 extern struct sx	emul_shared_lock;
 extern struct mtx	emul_lock;

Modified: head/sys/compat/linux/linux_misc.c
==============================================================================
--- head/sys/compat/linux/linux_misc.c	Mon Nov 22 09:04:29 2010	(r215663)
+++ head/sys/compat/linux/linux_misc.c	Mon Nov 22 09:06:59 2010	(r215664)
@@ -1695,34 +1695,23 @@ linux_setdomainname(struct thread *td, s
 int
 linux_exit_group(struct thread *td, struct linux_exit_group_args *args)
 {
-	struct linux_emuldata *em, *td_em, *tmp_em;
-	struct proc *sp;
+	struct linux_emuldata *em;
 
 #ifdef DEBUG
 	if (ldebug(exit_group))
 		printf(ARGS(exit_group, "%i"), args->error_code);
 #endif
 
-	if (linux_use26(td)) {
-		td_em = em_find(td->td_proc, EMUL_DONTLOCK);
-
-		KASSERT(td_em != NULL, ("exit_group: emuldata not found.\n"));
-
-		EMUL_SHARED_RLOCK(&emul_shared_lock);
-		LIST_FOREACH_SAFE(em, &td_em->shared->threads, threads, tmp_em) {
-			if (em->pid == td_em->pid)
-				continue;
-
-			sp = pfind(em->pid);
-			psignal(sp, SIGKILL);
-			PROC_UNLOCK(sp);
-#ifdef DEBUG
-			printf(LMSG("linux_sys_exit_group: kill PID %d\n"), em->pid);
-#endif
-		}
-
-		EMUL_SHARED_RUNLOCK(&emul_shared_lock);
+	em = em_find(td->td_proc, EMUL_DONTLOCK);
+	if (em->shared->refs > 1) {
+		EMUL_SHARED_WLOCK(&emul_shared_lock);
+		em->shared->flags |= EMUL_SHARED_HASXSTAT;
+		em->shared->xstat = W_EXITCODE(args->error_code, 0);
+		EMUL_SHARED_WUNLOCK(&emul_shared_lock);
+		if (linux_use26(td))
+			linux_kill_threads(td, SIGKILL);
 	}
+
 	/*
 	 * XXX: we should send a signal to the parent if
 	 * SIGNAL_EXIT_GROUP is set. We ignore that (temporarily?)

Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c	Mon Nov 22 09:04:29 2010	(r215663)
+++ head/sys/kern/kern_exit.c	Mon Nov 22 09:06:59 2010	(r215664)
@@ -200,6 +200,7 @@ exit1(struct thread *td, int rv)
 	while (p->p_lock > 0)
 		msleep(&p->p_lock, &p->p_mtx, PWAIT, "exithold", 0);
 
+	p->p_xstat = rv;	/* Let event handler change exit status */
 	PROC_UNLOCK(p);
 	/* Drain the limit callout while we don't have the proc locked */
 	callout_drain(&p->p_limco);
@@ -242,6 +243,7 @@ exit1(struct thread *td, int rv)
 	 * P_PPWAIT is set; we will wakeup the parent below.
 	 */
 	PROC_LOCK(p);
+	rv = p->p_xstat;	/* Event handler could change exit status */
 	stopprofclock(p);
 	p->p_flag &= ~(P_TRACED | P_PPWAIT);
 
@@ -424,7 +426,6 @@ exit1(struct thread *td, int rv)
 
 	/* Save exit status. */
 	PROC_LOCK(p);
-	p->p_xstat = rv;
 	p->p_xthread = td;
 
 	/* Tell the prison that we are gone. */
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 4 Alexander Leidinger freebsd_committer freebsd_triage 2010-11-22 09:08:39 UTC
State Changed
From-To: open->patched

Patched in -current. 


Comment 5 Alexander Leidinger freebsd_committer freebsd_triage 2010-11-22 09:08:39 UTC
Responsible Changed
From-To: dchagin->netchild

Patched in -current.
Comment 6 dfilter service freebsd_committer freebsd_triage 2011-03-02 09:53:33 UTC
Author: netchild
Date: Wed Mar  2 09:53:13 2011
New Revision: 219173
URL: http://svn.freebsd.org/changeset/base/219173

Log:
  MFC r215664:
    By using the 32-bit Linux version of Sun's Java Development Kit 1.6
    on FreeBSD (amd64), invocations of "javac" (or "java") eventually
    end with the output of "Killed" and exit code 137.
  
    This is caused by:
    1. After calling exec() in multithreaded linux program threads are not
       destroyed and continue running. They get killed after program being
       executed finishes.
  
    2. linux_exit_group doesn't return correct exit code when called not
       from group leader. Which happens regularly using sun jvm.
  
    The submitters fix this in a similar way to how NetBSD handles this.
  
    I took the PRs away from dchagin, who seems to be out of touch of
    this since a while (no response from him).
  
    The patches committed here are from [2], with some little modifications
    from me to the style.
  
    PR:                141439 [1], 144194 [2]
    Submitted by:        Stefan Schmidt <stefan.schmidt@stadtbuch.de>, gk
    Reviewed by:        rdivacky (in april 2010)
  
  MFC r215675:
    Do not take the process lock. The assignment to u_short inside the
    properly aligned structure is atomic on all supported architectures, and
    the thread that should see side-effect of assignment is the same thread
    that does assignment.
  
    Use a more appropriate conditional to detect the linux ABI.
  
    Suggested by:        kib

Modified:
  stable/8/sys/compat/linux/linux_emul.c
  stable/8/sys/compat/linux/linux_emul.h
  stable/8/sys/compat/linux/linux_misc.c
  stable/8/sys/kern/kern_exit.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/compat/linux/linux_emul.c
==============================================================================
--- stable/8/sys/compat/linux/linux_emul.c	Wed Mar  2 06:24:46 2011	(r219172)
+++ stable/8/sys/compat/linux/linux_emul.c	Wed Mar  2 09:53:13 2011	(r219173)
@@ -155,7 +155,7 @@ void
 linux_proc_exit(void *arg __unused, struct proc *p)
 {
 	struct linux_emuldata *em;
-	int error;
+	int error, shared_flags, shared_xstat;
 	struct thread *td = FIRST_THREAD_IN_PROC(p);
 	int *child_clear_tid;
 	struct proc *q, *nq;
@@ -187,6 +187,8 @@ linux_proc_exit(void *arg __unused, stru
 	}
 
 	EMUL_SHARED_WLOCK(&emul_shared_lock);
+	shared_flags = em->shared->flags;
+	shared_xstat = em->shared->xstat;
 	LIST_REMOVE(em, threads);
 
 	em->shared->refs--;
@@ -196,6 +198,9 @@ linux_proc_exit(void *arg __unused, stru
 	} else	
 		EMUL_SHARED_WUNLOCK(&emul_shared_lock);
 
+	if ((shared_flags & EMUL_SHARED_HASXSTAT) != 0)
+		p->p_xstat = shared_xstat;
+
 	if (child_clear_tid != NULL) {
 		struct linux_sys_futex_args cup;
 		int null = 0;
@@ -257,6 +262,10 @@ linux_proc_exec(void *arg __unused, stru
 	if (__predict_false(imgp->sysent == &elf_linux_sysvec
 	    && p->p_sysent != &elf_linux_sysvec))
 		linux_proc_init(FIRST_THREAD_IN_PROC(p), p->p_pid, 0);
+	if (__predict_false((p->p_sysent->sv_flags & SV_ABI_MASK) ==
+	    SV_ABI_LINUX))
+		/* Kill threads regardless of imgp->sysent value */
+		linux_kill_threads(FIRST_THREAD_IN_PROC(p), SIGKILL);
 	if (__predict_false(imgp->sysent != &elf_linux_sysvec
 	    && p->p_sysent == &elf_linux_sysvec)) {
 		struct linux_emuldata *em;
@@ -334,3 +343,29 @@ linux_set_tid_address(struct thread *td,
 	EMUL_UNLOCK(&emul_lock);
 	return 0;
 }
+
+void
+linux_kill_threads(struct thread *td, int sig)
+{
+	struct linux_emuldata *em, *td_em, *tmp_em;
+	struct proc *sp;
+
+	td_em = em_find(td->td_proc, EMUL_DONTLOCK);
+
+	KASSERT(td_em != NULL, ("linux_kill_threads: emuldata not found.\n"));
+
+	EMUL_SHARED_RLOCK(&emul_shared_lock);
+	LIST_FOREACH_SAFE(em, &td_em->shared->threads, threads, tmp_em) {
+		if (em->pid == td_em->pid)
+			continue;
+
+		sp = pfind(em->pid);
+		if ((sp->p_flag & P_WEXIT) == 0)
+			psignal(sp, sig);
+		PROC_UNLOCK(sp);
+#ifdef DEBUG
+		printf(LMSG("linux_kill_threads: kill PID %d\n"), em->pid);
+#endif
+	}
+	EMUL_SHARED_RUNLOCK(&emul_shared_lock);
+}

Modified: stable/8/sys/compat/linux/linux_emul.h
==============================================================================
--- stable/8/sys/compat/linux/linux_emul.h	Wed Mar  2 06:24:46 2011	(r219172)
+++ stable/8/sys/compat/linux/linux_emul.h	Wed Mar  2 09:53:13 2011	(r219173)
@@ -31,8 +31,12 @@
 #ifndef _LINUX_EMUL_H_
 #define	_LINUX_EMUL_H_
 
+#define EMUL_SHARED_HASXSTAT	0x01
+
 struct linux_emuldata_shared {
 	int	refs;
+	int	flags;
+	int	xstat;
 	pid_t	group_pid;
 
 	LIST_HEAD(, linux_emuldata) threads; /* head of list of linux threads */
@@ -76,6 +80,7 @@ int	linux_proc_init(struct thread *, pid
 void	linux_proc_exit(void *, struct proc *);
 void	linux_schedtail(void *, struct proc *);
 void	linux_proc_exec(void *, struct proc *, struct image_params *);
+void	linux_kill_threads(struct thread *, int);
 
 extern struct sx	emul_shared_lock;
 extern struct mtx	emul_lock;

Modified: stable/8/sys/compat/linux/linux_misc.c
==============================================================================
--- stable/8/sys/compat/linux/linux_misc.c	Wed Mar  2 06:24:46 2011	(r219172)
+++ stable/8/sys/compat/linux/linux_misc.c	Wed Mar  2 09:53:13 2011	(r219173)
@@ -1655,34 +1655,23 @@ linux_setdomainname(struct thread *td, s
 int
 linux_exit_group(struct thread *td, struct linux_exit_group_args *args)
 {
-	struct linux_emuldata *em, *td_em, *tmp_em;
-	struct proc *sp;
+	struct linux_emuldata *em;
 
 #ifdef DEBUG
 	if (ldebug(exit_group))
 		printf(ARGS(exit_group, "%i"), args->error_code);
 #endif
 
-	if (linux_use26(td)) {
-		td_em = em_find(td->td_proc, EMUL_DONTLOCK);
-
-		KASSERT(td_em != NULL, ("exit_group: emuldata not found.\n"));
-
-		EMUL_SHARED_RLOCK(&emul_shared_lock);
-		LIST_FOREACH_SAFE(em, &td_em->shared->threads, threads, tmp_em) {
-			if (em->pid == td_em->pid)
-				continue;
-
-			sp = pfind(em->pid);
-			psignal(sp, SIGKILL);
-			PROC_UNLOCK(sp);
-#ifdef DEBUG
-			printf(LMSG("linux_sys_exit_group: kill PID %d\n"), em->pid);
-#endif
-		}
-
-		EMUL_SHARED_RUNLOCK(&emul_shared_lock);
+	em = em_find(td->td_proc, EMUL_DONTLOCK);
+	if (em->shared->refs > 1) {
+		EMUL_SHARED_WLOCK(&emul_shared_lock);
+		em->shared->flags |= EMUL_SHARED_HASXSTAT;
+		em->shared->xstat = W_EXITCODE(args->error_code, 0);
+		EMUL_SHARED_WUNLOCK(&emul_shared_lock);
+		if (linux_use26(td))
+			linux_kill_threads(td, SIGKILL);
 	}
+
 	/*
 	 * XXX: we should send a signal to the parent if
 	 * SIGNAL_EXIT_GROUP is set. We ignore that (temporarily?)

Modified: stable/8/sys/kern/kern_exit.c
==============================================================================
--- stable/8/sys/kern/kern_exit.c	Wed Mar  2 06:24:46 2011	(r219172)
+++ stable/8/sys/kern/kern_exit.c	Wed Mar  2 09:53:13 2011	(r219173)
@@ -200,6 +200,7 @@ exit1(struct thread *td, int rv)
 	while (p->p_lock > 0)
 		msleep(&p->p_lock, &p->p_mtx, PWAIT, "exithold", 0);
 
+	p->p_xstat = rv;	/* Let event handler change exit status */
 	PROC_UNLOCK(p);
 	/* Drain the limit callout while we don't have the proc locked */
 	callout_drain(&p->p_limco);
@@ -242,6 +243,7 @@ exit1(struct thread *td, int rv)
 	 * P_PPWAIT is set; we will wakeup the parent below.
 	 */
 	PROC_LOCK(p);
+	rv = p->p_xstat;	/* Event handler could change exit status */
 	stopprofclock(p);
 	p->p_flag &= ~(P_TRACED | P_PPWAIT);
 
@@ -421,7 +423,6 @@ exit1(struct thread *td, int rv)
 
 	/* Save exit status. */
 	PROC_LOCK(p);
-	p->p_xstat = rv;
 	p->p_xthread = td;
 
 	/* Tell the prison that we are gone. */
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 Alexander Leidinger freebsd_committer freebsd_triage 2011-03-02 10:12:25 UTC
State Changed
From-To: patched->closed

The fix is now in 8-stable.