Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 23 Sep 2020 18:03:07 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r366085 - in head/sys: compat/cloudabi compat/freebsd32 compat/linux kern sys
Message-ID:  <202009231803.08NI371w091981@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Wed Sep 23 18:03:07 2020
New Revision: 366085
URL: https://svnweb.freebsd.org/changeset/base/366085

Log:
  Do not leak oldvmspace if image activation failed
  
  and current address space is already destroyed, so kern_execve()
  terminates the process.
  
  While there, clean up some internals of post_execve() inlined in init_main.
  
  Reported by:	Peter <pmc@citylink.dinoex.sub.org>
  Reviewed by:	markj
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week
  Differential revision:	https://reviews.freebsd.org/D26525

Modified:
  head/sys/compat/cloudabi/cloudabi_proc.c
  head/sys/compat/freebsd32/freebsd32_misc.c
  head/sys/compat/linux/linux_emul.c
  head/sys/kern/init_main.c
  head/sys/kern/kern_exec.c
  head/sys/sys/imgact.h
  head/sys/sys/syscallsubr.h

Modified: head/sys/compat/cloudabi/cloudabi_proc.c
==============================================================================
--- head/sys/compat/cloudabi/cloudabi_proc.c	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/compat/cloudabi/cloudabi_proc.c	Wed Sep 23 18:03:07 2020	(r366085)
@@ -57,7 +57,7 @@ cloudabi_sys_proc_exec(struct thread *td,
 	    uap->fds, uap->fds_len);
 	if (error == 0) {
 		args.fd = uap->fd;
-		error = kern_execve(td, &args, NULL);
+		error = kern_execve(td, &args, NULL, oldvmspace);
 	}
 	post_execve(td, error, oldvmspace);
 	return (error);

Modified: head/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- head/sys/compat/freebsd32/freebsd32_misc.c	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/compat/freebsd32/freebsd32_misc.c	Wed Sep 23 18:03:07 2020	(r366085)
@@ -440,7 +440,7 @@ freebsd32_execve(struct thread *td, struct freebsd32_e
 	error = freebsd32_exec_copyin_args(&eargs, uap->fname, UIO_USERSPACE,
 	    uap->argv, uap->envv);
 	if (error == 0)
-		error = kern_execve(td, &eargs, NULL);
+		error = kern_execve(td, &eargs, NULL, oldvmspace);
 	post_execve(td, error, oldvmspace);
 	return (error);
 }
@@ -459,7 +459,7 @@ freebsd32_fexecve(struct thread *td, struct freebsd32_
 	    uap->argv, uap->envv);
 	if (error == 0) {
 		eargs.fd = uap->fd;
-		error = kern_execve(td, &eargs, NULL);
+		error = kern_execve(td, &eargs, NULL, oldvmspace);
 	}
 	post_execve(td, error, oldvmspace);
 	return (error);

