sys_exec and sys_proc_create now use argenv (XCC)
authorKevin Klues <klueska@cs.berkeley.edu>
Sun, 12 Jul 2015 02:03:38 +0000 (19:03 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 14 Jul 2015 14:33:16 +0000 (10:33 -0400)
These syscalls now take an argenv pointer and it's length instead of a
pointer to a procinfo struct with the arg and env stuff embedded in it.

When these syscalls are issued, the kernel first copies the argenv
structure to kernel memory, verifies internal consistency of all
pointers, and unpacks it into argc, envc, argv, and envp variables for
further processing. It then passes these values to a new implementation
of load_elf, who is now responsible for making sure these values end up
in the right place for the new process.  Eventually this will be on it's
stack at the locations defined by the SYSV ABI.  For now, it just copies
them into procinfo since taht is where existing code expects it to be
when popping into user space.

The main purpose of this commit was to make sure that this new method of
passing arguments and environment is going to work, moving forward. The
next step is to modify where these values are placed (i.e. on the new
processes stack), and then update the low levels of user-space to
account for this change.

kern/include/elf.h
kern/include/ros/limits.h
kern/include/ros/procinfo.h
kern/src/elf.c
kern/src/process.c
kern/src/syscall.c
tests/old/proctests.c
tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/execve.c
user/parlib/syscall.c

index d331d91..75398c0 100644 (file)
@@ -149,6 +149,7 @@ typedef long elf_aux_t[2];
 
 struct file;
 bool is_valid_elf(struct file *f);
-int load_elf(struct proc* p, struct file* f);
+int load_elf(struct proc* p, struct file* f,
+             int argc, char *argv[], int envc, char *envp[]);
 
 #endif /* !ROS_INC_ELF_H */
index 20feec0..28f31af 100644 (file)
@@ -8,4 +8,7 @@
 
 #define PATH_MAX 4096 /* includes null-termination */
 
+/* # bytes of args + environ for exec()  (i.e. max size of argenv) */
+#define ARG_MAX (32 * 4096) /* Good chunk of our 256 page stack */
+
 #endif /* ROS_INC_LIMITS_H */
index e54b184..ea9cf69 100644 (file)
@@ -67,42 +67,6 @@ typedef struct procinfo {
 } procinfo_t;
 #define PROCINFO_NUM_PAGES  ((sizeof(procinfo_t)-1)/PGSIZE + 1)        
 
-static int
-procinfo_pack_args(procinfo_t* p, char* const* argv, char* const* envp)
-{
-       int nargv = 0, nenvp = 0;
-       if(argv) while(argv[nargv]) nargv++;
-       if(envp) while(envp[nenvp]) nenvp++;
-
-       if(nargv+nenvp+2 > PROCINFO_MAX_ARGP)
-               return -1;
-
-       int pos = 0;
-       int i;
-       for(i = 0; i < nargv; i++)
-       {
-               int len = strlen(argv[i])+1;
-               if(pos+len > PROCINFO_ARGBUF_SIZE)
-                       return -1;
-               p->argp[i] = ((procinfo_t*)UINFO)->argbuf+pos;
-               memcpy(p->argbuf+pos,argv[i],len);
-               pos += len;
-       }
-       p->argp[nargv] = 0;
-
-       for(i = 0; i < nenvp; i++)
-       {
-               int len = strlen(envp[i])+1;
-               if(pos+len > PROCINFO_ARGBUF_SIZE)
-                       return -1;
-               p->argp[nargv+1+i] = ((procinfo_t*)UINFO)->argbuf+pos;
-               memcpy(p->argbuf+pos,envp[i],len);
-               pos += len;
-       }
-       p->argp[nargv+nenvp+1] = 0;
-       
-       return 0;
-}
 
 // this is how user programs access the procinfo page
 #ifndef ROS_KERNEL
index 245b0b1..d899f9a 100644 (file)
@@ -258,7 +258,8 @@ fail:
        return ret;
 }
 
