Date: Tue, 20 May 2014 23:35:01 +0800 From: Julian Elischer <julian@freebsd.org> To: Konstantin Belousov <kib@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r266464 - in head/sys: kern sys vm Message-ID: <537B7625.4080908@freebsd.org> In-Reply-To: <201405200919.s4K9JZvg087765@svn.freebsd.org> References: <201405200919.s4K9JZvg087765@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/20/14, 5:19 PM, Konstantin Belousov wrote: > Author: kib > Date: Tue May 20 09:19:35 2014 > New Revision: 266464 > URL: http://svnweb.freebsd.org/changeset/base/266464 > > Log: > When exec_new_vmspace() decides that current vmspace cannot be reused > on execve(2), it calls vmspace_exec(), which frees the current > vmspace. The thread executing an exec syscall gets new vmspace > assigned, and old vmspace is freed if only referenced by the current > process. The free operation includes pmap_release(), which > de-constructs the paging structures used by hardware. > > If the calling process is multithreaded, other threads are suspended > in the thread_suspend_check(), and need to be unsuspended and run to > be able to exit on successfull exec. Now, since the old vmspace is > destroyed, paging structures are invalid, threads are resumed on the > non-existent pmaps (page tables), which leads to triple fault on x86. > > To fix, postpone the free of old vmspace until the threads are resumed > and exited. To avoid modifications to all image activators all of > which use exec_new_vmspace(), memoize the current (old) vmspace in > kern_execve(), and notify it about the need to call vmspace_free() > with a thread-private flag TDP_EXECVMSPC. I was sure that we covered this case at some time in the past.. I think all threads but the caller were killed at the kernel boundary and exec waited for that to happen. > > http://bugs.debian.org/743141 > > Reported by: Ivo De Decker <ivo.dedecker@ugent.be> through secteam > Sponsored by: The FreeBSD Foundation > MFC after: 3 days > > Modified: > head/sys/kern/kern_exec.c > head/sys/sys/proc.h > head/sys/vm/vm_map.c > > Modified: head/sys/kern/kern_exec.c > ============================================================================== > --- head/sys/kern/kern_exec.c Tue May 20 03:00:20 2014 (r266463) > +++ head/sys/kern/kern_exec.c Tue May 20 09:19:35 2014 (r266464) > @@ -282,6 +282,7 @@ kern_execve(td, args, mac_p) > struct mac *mac_p; > { > struct proc *p = td->td_proc; > + struct vmspace *oldvmspace; > int error; > > AUDIT_ARG_ARGV(args->begin_argv, args->argc, > @@ -298,6 +299,8 @@ kern_execve(td, args, mac_p) > PROC_UNLOCK(p); > } > > + KASSERT((td->td_pflags & TDP_EXECVMSPC) == 0, ("nested execve")); > + oldvmspace = td->td_proc->p_vmspace; > error = do_execve(td, args, mac_p); > > if (p->p_flag & P_HADTHREADS) { > @@ -312,6 +315,12 @@ kern_execve(td, args, mac_p) > thread_single_end(); > PROC_UNLOCK(p); > } > + if ((td->td_pflags & TDP_EXECVMSPC) != 0) { > + KASSERT(td->td_proc->p_vmspace != oldvmspace, > + ("oldvmspace still used")); > + vmspace_free(oldvmspace); > + td->td_pflags &= ~TDP_EXECVMSPC; > + } > > return (error); > } > > Modified: head/sys/sys/proc.h > ============================================================================== > --- head/sys/sys/proc.h Tue May 20 03:00:20 2014 (r266463) > +++ head/sys/sys/proc.h Tue May 20 09:19:35 2014 (r266464) > @@ -429,6 +429,7 @@ do { \ > #define TDP_NERRNO 0x08000000 /* Last errno is already in td_errno */ > #define TDP_UIOHELD 0x10000000 /* Current uio has pages held in td_ma */ > #define TDP_DEVMEMIO 0x20000000 /* Accessing memory for /dev/mem */ > +#define TDP_EXECVMSPC 0x40000000 /* Execve destroyed old vmspace */ > > /* > * Reasons that the current thread can not be run yet. > > Modified: head/sys/vm/vm_map.c > ============================================================================== > --- head/sys/vm/vm_map.c Tue May 20 03:00:20 2014 (r266463) > +++ head/sys/vm/vm_map.c Tue May 20 09:19:35 2014 (r266464) > @@ -3758,6 +3758,8 @@ vmspace_exec(struct proc *p, vm_offset_t > struct vmspace *oldvmspace = p->p_vmspace; > struct vmspace *newvmspace; > > + KASSERT((curthread->td_pflags & TDP_EXECVMSPC) == 0, > + ("vmspace_exec recursed")); > newvmspace = vmspace_alloc(minuser, maxuser, NULL); > if (newvmspace == NULL) > return (ENOMEM); > @@ -3774,7 +3776,7 @@ vmspace_exec(struct proc *p, vm_offset_t > PROC_VMSPACE_UNLOCK(p); > if (p == curthread->td_proc) > pmap_activate(curthread); > - vmspace_free(oldvmspace); > + curthread->td_pflags |= TDP_EXECVMSPC; > return (0); > } > > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?537B7625.4080908>