Fixes bug with mmapping beyond a file's last page
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 3 Feb 2012 21:02:07 +0000 (13:02 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 3 Feb 2012 21:20:56 +0000 (13:20 -0800)
The kernel was page faulting.  Now, we check for it at do_mmap() time,
and recheck during handle_page_fault().  Linux sends a SIGBUS at access
time if you tried to access beyond the filesize.

Note that we still don't have any concurrency protection regarding the
file size.  Someone could be truncating the file right as you are
loading the page, but after you checked i_size.

kern/src/mm.c
tests/test_mmap_ipc.c

index 4a2ab30..7d59b50 100644 (file)
@@ -280,6 +280,11 @@ void print_vmrs(struct proc *p)
                       vmr->vm_file, vmr->vm_foff);
 }
 
+/* Helper: returns the number of pages required to hold nr_bytes */
+unsigned long nr_pages(unsigned long nr_bytes)
+{
+       return (nr_bytes >> PGSHIFT) + (PGOFF(nr_bytes) ? 1 : 0);
+}
 
 /* Error values aren't quite comprehensive - check man mmap() once we do better
  * with the FS.
@@ -428,6 +433,13 @@ void *__do_mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
                        set_errno(EACCES);
                        return MAP_FAILED;
                }
+               /* TODO: consider locking the file while checking (not as manadatory as
+                * in handle_page_fault() */
+               if (nr_pages(offset + len) > nr_pages(file->f_dentry->d_inode->i_size)) {
+                       destroy_vmr(vmr);
+                       set_errno(ESPIPE); /* linux sends a SIGBUS at access time */
+                       return MAP_FAILED;
+               }
                kref_get(&file->f_kref, 1);
        }
        vmr->vm_file = file;
@@ -645,7 +657,17 @@ int __handle_page_fault(struct proc *p, uintptr_t va, int prot)
                 * such that we can block and resume later. */
                assert(!PGOFF(va - vmr->vm_base + vmr->vm_foff));
                f_idx = (va - vmr->vm_base + vmr->vm_foff) >> PGSHIFT;
+               /* TODO: need some sort of lock on the file to deal with someone
+                * concurrently shrinking it.  Adding 1 to f_idx, since it is
+                * zero-indexed */
+               if (f_idx + 1 > nr_pages(vmr->vm_file->f_dentry->d_inode->i_size)) {
+                       /* We're asking for pages that don't exist in the file */
+                       /* TODO: unlock the file */
+                       return -ESPIPE; /* linux sends a SIGBUS at access time */
+               }
                retval = pm_load_page(vmr->vm_file->f_mapping, f_idx, &a_page);
+               /* TODO: should be able to let go of that file shrink-lock now.  We have
+                * a page refcnt, which might be enough (depending on how it works) */
                if (retval)
                        return retval;
                /* If we want a private map, we'll preemptively give you a new page.  We
index aede6c6..e6fc3e2 100644 (file)
@@ -15,17 +15,21 @@ int main(void)
        pFile = open ("hello.txt", O_RDWR | O_CREAT, (mode_t)0600);
        /* this mmap will give you a Bus Error on linux if you try to map more
         * pages than the file contains (i think)... */
-       first = (int*)mmap(0, 8192, PROT_READ | PROT_WRITE, MAP_SHARED, pFile, 0);
+       first = (int*)mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, pFile, 0);
+       if (first == MAP_FAILED) {
+               fprintf(stderr, "Unable to mmap the file (%d), aborting!\n", errno);
+               return -1;
+       }
        first[0] = 3;
        printf("the first number after initialization is %d at %08p\n", first[0],
               first);
-       munmap(first, 8192);
+       munmap(first, 4096);
        if ((pid = fork()) < 0) {
                perror("fork error");
                exit(1);
        }
        if (pid == 0) {
-               first = (int*)mmap(0, 8192, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
+               first = (int*)mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
                                   0);
                /* delay here, to avoid the race a bit */
                udelay(1000000);
@@ -36,7 +40,7 @@ int main(void)
        }
        else
        {
-               first = (int*)mmap(0, 8192, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
+               first = (int*)mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
                                   0);
                printf("After fork in the child, the first number is %d\n", first[0]);
                first[0] = 11;