Map PTEs for MAP_SHARED | MAP_LOCKED files on fork
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 23 Mar 2016 19:23:05 +0000 (15:23 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 31 Mar 2016 20:53:42 +0000 (16:53 -0400)
If you had a process that forked but did not exec, then the read-only parts
of the binary would not be in its address space.  Those parts would be in
the page cache, so long as the parent was still around (which had the VMR
MAP_LOCKED) (or if it left and we flushed the cache).

Then later, the child asks the kernel to perform a syscall on one of its
addresses in the read-only section, e.g. .rodata.  The kernel would then
page fault.

Right now, the kernel won't attempt to handle a PF *of its own* by talking
to the page cache.  Eventually we'll need to do this.  But it's also just
wrong for us to not have MAP_LOCKED VMRs present in a process's page table.

I <3 fork.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/trap.c
kern/src/mm.c

index a5a4abf..72d0c64 100644 (file)
@@ -282,7 +282,8 @@ static bool __handler_kernel_page_fault(struct hw_trapframe *hw_tf,
         * In an effort to reduce the number of locks (both now and in the future),
         * the kernel will not attempt to handle faults on file-back VMRs.  We
         * probably can turn that on in the future, but I'd rather keep things safe
-        * for now.
+        * for now.  (We'll probably need to change this when we stop
+        * MAP_POPULATE | MAP_LOCKED entire binaries).
         *
         * Note that we do not enable IRQs here, unlike in the user case.  Again,
         * this is to limit the locks we could be grabbing. */
index 6d20795..8abeef8 100644 (file)
 struct kmem_cache *vmr_kcache;
 
 static int __vmr_free_pgs(struct proc *p, pte_t pte, void *va, void *arg);
+static int populate_pm_va(struct proc *p, uintptr_t va, unsigned long nr_pgs,
+                          int pte_prot, struct page_map *pm, size_t offset,
+                          int flags, bool exec);
+
 /* minor helper, will ease the file->chan transition */
 static struct page_map *file2pm(struct file *file)
 {
@@ -313,8 +317,38 @@ static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
                                 new_p);
 }
 
+static int fill_vmr(struct proc *p, struct proc *new_p, struct vm_region *vmr)
+{
+       int ret = 0;
+
+       if ((!vmr->vm_file) || (vmr->vm_flags & MAP_PRIVATE)) {
+               assert(!(vmr->vm_flags & MAP_SHARED));
+               ret = copy_pages(p, new_p, vmr->vm_base, vmr->vm_end);
+       } else {
+               /* non-private file, i.e. page cacheable.  we have to honor MAP_LOCKED,
+                * (but we might be able to ignore MAP_POPULATE). */
+               if (vmr->vm_flags & MAP_LOCKED) {
+                       /* need to keep the file alive in case we unlock/block */
+                       kref_get(&vmr->vm_file->f_kref, 1);
+                       /* math is a bit nasty if vm_base isn't page aligned */
+                       assert(!PGOFF(vmr->vm_base));
+                       ret = populate_pm_va(new_p, vmr->vm_base,
+                                            (vmr->vm_end - vmr->vm_base) >> PGSHIFT,
+                                            vmr->vm_prot, vmr->vm_file->f_mapping,
+                                            vmr->vm_foff, vmr->vm_flags,
+                                            vmr->vm_prot & PROT_EXEC);
+                       kref_put(&vmr->vm_file->f_kref);
+               }
+       }
+       return ret;
+}
+
 /* This will make new_p have the same VMRs as p, and it will make sure all
  * physical pages are copied over, with the exception of MAP_SHARED files.
+ * MAP_SHARED files that are also MAP_LOCKED will be attached to the process -
+ * presumably they are in the page cache since the parent locked them.  This is
+ * all pretty nasty.
+ *
  * This is used by fork().
  *
  * Note that if you are working on a VMR that is a file, you'll want to be
@@ -338,17 +372,14 @@ int duplicate_vmrs(struct proc *p, struct proc *new_p)
                        kref_get(&vm_i->vm_file->f_kref, 1);
                        pm_add_vmr(file2pm(vm_i->vm_file), vmr);
                }
-               if (!vmr->vm_file || vmr->vm_flags & MAP_PRIVATE) {
-                       assert(!(vmr->vm_flags & MAP_SHARED));
-                       /* Copy over the memory from one VMR to the other */
-                       if ((ret = copy_pages(p, new_p, vmr->vm_base, vmr->vm_end))) {
-                               if (vm_i->vm_file) {
-                                       pm_remove_vmr(file2pm(vm_i->vm_file), vmr);
-                                       kref_put(&vm_i->vm_file->f_kref);
-                               }
-                               kmem_cache_free(vmr_kcache, vm_i);
-                               return ret;
+               ret = fill_vmr(p, new_p, vmr);
+               if (ret) {
+                       if (vm_i->vm_file) {
+                               pm_remove_vmr(file2pm(vm_i->vm_file), vmr);
+                               kref_put(&vm_i->vm_file->f_kref);
                        }
+                       kmem_cache_free(vmr_kcache, vm_i);
+                       return ret;
                }
                TAILQ_INSERT_TAIL(&new_p->vm_regions, vmr, vm_link);
        }