svn commit: r215664 - in head/sys: compat/linux kern

[Gleb Kurtsou]

On (22/11/2010 11:31), Kostik Belousov wrote:
> On Mon, Nov 22, 2010 at 09:07:00AM +0000, Alexander Leidinger wrote:
> > 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 at 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);
> > +	}
> Why is process lock taken there ? 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.
> 
> > +
> >  	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);
> This is better expressed by
> 	if ((p->p_sysent->sv_flags & SV_ABI_MASK) == SV_ABI_LINUX)
> 
> Regardless of this mostly cosmetic issue, this is racy. Other
> linux thread in the same process might do an execve(3).
> More, if execve(3) call fails, then you return into the process
> that lacks all threads except the one that called execve(3).

execve(3) races in linuxulator are known, and that's not the only case:
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/142082

But fixing it is not easy, as you've noted in other email current hook
mechanism is not good enough for it.