-int load_elf(struct proc* p, struct file* f)
+int load_elf(struct proc* p, struct file* f,
+             int argc, char *argv[], int envc, char *envp[])
 {
        elf_info_t ei, interp_ei;
        if (load_one_elf(p, f, 0, &ei, FALSE))
@@ -280,6 +281,32 @@ int load_elf(struct proc* p, struct file* f)
                        return -1;
        }
 
+       /* Copy the contents of the argenv array into procinfo. This is only
+        * temporary so that we can verify everything works with the new structure
+        * up to this point.  Soon we will map this stuff on the stack properly, as
+        * per the SYSV ABI. */
+       if (argc + 1 + envc + 1 > PROCINFO_MAX_ARGP)
+               return -1;
+       int pos = 0;
+       for(int i = 0; i < argc; i++) {
+               int len = strlen(argv[i]) + 1;
+               if(pos + len > PROCINFO_ARGBUF_SIZE)
+                       return -1;
+               p->procinfo->argp[i] = ((procinfo_t*)UINFO)->argbuf + pos;
+               memcpy(p->procinfo->argbuf + pos, argv[i], len);
+               pos += len;
+       }
+       p->procinfo->argp[argc] = NULL;
+       for(int i = 0; i < envc; i++) {
+               int len = strlen(envp[i]) + 1;
+               if(pos + len > PROCINFO_ARGBUF_SIZE)
+                       return -1;
+               p->procinfo->argp[argc + 1 + i] = ((procinfo_t*)UINFO)->argbuf + pos;
+               memcpy(p->procinfo->argbuf + pos, envp[i], len);
+               pos += len;
+       }
+       p->procinfo->argp[argc + 1 + envc] = NULL;
+
        // fill in auxiliary info for dynamic linker/runtime
        elf_aux_t auxp[] = {{ELF_AUX_PHDR, ei.phdr},
                            {ELF_AUX_PHENT, sizeof(proghdr32_t)},
index cd1409c..d73e913 100644 (file)
@@ -434,9 +434,11 @@ struct proc *proc_create(struct file *prog, char **argv, char **envp)
        error_t r;
        if ((r = proc_alloc(&p, current, 0 /* flags */)) < 0)
                panic("proc_create: %e", r);    /* one of 3 quaint usages of %e */
-       proc_set_progname(p, argv[0]);
-       procinfo_pack_args(p->procinfo, argv, envp);
-       assert(load_elf(p, prog) == 0);
+       int argc = 0, envc = 0;
+       if(argv) while(argv[argc]) argc++;
+       if(envp) while(envp[envc]) envc++;
+       proc_set_progname(p, argc ? argv[0] : NULL);
+       assert(load_elf(p, prog, argc, argv, envc, envp) == 0);
        __proc_ready(p);
        return p;
 }
index 0c26730..fc96fde 100644 (file)
@@ -2,6 +2,7 @@
 
 //#define DEBUG
 #include <ros/common.h>
+#include <ros/limits.h>
 #include <arch/types.h>
 #include <arch/arch.h>
 #include <arch/mmu.h>
@@ -330,6 +331,44 @@ static char *copy_in_path(struct proc *p, const char *path, size_t path_l)
        return t_path;
 }
 
