Date: Thu, 29 Nov 2018 21:00:56 +0000 (UTC) From: Brooks Davis <brooks@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r341263 - in head/sys: compat/cloudabi32 compat/cloudabi64 compat/freebsd32 kern sys Message-ID: <201811292100.wATL0u2O069306@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: brooks Date: Thu Nov 29 21:00:56 2018 New Revision: 341263 URL: https://svnweb.freebsd.org/changeset/base/341263 Log: 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 Modified: head/sys/compat/cloudabi32/cloudabi32_module.c head/sys/compat/cloudabi64/cloudabi64_module.c head/sys/compat/freebsd32/freebsd32_misc.c head/sys/kern/imgact_binmisc.c head/sys/kern/imgact_shell.c head/sys/kern/kern_exec.c head/sys/sys/imgact.h Modified: head/sys/compat/cloudabi32/cloudabi32_module.c ============================================================================== --- head/sys/compat/cloudabi32/cloudabi32_module.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/compat/cloudabi32/cloudabi32_module.c Thu Nov 29 21:00:56 2018 (r341263) @@ -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 * 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; Modified: head/sys/compat/cloudabi64/cloudabi64_module.c ============================================================================== --- head/sys/compat/cloudabi64/cloudabi64_module.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/compat/cloudabi64/cloudabi64_module.c Thu Nov 29 21:00:56 2018 (r341263) @@ -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 * 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; Modified: head/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- head/sys/compat/freebsd32/freebsd32_misc.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/compat/freebsd32/freebsd32_misc.c Thu Nov 29 21:00:56 2018 (r341263) @@ -337,7 +337,6 @@ freebsd32_exec_copyin_args(struct image_args *args, co { char *argp, *envp; u_int32_t *p32, arg; - size_t length; int error; bzero(args, sizeof(*args)); @@ -355,20 +354,10 @@ freebsd32_exec_copyin_args(struct image_args *args, co /* * Copy the file name. */ - 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) - goto err_exit; - } else - length = 0; + error = exec_args_add_fname(args, fname, segflg); + if (error != 0) + goto err_exit; - args->begin_argv = args->buf + length; - args->endp = args->begin_argv; - args->stringspace = ARG_MAX; - /* * extract arguments first */ @@ -380,19 +369,11 @@ freebsd32_exec_copyin_args(struct image_args *args, co if (arg == 0) break; argp = PTRIN(arg); - error = copyinstr(argp, args->endp, args->stringspace, &length); - if (error) { - if (error == ENAMETOOLONG) - error = E2BIG; + error = exec_args_add_arg(args, argp, UIO_USERSPACE); + if (error != 0) goto err_exit; - } - args->stringspace -= length; - args->endp += length; - args->argc++; } - args->begin_envv = args->endp; - /* * extract environment strings */ @@ -405,16 +386,9 @@ freebsd32_exec_copyin_args(struct image_args *args, co if (arg == 0) break; envp = PTRIN(arg); - error = copyinstr(envp, args->endp, args->stringspace, - &length); - if (error) { - if (error == ENAMETOOLONG) - error = E2BIG; + error = exec_args_add_env(args, envp, UIO_USERSPACE); + if (error != 0) goto err_exit; - } - args->stringspace -= length; - args->endp += length; - args->envc++; } } Modified: head/sys/kern/imgact_binmisc.c ============================================================================== --- head/sys/kern/imgact_binmisc.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/kern/imgact_binmisc.c Thu Nov 29 21:00:56 2018 (r341263) @@ -649,21 +649,12 @@ imgact_binmisc_exec(struct image_params *imgp) s++; } - /* Check to make sure we won't overrun the stringspace. */ - if (offset > imgp->args->stringspace) { + /* Make room for the interpreter */ + 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; Modified: head/sys/kern/imgact_shell.c ============================================================================== --- head/sys/kern/imgact_shell.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/kern/imgact_shell.c Thu Nov 29 21:00:56 2018 (r341263) @@ -196,19 +196,12 @@ 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 Modified: head/sys/kern/kern_exec.c ============================================================================== --- head/sys/kern/kern_exec.c Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/kern/kern_exec.c Thu Nov 29 21:00:56 2018 (r341263) @@ -345,9 +345,9 @@ kern_execve(struct thread *td, struct image_args *args { AUDIT_ARG_ARGV(args->begin_argv, args->argc, - args->begin_envv - args->begin_argv); - AUDIT_ARG_ENVV(args->begin_envv, args->envc, - args->endp - args->begin_envv); + 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)); } @@ -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,67 +1189,43 @@ exec_copyin_args(struct image_args *args, const char * /* * Copy the file name. */ - 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) - goto err_exit; - } else - length = 0; + error = exec_args_add_fname(args, fname, segflg); + if (error != 0) + goto err_exit; - 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, - args->stringspace, &length); - if (error != 0) { - if (error == ENAMETOOLONG) - error = E2BIG; + error = exec_args_add_arg(args, (char *)(uintptr_t)arg, + UIO_USERSPACE); + if (error != 0) 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, - args->endp, args->stringspace, &length); - if (error != 0) { - if (error == ENAMETOOLONG) - error = E2BIG; + error = exec_args_add_env(args, + (char *)(uintptr_t)env, UIO_USERSPACE); + if (error != 0) goto err_exit; - } - args->stringspace -= length; - args->endp += length; - args->envc++; } } @@ -1305,8 +1280,6 @@ exec_copyin_data_fds(struct thread *td, struct image_a /* 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); @@ -1460,6 +1433,126 @@ exec_free_args(struct image_args *args) } if (args->fdp != NULL) 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); } /* Modified: head/sys/sys/imgact.h ============================================================================== --- head/sys/sys/imgact.h Thu Nov 29 20:59:18 2018 (r341262) +++ head/sys/sys/imgact.h Thu Nov 29 21:00:56 2018 (r341263) @@ -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 *);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811292100.wATL0u2O069306>