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 <p...@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, _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
