Add helper functions to copy strings into struct image_args.

Given a zeroed struct image_args with an allocated buf member,
exec_args_add_fname() must be called to install a file name (or NULL).
Then zero or more calls to exec_args_add_env() followed by zero or
more calls to exec_args_add_env(). exec_args_adjust_args() may be
called after args and/or env to allow an interpreter to be prepended to
the argument list.

To allow code reuse when adding arg and env variables, begin_envv
should be accessed with the accessor exec_args_get_begin_envv()
which handles the case when no environment entries have been added.

Use these functions to simplify exec_copyin_args() and
freebsd32_exec_copyin_args().

Reviewed by:	kib
Obtained from:	CheriBSD
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D15468

sys/compat/cloudabi32/cloudabi32_module.c

@@ -54,7 +54,7 @@ cloudabi32_copyout_strings(struct image_params *imgp)
 
 	/* Copy out program arguments. */
 	args = imgp->args;
-	len = args->begin_envv - args->begin_argv;
+	len = exec_args_get_begin_envv(args) - args->begin_argv;
 	begin = rounddown2(imgp->sysent->sv_usrstack - len, sizeof(register_t));
 	copyout(args->begin_argv, (void *)begin, len);
 	return ((register_t *)begin);
@@ -109,7 +109,8 @@ cloudabi32_fixup(register_t **stack_base, struct image_params *imgp)
 	 * exec_copyin_data_fds(). Undo this by reducing the length.
 	 */
 	args = (Elf32_Auxargs *)imgp->auxargs;
-	argdatalen = imgp->args->begin_envv - imgp->args->begin_argv;
+	argdatalen = exec_args_get_begin_envv(imgp->args) -
+	    imgp->args->begin_argv;
 	if (argdatalen > 0)
 		--argdatalen;
 

sys/compat/cloudabi64/cloudabi64_module.c

@@ -54,7 +54,7 @@ cloudabi64_copyout_strings(struct image_params *imgp)
 
 	/* Copy out program arguments. */
 	args = imgp->args;
-	len = args->begin_envv - args->begin_argv;
+	len = exec_args_get_begin_envv(args) - args->begin_argv;
 	begin = rounddown2(imgp->sysent->sv_usrstack - len, sizeof(register_t));
 	copyout(args->begin_argv, (void *)begin, len);
 	return ((register_t *)begin);
@@ -109,7 +109,8 @@ cloudabi64_fixup(register_t **stack_base, struct image_params *imgp)
 	 * exec_copyin_data_fds(). Undo this by reducing the length.
 	 */
 	args = (Elf64_Auxargs *)imgp->auxargs;
-	argdatalen = imgp->args->begin_envv - imgp->args->begin_argv;
+	argdatalen = exec_args_get_begin_envv(imgp->args) -
+	    imgp->args->begin_argv;
 	if (argdatalen > 0)
 		--argdatalen;
 

sys/compat/freebsd32/freebsd32_misc.c

