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 *);
_______________________________________________
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"

Reply via email to