+static int unpack_argenv(struct argenv *argenv, size_t argenv_l,
+                         int *argc_p, char ***argv_p,
+                         int *envc_p, char ***envp_p)
+{
+       int argc = argenv->argc;
+       int envc = argenv->envc;
+       char **argv = (char**)argenv->buf;
+       char **envp = argv + argc;
+       char *argbuf = (char*)(envp + envc);
+       uintptr_t argbuf_offset = (uintptr_t)(argbuf - (char*)(argenv));
+
+       if (((char*)argv - (char*)argenv) > argenv_l)
+               return -1;
+       if (((char*)argv + (argc * sizeof(char**)) - (char*)argenv) > argenv_l)
+               return -1;
+       if (((char*)envp - (char*)argenv) > argenv_l)
+               return -1;
+       if (((char*)envp + (envc * sizeof(char**)) - (char*)argenv) > argenv_l)
+               return -1;
+       if (((char*)argbuf - (char*)argenv) > argenv_l)
+               return -1;
+       for (int i = 0; i < argc; i++) {
+               if ((uintptr_t)(argv[i] + argbuf_offset) > argenv_l)
+                       return -1;
+               argv[i] += (uintptr_t)argbuf;
+       }
+       for (int i = 0; i < envc; i++) {
+               if ((uintptr_t)(envp[i] + argbuf_offset) > argenv_l)
+                       return -1;
+               envp[i] += (uintptr_t)argbuf;
+       }
+       *argc_p = argc;
+       *argv_p = argv;
+       *envc_p = envc;
+       *envp_p = envp;
+       return 0;
+}
+
 /* Helper, frees a path that was allocated with copy_in_path. */
 static void free_path(struct proc *p, char *t_path)
 {
@@ -489,14 +528,17 @@ static pid_t sys_getpid(struct proc *p)
 
 /* Creates a process from the file 'path'.  The process is not runnable by
  * default, so it needs it's status to be changed so that the next call to
- * schedule() will try to run it.  TODO: take args/envs from userspace. */
+ * schedule() will try to run it. */
 static int sys_proc_create(struct proc *p, char *path, size_t path_l,
-                           struct procinfo *pi, int flags)
+                           char *argenv, size_t argenv_l, int flags)
 {
        int pid = 0;
        char *t_path;
        struct file *program;
        struct proc *new_p;
+       int argc, envc;
+       char **argv, **envp;
+       struct argenv *kargenv;
 
        t_path = copy_in_path(p, path, path_l);
        if (!t_path)
@@ -506,6 +548,27 @@ static int sys_proc_create(struct proc *p, char *path, size_t path_l,
        free_path(p, t_path);
        if (!program)
                return -1;                      /* presumably, errno is already set */
+
+       /* Check the size of the argenv array, error out if too large. */
+       if ((argenv_l < sizeof(struct argenv)) || (argenv_l > ARG_MAX)) {
+               set_errno(EINVAL);
+               set_errstr("The argenv array has an invalid size: %lu\n", argenv_l);
+               return -1;
+       }
+       /* Copy the argenv array into a kernel buffer. Delay processing of the
+        * array to load_elf(). */
+       kargenv = user_memdup_errno(p, argenv, argenv_l);
+       if (!kargenv) {
+               set_errstr("Failed to copy in the args");
+               return -1;
+       }
+       /* Unpack the argenv array into more usable variables. Integrity checking
+        * done along side this as well. */
+       if (unpack_argenv(kargenv, argenv_l, &argc, &argv, &envc, &envp)) {
+               set_errstr("Failed to unpack the args");
+               goto early_error;
+       }
+
        /* TODO: need to split the proc creation, since you must load after setting
         * args/env, since auxp gets set up there. */
        //new_p = proc_create(program, 0, 0);
@@ -516,24 +579,15 @@ static int sys_proc_create(struct proc *p, char *path, size_t path_l,
        /* close the CLOEXEC ones, even though this isn't really an exec */
        close_9ns_files(new_p, TRUE);
        close_all_files(&new_p->open_files, TRUE);
-       /* Set the argument stuff needed by glibc */
-       if (memcpy_from_user_errno(p, new_p->procinfo->argp, pi->argp,
-                                  sizeof(pi->argp))) {
-               set_errstr("Failed to memcpy argp");
-               goto late_error;
-       }
-       if (memcpy_from_user_errno(p, new_p->procinfo->argbuf, pi->argbuf,
-                                  sizeof(pi->argbuf))) {
-               set_errstr("Failed to memcpy argbuf");
-               goto late_error;
-       }
-       if (load_elf(new_p, program)) {
+       /* Load the elf. */
+       if (load_elf(new_p, program, argc, argv, envc, envp)) {
                set_errstr("Failed to load elf");
                goto late_error;
        }
        /* progname is argv0, which accounts for symlinks */
-       proc_set_progname(p, p->procinfo->argbuf);
+       proc_set_progname(p, argc ? argv[0] : NULL);
        kref_put(&program->f_kref);
+       user_memdup_free(p, kargenv);
        __proc_ready(new_p);
        pid = new_p->pid;
        proc_decref(new_p);     /* give up the reference created in proc_create() */
@@ -547,6 +601,8 @@ late_error:
        proc_destroy(new_p);
 mid_error:
        kref_put(&program->f_kref);
+early_error:
+       user_memdup_free(p, kargenv);
        return -1;
 }
 
@@ -703,13 +759,16 @@ static ssize_t sys_fork(env_t* e)
  * Note: if someone batched syscalls with this call, they could clobber their
  * old memory (and will likely PF and die).  Don't do it... */
 static int sys_exec(struct proc *p, char *path, size_t path_l,
-                    struct procinfo *pi)
+                    char *argenv, size_t argenv_l)
 {
        int ret = -1;
        char *t_path;
        struct file *program;
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
        int8_t state = 0;
+       int argc, envc;
+       char **argv, **envp;
+       struct argenv *kargenv;
 
        /* We probably want it to never be allowed to exec if it ever was _M */
        if (p->state != PROC_RUNNING_S) {
@@ -723,6 +782,7 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
        t_path = copy_in_path(p, path, path_l);
        if (!t_path)
                return -1;
+
        disable_irqsave(&state);        /* protect cur_ctx */
        /* Can't exec if we don't have a current_ctx to restart (if we fail).  This
         * isn't 100% true, but I'm okay with it. */
@@ -743,6 +803,28 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
         * userspace and then asynchronously finish the exec later. */
        clear_owning_proc(core_id());
        enable_irqsave(&state);
+
+       /* Check the size of the argenv array, error out if too large. */
+       if ((argenv_l < sizeof(struct argenv)) || (argenv_l > ARG_MAX)) {
+               set_errno(EINVAL);
+               set_errstr("The argenv array has an invalid size: %lu\n", argenv_l);
+               return -1;
+       }
+       /* Copy the argenv array into a kernel buffer. */
+       kargenv = user_memdup_errno(p, argenv, argenv_l);
+       if (!kargenv) {
+               set_errstr("Failed to copy in the args and environment");
+               return -1;
+       }
+       /* Unpack the argenv array into more usable variables. Integrity checking
+        * done along side this as well. */
+       if (unpack_argenv(kargenv, argenv_l, &argc, &argv, &envc, &envp)) {
+               user_memdup_free(p, kargenv);
+               set_errno(EINVAL);
+               set_errstr("Failed to unpack the args");
+               return -1;
+       }
+
        /* This could block: */
        /* TODO: 9ns support */
        program = do_file_open(t_path, 0, 0);
@@ -751,18 +833,11 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
                goto early_error;
        if (!is_valid_elf(program)) {
                set_errno(ENOEXEC);
-               goto early_error;
-       }
-       /* Set the argument stuff needed by glibc */
-       if (memcpy_from_user_errno(p, p->procinfo->argp, pi->argp,
-                                  sizeof(pi->argp)))
-               goto mid_error;
-       if (memcpy_from_user_errno(p, p->procinfo->argbuf, pi->argbuf,
-                                  sizeof(pi->argbuf)))
                goto mid_error;
+       }
        /* This is the point of no return for the process. */
        /* progname is argv0, which accounts for symlinks */
-       proc_set_progname(p, p->procinfo->argbuf);
+       proc_set_progname(p, argc ? argv[0] : NULL);
        #ifdef CONFIG_X86
        /* clear this, so the new program knows to get an LDT */
        p->procdata->ldt = 0;
@@ -774,8 +849,9 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
        close_9ns_files(p, TRUE);
        close_all_files(&p->open_files, TRUE);
        env_user_mem_free(p, 0, UMAPTOP);
-       if (load_elf(p, program)) {
+       if (load_elf(p, program, argc, argv, envc, envp)) {
                kref_put(&program->f_kref);
+               user_memdup_free(p, kargenv);
                /* Note this is an inedible reference, but proc_destroy now returns */
                proc_destroy(p);
                /* We don't want to do anything else - we just need to not accidentally
@@ -797,6 +873,7 @@ early_error:
        finish_current_sysc(-1);
        systrace_finish_trace(pcpui->cur_kthread, -1);
 success:
+       user_memdup_free(p, kargenv);
        free_sysc_str(pcpui->cur_kthread);
        /* Here's how we restart the new (on success) or old (on failure) proc: */
        spin_lock(&p->proc_lock);
index 0682ed3..9af0274 100644 (file)
@@ -15,7 +15,7 @@ int main(int argc, char** argv)
        int child_pid[NUM_KIDS];
        if (pid == 0x1000) {
                for (int i = 0; i < NUM_KIDS; i++)
-                       child_pid[i] = sys_proc_create(FILENAME, strlen(FILENAME), 0, 0);
+                       child_pid[i] = sys_proc_create(FILENAME, strlen(FILENAME), NULL, NULL, 0);
                for (int i = 0; i < NUM_KIDS; i++) {
                        printf("U: attempting to spawn yielders (pid: %d)\n", child_pid[i]);
                        sys_proc_run(child_pid[i]);
index e7f2949..0b386cc 100644 (file)
@@ -23,8 +23,8 @@
 #include <string.h>
 #include <fcntl.h>
 #include <elf/elf.h>
-#include <ros/procinfo.h>
 #include <assert.h>
+#include <parlib/serialize.h>
 
 /* Replace the current process, executing PATH with arguments ARGV and
    environment ENVP.  ARGV and ENVP are terminated by NULL pointers.  */
@@ -34,18 +34,16 @@ __execve (path, argv, envp)
      char *const argv[];
      char *const envp[];
 {
-  procinfo_t pi;
-  if(procinfo_pack_args(&pi,argv,envp))
-  {
+  struct serialized_data *sd = serialize_argv_envp(argv, envp);
+  if (!sd) {
     errno = ENOMEM;
     return -1;
   }
 
-  int ret = ros_syscall(SYS_exec, path, strlen(path), (uintptr_t)&pi, 0, 0, 0);
-
+  int ret = ros_syscall(SYS_exec, path, strlen(path), sd->buf, sd->len, 0, 0);
   // if we got here, then exec better have failed...
   assert(ret == -1);
-
+  free_serialized_data(sd);
   return ret;
 }
 weak_alias (__execve, execve)
index 0eac457..c645d9c 100644 (file)
@@ -2,6 +2,7 @@
 
 #include <parlib/parlib.h>
 #include <parlib/vcore.h>
+#include <parlib/serialize.h>
 
 int sys_proc_destroy(int pid, int exitcode)
 {
@@ -59,12 +60,15 @@ void sys_yield(bool being_nice)
 int sys_proc_create(char *path, size_t path_l, char *argv[], char *envp[],
                     int flags)
 {
-       struct procinfo pi;
-       if (procinfo_pack_args(&pi, argv, envp)) {
+       struct serialized_data *sd = serialize_argv_envp(argv, envp);
+       if (!sd) {
                errno = ENOMEM;
                return -1;
        }
-       return ros_syscall(SYS_proc_create, path, path_l, &pi, flags, 0, 0);
+       int ret = ros_syscall(SYS_proc_create, path, path_l,
+                             sd->buf, sd->len, flags, 0);
+       free_serialized_data(sd);
+       return ret;
 }
 
 int sys_proc_run(int pid)