Fork and exec handle files better
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 6 Aug 2010 06:18:43 +0000 (23:18 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:50 +0000 (17:35 -0700)
A fork-created process has the same files as its parent, and exec'd
process will close all of its O_CLOEXEC files.  Not tested much.

kern/include/vfs.h
kern/src/manager.c
kern/src/process.c
kern/src/syscall.c
kern/src/vfs.c

index a4e97fb..07270d7 100644 (file)
@@ -113,22 +113,27 @@ struct nameidata {
 #define LOOKUP_ACCESS          0x12    /* access / check user permissions */
 
 /* TODO: make our own versions (fucking octal) and put it in fcntl.h */
-/* File access modes for `open' and `fcntl'.  */
-#define O_RDONLY    0   /* Open read-only.  */
-#define O_WRONLY    1   /* Open write-only.  */
-#define O_RDWR      2   /* Open read/write.  */
-#define O_ACCMODE   3
-
-/* Bits OR'd into the second argument to open.  */
-#define O_CREAT        0100 /* not fcntl */
-#define O_EXCL         0200 /* not fcntl */
-#define O_NOCTTY       0400 /* not fcntl */
-#define O_TRUNC       01000 /* not fcntl */
-#define O_APPEND      02000
-#define O_NONBLOCK    04000
-#define O_SYNC       010000
-#define O_FSYNC      O_SYNC
-#define O_ASYNC      020000
+/* File access modes for open and fcntl. */
+#define O_RDONLY               0                       /* Open read-only. */
+#define O_WRONLY               1                       /* Open write-only. */
+#define O_RDWR                 2                       /* Open read/write. */
+#define O_ACCMODE              3
+
+/* Bits OR'd into the second argument to open. */
+#define O_CREAT                        00000100        /* not fcntl */
+#define O_EXCL                 00000200        /* not fcntl */
+#define O_NOCTTY               00000400        /* not fcntl */
+#define O_TRUNC                        00001000        /* not fcntl */
+#define O_APPEND               00002000
+#define O_NONBLOCK             00004000
+#define O_SYNC                 00010000
+#define O_FSYNC                        O_SYNC
+#define O_ASYNC                        00020000
+#define O_DIRECT               00040000        /* Direct disk access. */
+#define O_DIRECTORY            00200000        /* Must be a directory. */
+#define O_NOFOLLOW             00400000        /* Do not follow links. */
+#define O_NOATIME              01000000        /* Do not set atime. */
+#define O_CLOEXEC              02000000        /* Set close_on_exec. */
 
 /* Every object that has pages, like an inode or the swap (or even direct block
  * devices) has a page_map, tracking which of its pages are currently in memory.
@@ -486,6 +491,7 @@ int file_load_page(struct file *file, unsigned long index, struct page **pp);
 struct file *get_file_from_fd(struct files_struct *open_files, int fd);
 struct file *put_file_from_fd(struct files_struct *open_files, int file_desc);
 int insert_file(struct files_struct *open_files, struct file *file);
-void close_all_files(struct files_struct *open_files);
+void close_all_files(struct files_struct *open_files, bool cloexec);
+void clone_files(struct files_struct *src, struct files_struct *dst);
 
 #endif /* ROS_KERN_VFS_H */
index cdf4e78..e7c675c 100644 (file)
@@ -109,7 +109,7 @@ char *p_envp[] = {"LD_LIBRARY_PATH=/lib", 0};
 
 void manager_brho(void)
 {
-       static uint8_t RACY progress = 0;
+       static uint8_t RACY progress = 0;       /* this will wrap around. */
        static struct proc *p;
        struct file *temp_f;
 
@@ -119,6 +119,7 @@ void manager_brho(void)
 
        switch (progress++) {
                case 0:
+                       printk("Top of the manager to ya!\n");
                        /* 124 is half of the available boxboro colors (with the kernel
                         * getting 8) */
                        //quick_proc_color_run("msr_dumb_while", p, 124, temp_f);
index c146799..3ef3272 100644 (file)
@@ -325,7 +325,9 @@ error_t proc_alloc(struct proc **pp, struct proc *parent)
        p->open_files.max_fdset = NR_FILE_DESC_DEFAULT;
        p->open_files.fd = p->open_files.fd_array;
        p->open_files.open_fds = (struct fd_set*)&p->open_files.open_fds_init;
-       /* 0, 1, and 2 are reserved, but prob shouldn't do it this way */
+       /* TODO: 0, 1, and 2 are reserved, but prob shouldn't do it this way.
+        * Whatever we do for stdin/out/err, we need to keep it in sync for created
+        * processes and forked processes (clone_files). */
        p->open_files.next_fd = 3;
        for (int i = 0; i < 3; i++)
                SET_BITMASK_BIT(p->open_files.open_fds->fds_bits, i);
@@ -365,7 +367,7 @@ static void __proc_free(struct kref *kref)
        // All parts of the kernel should have decref'd before __proc_free is called
        assert(kref_refcnt(&p->kref) == 0);
 
-       close_all_files(&p->open_files);
+       close_all_files(&p->open_files, FALSE);
        destroy_vmrs(p);
        frontend_proc_free(p);  /* TODO: please remove me one day */
        /* Free any colors allocated to this process */
@@ -651,6 +653,8 @@ void proc_destroy(struct proc *p)
         * but currently aren't for all things (like traphandlers). */
        spin_unlock(&p->proc_lock);
        kref_put(&p->kref);             /* for the hashtable ref */
+       /* at this point, we normally have one ref to be eaten in kmsg_pending and
+        * one for every 'current'.  and maybe one for a parent */
        __proc_kmsg_pending(p, self_ipi_pending);
        return;
 }
index 19cb653..37b1be5 100644 (file)
@@ -369,9 +369,7 @@ static ssize_t sys_fork(env_t* e)
                set_errno(current_tf,ENOMEM);
                return -1;
        }
-       
-       /* TODO: copy all open files, except O_CLOEXEC */
-
+       clone_files(&e->open_files, &env->open_files);
        __proc_set_state(env, PROC_RUNNABLE_S);
        schedule_proc(env);
 
@@ -419,6 +417,8 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
         * okay.  Potentially issues with the nm and vcpd if we were in _M before
         * and someone is trying to notify. */
        memset(p->procdata, 0, sizeof(procdata_t));
+       destroy_vmrs(p);
+       close_all_files(&p->open_files, TRUE);
        env_user_mem_free(p, 0, UMAPTOP);
        if (load_elf(p, program)) {
                kref_put(&program->f_kref);
index af93a5c..eb095dd 100644 (file)
@@ -1020,8 +1020,9 @@ int insert_file(struct files_struct *open_files, struct file *file)
        return slot;
 }
 
-/* Closes all open files.  Mostly just a "put" for all files. */
-void close_all_files(struct files_struct *open_files)
+/* Closes all open files.  Mostly just a "put" for all files.  If cloexec, it
+ * will only close files that are opened with O_CLOEXEC. */
+void close_all_files(struct files_struct *open_files, bool cloexec)
 {
        struct file *file;
        spin_lock(&open_files->lock);
@@ -1031,6 +1032,8 @@ void close_all_files(struct files_struct *open_files)
                         * have a valid fdset higher than files */
                        assert(i < open_files->max_files);
                        file = open_files->fd[i];
+                       if (cloexec && !(file->f_flags | O_CLOEXEC))
+                               continue;
                        open_files->fd[i] = 0;
                        /* the if case is due to files (stdin) without a *file yet */
                        if (file)
@@ -1040,3 +1043,27 @@ void close_all_files(struct files_struct *open_files)
        }
        spin_unlock(&open_files->lock);
 }
+
+/* Inserts all of the files from src into dst, used by sys_fork(). */
+void clone_files(struct files_struct *src, struct files_struct *dst)
+{
+       struct file *file;
+       spin_lock(&src->lock);
+       spin_lock(&dst->lock);
+       for (int i = 0; i < src->max_fdset; i++) {
+               if (GET_BITMASK_BIT(src->open_fds->fds_bits, i)) {
+                       /* while max_files and max_fdset might not line up, we should never
+                        * have a valid fdset higher than files */
+                       assert(i < src->max_files);
+                       file = src->fd[i];
+                       SET_BITMASK_BIT(dst->open_fds->fds_bits, i);
+                       assert(i < dst->max_files && dst->fd[i] == 0);
+                       dst->fd[i] = file;
+                       /* the if case is due to files (stdin) without a *file yet */
+                       if (file)
+                               kref_get(&file->f_kref, 1);
+               }
+       }
+       spin_unlock(&dst->lock);
+       spin_unlock(&src->lock);
+}