@@ -337,7 +337,6 @@ freebsd32_exec_copyin_args(struct image_args *args, const char *fname,
 {
 	char *argp, *envp;
 	u_int32_t *p32, arg;
-	size_t length;
 	int error;
 
 	bzero(args, sizeof(*args));
@@ -355,19 +354,9 @@ freebsd32_exec_copyin_args(struct image_args *args, const char *fname,
 	/*
 	 * Copy the file name.
 	 */
-	if (fname != NULL) {
+	error = exec_args_add_fname(args, fname, segflg);
-		args->fname = args->buf;
-		error = (segflg == UIO_SYSSPACE) ?
-		    copystr(fname, args->fname, PATH_MAX, &length) :
-		    copyinstr(fname, args->fname, PATH_MAX, &length);
 	if (error != 0)
 		goto err_exit;
-	} else
-		length = 0;
-
-	args->begin_argv = args->buf + length;
-	args->endp = args->begin_argv;
-	args->stringspace = ARG_MAX;
 
 	/*
 	 * extract arguments first
@@ -380,18 +369,10 @@ freebsd32_exec_copyin_args(struct image_args *args, const char *fname,
 		if (arg == 0)
 			break;
 		argp = PTRIN(arg);
-		error = copyinstr(argp, args->endp, args->stringspace, &length);
+		error = exec_args_add_arg(args, argp, UIO_USERSPACE);
-		if (error) {
+		if (error != 0)
-			if (error == ENAMETOOLONG)
-				error = E2BIG;
 			goto err_exit;
 	}
-		args->stringspace -= length;
-		args->endp += length;
-		args->argc++;
-	}
-			
-	args->begin_envv = args->endp;
 			
 	/*
 	 * extract environment strings
@@ -405,17 +386,10 @@ freebsd32_exec_copyin_args(struct image_args *args, const char *fname,
 			if (arg == 0)
 				break;
 			envp = PTRIN(arg);
-			error = copyinstr(envp, args->endp, args->stringspace,
+			error = exec_args_add_env(args, envp, UIO_USERSPACE);
-			    &length);
+			if (error != 0)
-			if (error) {
-				if (error == ENAMETOOLONG)
-					error = E2BIG;
 				goto err_exit;
 		}
-			args->stringspace -= length;
-			args->endp += length;
-			args->envc++;
-		}
 	}
 
 	return (0);

sys/kern/imgact_binmisc.c

@@ -649,22 +649,13 @@ imgact_binmisc_exec(struct image_params *imgp)
 		s++;
 	}
 
-	/* Check to make sure we won't overrun the stringspace. */
+	/* Make room for the interpreter */
-	if (offset > imgp->args->stringspace) {
+	error = exec_args_adjust_args(imgp->args, 0, offset);
+	if (error != 0) {
 		sx_sunlock(&interp_list_sx);
-		error = E2BIG;
 		goto done;
 	}
 
-	/* Make room for the interpreter */
-	bcopy(imgp->args->begin_argv, imgp->args->begin_argv + offset,
-	    imgp->args->endp - imgp->args->begin_argv);
-
-	/* Adjust everything by the offset. */
-	imgp->args->begin_envv += offset;
-	imgp->args->endp += offset;
-	imgp->args->stringspace -= offset;
-
 	/* Add the new argument(s) in the count. */
 	imgp->args->argc += ibe->ibe_interp_argcnt;
 

sys/kern/imgact_shell.c

@@ -196,20 +196,13 @@ exec_shell_imgact(struct image_params *imgp)
 	length = (imgp->args->argc == 0) ? 0 :
 	    strlen(imgp->args->begin_argv) + 1;		/* bytes to delete */
 
-	if (offset > imgp->args->stringspace + length) {
+	error = exec_args_adjust_args(imgp->args, length, offset);
+	if (error != 0) {
 		if (sname != NULL)
 			sbuf_delete(sname);
-		return (E2BIG);
+		return (error);
 	}
 
-	bcopy(imgp->args->begin_argv + length, imgp->args->begin_argv + offset,
-	    imgp->args->endp - (imgp->args->begin_argv + length));
-
-	offset -= length;		/* calculate actual adjustment */
-	imgp->args->begin_envv += offset;
-	imgp->args->endp += offset;
-	imgp->args->stringspace -= offset;
-
 	/*
 	 * If there was no arg[0] when we started, then the interpreter_name
 	 * is adding an argument (instead of replacing the arg[0] we started

sys/kern/kern_exec.c

@@ -345,9 +345,9 @@ kern_execve(struct thread *td, struct image_args *args, struct mac *mac_p)
 {
 
 	AUDIT_ARG_ARGV(args->begin_argv, args->argc,
-	    args->begin_envv - args->begin_argv);
+	    exec_args_get_begin_envv(args) - args->begin_argv);
-	AUDIT_ARG_ENVV(args->begin_envv, args->envc,
+	AUDIT_ARG_ENVV(exec_args_get_begin_envv(args), args->envc,
-	    args->endp - args->begin_envv);
+	    args->endp - exec_args_get_begin_envv(args));
 	return (do_execve(td, args, mac_p));
 }
 
@@ -717,7 +717,7 @@ interpret:
 	/*
 	 * Malloc things before we need locks.
 	 */
-	i = imgp->args->begin_envv - imgp->args->begin_argv;
+	i = exec_args_get_begin_envv(imgp->args) - imgp->args->begin_argv;
 	/* Cache arguments if they fit inside our allowance */
 	if (ps_arg_cache_limit >= i + sizeof(struct pargs)) {
 		newargs = pargs_alloc(i);
@@ -1171,9 +1171,8 @@ int
 exec_copyin_args(struct image_args *args, const char *fname,
     enum uio_seg segflg, char **argv, char **envv)
 {
-	u_long argp, envp;
+	u_long arg, env;
 	int error;
-	size_t length;
 
 	bzero(args, sizeof(*args));
 	if (argv == NULL)
@@ -1190,68 +1189,44 @@ exec_copyin_args(struct image_args *args, const char *fname,
 	/*
 	 * Copy the file name.
 	 */
-	if (fname != NULL) {
+	error = exec_args_add_fname(args, fname, segflg);
-		args->fname = args->buf;
-		error = (segflg == UIO_SYSSPACE) ?
-		    copystr(fname, args->fname, PATH_MAX, &length) :
-		    copyinstr(fname, args->fname, PATH_MAX, &length);
 	if (error != 0)
 		goto err_exit;
-	} else
-		length = 0;
-
-	args->begin_argv = args->buf + length;
-	args->endp = args->begin_argv;
-	args->stringspace = ARG_MAX;
 
 	/*
 	 * extract arguments first
 	 */
 	for (;;) {
-		error = fueword(argv++, &argp);
+		error = fueword(argv++, &arg);
 		if (error == -1) {
 			error = EFAULT;
 			goto err_exit;
 		}
-		if (argp == 0)
+		if (arg == 0)
 			break;
-		error = copyinstr((void *)(uintptr_t)argp, args->endp,
+		error = exec_args_add_arg(args, (char *)(uintptr_t)arg,
-		    args->stringspace, &length);
+		    UIO_USERSPACE);
-		if (error != 0) {
+		if (error != 0)
-			if (error == ENAMETOOLONG) 
-				error = E2BIG;
 			goto err_exit;
 	}
-		args->stringspace -= length;
-		args->endp += length;
-		args->argc++;
-	}
-
-	args->begin_envv = args->endp;
 
 	/*
 	 * extract environment strings
 	 */
 	if (envv) {
 		for (;;) {
-			error = fueword(envv++, &envp);
+			error = fueword(envv++, &env);
 			if (error == -1) {
 				error = EFAULT;
 				goto err_exit;
 			}
-			if (envp == 0)
+			if (env == 0)
 				break;
-			error = copyinstr((void *)(uintptr_t)envp,
+			error = exec_args_add_env(args,
-			    args->endp, args->stringspace, &length);
+			    (char *)(uintptr_t)env, UIO_USERSPACE);
-			if (error != 0) {
+			if (error != 0)
-				if (error == ENAMETOOLONG)
-					error = E2BIG;
 				goto err_exit;
 		}
-			args->stringspace -= length;
-			args->endp += length;
-			args->envc++;
-		}
 	}
 
 	return (0);
@@ -1305,8 +1280,6 @@ exec_copyin_data_fds(struct thread *td, struct image_args *args,
 		/* No argument buffer provided. */
 		args->endp = args->begin_argv;
 	}
-	/* There are no environment variables. */
-	args->begin_envv = args->endp;
 
 	/* Create new file descriptor table. */
 	kfds = malloc(fdslen * sizeof(int), M_TEMP, M_WAITOK);
@@ -1462,6 +1435,126 @@ exec_free_args(struct image_args *args)
 		fdescfree_remapped(args->fdp);
 }
 
+/*
+ * A set to functions to fill struct image args.
+ *
+ * NOTE: exec_args_add_fname() must be called (possibly with a NULL
+ * fname) before the other functions.  All exec_args_add_arg() calls must
+ * be made before any exec_args_add_env() calls.  exec_args_adjust_args()
+ * may be called any time after exec_args_add_fname().
+ *
+ * exec_args_add_fname() - install path to be executed
+ * exec_args_add_arg() - append an argument string
+ * exec_args_add_env() - append an env string
+ * exec_args_adjust_args() - adjust location of the argument list to
+ *                           allow new arguments to be prepended
+ */
+int
+exec_args_add_fname(struct image_args *args, const char *fname,
+    enum uio_seg segflg)
+{
+	int error;
+	size_t length;
+
+	KASSERT(args->fname == NULL, ("fname already appended"));
+	KASSERT(args->endp == NULL, ("already appending to args"));
+
+	if (fname != NULL) {
+		args->fname = args->buf;
+		error = segflg == UIO_SYSSPACE ?
+		    copystr(fname, args->fname, PATH_MAX, &length) :
+		    copyinstr(fname, args->fname, PATH_MAX, &length);
+		if (error != 0)
+			return (error == ENAMETOOLONG ? E2BIG : error);
+	} else
+		length = 0;
+
+	/* Set up for _arg_*()/_env_*() */
+	args->endp = args->buf + length;
+	/* begin_argv must be set and kept updated */
+	args->begin_argv = args->endp;
+	KASSERT(exec_map_entry_size - length >= ARG_MAX,
+	    ("too little space remaining for arguments %zu < %zu",
+	    exec_map_entry_size - length, (size_t)ARG_MAX));
+	args->stringspace = ARG_MAX;
+
+	return (0);
+}
+
+static int
+exec_args_add_str(struct image_args *args, const char *str,
+    enum uio_seg segflg, int *countp)
+{
+	int error;
+	size_t length;
+
+	KASSERT(args->endp != NULL, ("endp not initialized"));
+	KASSERT(args->begin_argv != NULL, ("begin_argp not initialized"));
+
+	error = (segflg == UIO_SYSSPACE) ?
+	    copystr(str, args->endp, args->stringspace, &length) :
+	    copyinstr(str, args->endp, args->stringspace, &length);
+	if (error != 0)
+		return (error == ENAMETOOLONG ? E2BIG : error);
+	args->stringspace -= length;
+	args->endp += length;
+	(*countp)++;
+
+	return (0);
+}
+
+int
+exec_args_add_arg(struct image_args *args, const char *argp,
+    enum uio_seg segflg)
+{
+
+	KASSERT(args->envc == 0, ("appending args after env"));
+
+	return (exec_args_add_str(args, argp, segflg, &args->argc));
+}
+
+int
+exec_args_add_env(struct image_args *args, const char *envp,
+    enum uio_seg segflg)
+{
+
+	if (args->envc == 0)
+		args->begin_envv = args->endp;
+
+	return (exec_args_add_str(args, envp, segflg, &args->envc));
+}
+
+int
+exec_args_adjust_args(struct image_args *args, size_t consume, ssize_t extend)
+{
+	ssize_t offset;
+
+	KASSERT(args->endp != NULL, ("endp not initialized"));
+	KASSERT(args->begin_argv != NULL, ("begin_argp not initialized"));
+
+	offset = extend - consume;
+	if (args->stringspace < offset)
+		return (E2BIG);
+	memmove(args->begin_argv + extend, args->begin_argv + consume,
+	    args->endp - args->begin_argv + consume);
+	if (args->envc > 0)
+		args->begin_envv += offset;
+	args->endp += offset;
+	args->stringspace -= offset;
+	return (0);
+}
+
+char *
+exec_args_get_begin_envv(struct image_args *args)
+{
+
+	KASSERT(args->endp != NULL, ("endp not initialized"));
+
+	if (args->envc > 0)
+		return (args->begin_envv);
+	return (args->endp);
+}
+
 /*
  * Copy strings out to the new process address space, constructing new arg
  * and env vector tables. Return a pointer to the base so that it can be used

sys/sys/imgact.h

@@ -46,7 +46,8 @@ struct image_args {
 	char *buf;		/* pointer to string buffer */
 	void *bufkva;		/* cookie for string buffer KVA */
 	char *begin_argv;	/* beginning of argv in buf */
-	char *begin_envv;	/* beginning of envv in buf */
+	char *begin_envv;	/* (interal use only) beginning of envv in buf,
+				 * access with exec_args_get_begin_envv(). */
 	char *endp;		/* current `end' pointer of arg & env strings */
 	char *fname;            /* pointer to filename of executable (system space) */
 	char *fname_buf;	/* pointer to optional malloc(M_TEMP) buffer */
@@ -96,6 +97,15 @@ struct thread;
 struct vmspace;
 
 int	exec_alloc_args(struct image_args *);
+int	exec_args_add_arg(struct image_args *args, const char *argp,
+	    enum uio_seg segflg);
+int	exec_args_add_env(struct image_args *args, const char *envp,
+	    enum uio_seg segflg);
+int	exec_args_add_fname(struct image_args *args, const char *fname,
+	    enum uio_seg segflg);
+int	exec_args_adjust_args(struct image_args *args, size_t consume,
+	    ssize_t extend);
+char	*exec_args_get_begin_envv(struct image_args *args);
 int	exec_check_permissions(struct image_params *);
 register_t *exec_copyout_strings(struct image_params *);
 void	exec_free_args(struct image_args *);