Modified: head/sys/compat/linux/linux_emul.c
==============================================================================
--- head/sys/compat/linux/linux_emul.c	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/compat/linux/linux_emul.c	Wed Sep 23 18:03:07 2020	(r366085)
@@ -256,7 +256,7 @@ linux_common_execve(struct thread *td, struct image_ar
 	if (error != 0)
 		return (error);
 
-	error = kern_execve(td, eargs, NULL);
+	error = kern_execve(td, eargs, NULL, oldvmspace);
 	post_execve(td, error, oldvmspace);
 	if (error != EJUSTRETURN)
 		return (error);

Modified: head/sys/kern/init_main.c
==============================================================================
--- head/sys/kern/init_main.c	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/kern/init_main.c	Wed Sep 23 18:03:07 2020	(r366085)
@@ -752,16 +752,11 @@ start_init(void *dummy)
 		KASSERT((td->td_pflags & TDP_EXECVMSPC) == 0,
 		    ("nested execve"));
 		oldvmspace = td->td_proc->p_vmspace;
-		error = kern_execve(td, &args, NULL);
+		error = kern_execve(td, &args, NULL, oldvmspace);
 		KASSERT(error != 0,
 		    ("kern_execve returned success, not EJUSTRETURN"));
 		if (error == EJUSTRETURN) {
-			if ((td->td_pflags & TDP_EXECVMSPC) != 0) {
-				KASSERT(p->p_vmspace != oldvmspace,
-				    ("oldvmspace still used"));
-				vmspace_free(oldvmspace);
-				td->td_pflags &= ~TDP_EXECVMSPC;
-			}
+			exec_cleanup(td, oldvmspace);
 			free(free_init_path, M_TEMP);
 			TSEXIT();
 			return;

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/kern/kern_exec.c	Wed Sep 23 18:03:07 2020	(r366085)
@@ -118,7 +118,7 @@ static int sysctl_kern_ps_strings(SYSCTL_HANDLER_ARGS)
 static int sysctl_kern_usrstack(SYSCTL_HANDLER_ARGS);
 static int sysctl_kern_stackprot(SYSCTL_HANDLER_ARGS);
 static int do_execve(struct thread *td, struct image_args *args,
-    struct mac *mac_p);
+    struct mac *mac_p, struct vmspace *oldvmspace);
 
 /* XXX This should be vm_size_t. */
 SYSCTL_PROC(_kern, KERN_PS_STRINGS, ps_strings, CTLTYPE_ULONG|CTLFLAG_RD|
@@ -223,7 +223,7 @@ sys_execve(struct thread *td, struct execve_args *uap)
 	error = exec_copyin_args(&args, uap->fname, UIO_USERSPACE,
 	    uap->argv, uap->envv);
 	if (error == 0)
-		error = kern_execve(td, &args, NULL);
+		error = kern_execve(td, &args, NULL, oldvmspace);
 	post_execve(td, error, oldvmspace);
 	return (error);
 }
@@ -249,7 +249,7 @@ sys_fexecve(struct thread *td, struct fexecve_args *ua
 	    uap->argv, uap->envv);
 	if (error == 0) {
 		args.fd = uap->fd;
-		error = kern_execve(td, &args, NULL);
+		error = kern_execve(td, &args, NULL, oldvmspace);
 	}
 	post_execve(td, error, oldvmspace);
 	return (error);
@@ -278,7 +278,7 @@ sys___mac_execve(struct thread *td, struct __mac_execv
 	error = exec_copyin_args(&args, uap->fname, UIO_USERSPACE,
 	    uap->argv, uap->envv);
 	if (error == 0)
-		error = kern_execve(td, &args, uap->mac_p);
+		error = kern_execve(td, &args, uap->mac_p, oldvmspace);
 	post_execve(td, error, oldvmspace);
 	return (error);
 #else
@@ -326,30 +326,26 @@ post_execve(struct thread *td, int error, struct vmspa
 			thread_single_end(p, SINGLE_BOUNDARY);
 		PROC_UNLOCK(p);
 	}
-	if ((td->td_pflags & TDP_EXECVMSPC) != 0) {
-		KASSERT(p->p_vmspace != oldvmspace,
-		    ("oldvmspace still used"));
-		vmspace_free(oldvmspace);
-		td->td_pflags &= ~TDP_EXECVMSPC;
-	}
+	exec_cleanup(td, oldvmspace);
 }
 
 /*
- * XXX: kern_execve has the astonishing property of not always returning to
+ * kern_execve() has the astonishing property of not always returning to
  * the caller.  If sufficiently bad things happen during the call to
  * do_execve(), it can end up calling exit1(); as a result, callers must
  * avoid doing anything which they might need to undo (e.g., allocating
  * memory).
  */
 int
-kern_execve(struct thread *td, struct image_args *args, struct mac *mac_p)
+kern_execve(struct thread *td, struct image_args *args, struct mac *mac_p,
+    struct vmspace *oldvmspace)
 {
 
 	AUDIT_ARG_ARGV(args->begin_argv, args->argc,
 	    exec_args_get_begin_envv(args) - args->begin_argv);
 	AUDIT_ARG_ENVV(exec_args_get_begin_envv(args), args->envc,
 	    args->endp - exec_args_get_begin_envv(args));
-	return (do_execve(td, args, mac_p));
+	return (do_execve(td, args, mac_p, oldvmspace));
 }
 
 /*
@@ -357,7 +353,8 @@ kern_execve(struct thread *td, struct image_args *args
  * userspace pointers from the passed thread.
  */
 static int
-do_execve(struct thread *td, struct image_args *args, struct mac *mac_p)
+do_execve(struct thread *td, struct image_args *args, struct mac *mac_p,
+    struct vmspace *oldvmspace)
 {
 	struct proc *p = td->td_proc;
 	struct nameidata nd;
@@ -949,6 +946,7 @@ exec_fail:
 
 	if (error && imgp->vmspace_destroyed) {
 		/* sorry, no more process anymore. exit gracefully */
+		exec_cleanup(td, oldvmspace);
 		exit1(td, 0, SIGABRT);
 		/* NOT REACHED */
 	}
@@ -965,6 +963,20 @@ exec_fail:
 	 * registers unmodified when returning EJUSTRETURN.
 	 */
 	return (error == 0 ? EJUSTRETURN : error);
+}
+
+void
+exec_cleanup(struct thread *td, struct vmspace *oldvmspace)
+{
+	struct proc *p;
+
+	p = td->td_proc;
+	if ((td->td_pflags & TDP_EXECVMSPC) != 0) {
+		KASSERT(p->p_vmspace != oldvmspace,
+		    ("oldvmspace still used"));
+		vmspace_free(oldvmspace);
+		td->td_pflags &= ~TDP_EXECVMSPC;
+	}
 }
 
 int

Modified: head/sys/sys/imgact.h
==============================================================================
--- head/sys/sys/imgact.h	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/sys/imgact.h	Wed Sep 23 18:03:07 2020	(r366085)
@@ -112,6 +112,7 @@ int	exec_args_adjust_args(struct image_args *args, siz
 	    ssize_t extend);
 char	*exec_args_get_begin_envv(struct image_args *args);
 int	exec_check_permissions(struct image_params *);
+void	exec_cleanup(struct thread *td, struct vmspace *);
 int	exec_copyout_strings(struct image_params *, uintptr_t *);
 void	exec_free_args(struct image_args *);
 int	exec_new_vmspace(struct image_params *, struct sysentvec *);

Modified: head/sys/sys/syscallsubr.h
==============================================================================
--- head/sys/sys/syscallsubr.h	Wed Sep 23 17:42:19 2020	(r366084)
+++ head/sys/sys/syscallsubr.h	Wed Sep 23 18:03:07 2020	(r366085)
@@ -63,6 +63,7 @@ struct stat;
 struct thr_param;
 struct uio;
 struct vm_map;
+struct vmspace;
 
 typedef int (*mmap_check_fp_fn)(struct file *, int, int, int);
 
@@ -129,7 +130,7 @@ int	kern_cpuset_setid(struct thread *td, cpuwhich_t wh
 	    id_t id, cpusetid_t setid);
 int	kern_dup(struct thread *td, u_int mode, int flags, int old, int new);
 int	kern_execve(struct thread *td, struct image_args *args,
-	    struct mac *mac_p);
+	    struct mac *mac_p, struct vmspace *oldvmspace);
 int	kern_fchmodat(struct thread *td, int fd, const char *path,
 	    enum uio_seg pathseg, mode_t mode, int flag);
 int	kern_fchownat(struct thread *td, int fd, const char *path,



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