mm: check for valid prot settings (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 9 Apr 2019 00:31:44 +0000 (20:31 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 9 Apr 2019 00:38:55 +0000 (20:38 -0400)
Prior to this commit, we were not checking 'prot' in mmap() and
mprotect() for extra bits.  The user could put arbitrary bits into a
VMR's prot.  We only checked the valid bits, since commit ee6bef89ffdb
("mm: fix checks for PROT_NONE").

One potential issue is buggy programs that had been passing extra bits.
Those will fail early now.

Tested with ssh and get_html.

Reinstall your kernel headers.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ros/mman.h
kern/src/mm.c

index ee46a12..21416c9 100644 (file)
@@ -15,6 +15,7 @@
 #define PROT_WRITE             0x2
 #define PROT_EXEC              0x4
 #define PROT_NONE              0x0
+#define PROT_VALID_PROTS       (PROT_READ | PROT_WRITE | PROT_EXEC)
 
 /* mmap flags, only anonymous is supported now, feel free to pass others */
 #define MAP_SHARED             0x01
index 5dfb85e..1284ed8 100644 (file)
@@ -653,6 +653,12 @@ static bool mmap_flags_priv_ok(int flags)
               (flags & (MAP_PRIVATE | MAP_SHARED)) == MAP_SHARED;
 }
 
+static bool prot_is_valid(int prot)
+{
+       /* Remember PROT_NONE (0) is valid. */
+       return !(prot & ~PROT_VALID_PROTS);
+}
+
 static bool prot_has_access(int prot)
 {
        return prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
@@ -681,6 +687,11 @@ void *mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
                set_errno(EINVAL);
                return MAP_FAILED;
        }
+       if (!prot_is_valid(prot)) {
+               set_error(EINVAL, "invalid prot 0x%x (%x)", prot,
+                         PROT_VALID_PROTS);
+               return MAP_FAILED;
+       }
        if (!len) {
                set_errno(EINVAL);
                return MAP_FAILED;
@@ -871,6 +882,8 @@ void *do_mmap(struct proc *p, uintptr_t addr, size_t len, int prot, int flags,
        struct vm_region *vmr, *vmr_temp;
 
        assert(mmap_flags_priv_ok(flags));
+       assert(prot_is_valid(prot));
+
        vmr = vmr_zalloc();
 
        /* Sanity check, for callers that bypass mmap().  We want addr for anon
@@ -980,6 +993,11 @@ int mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
        int ret;
 
        printd("mprotect: (addr %p, len %p, prot 0x%x)\n", addr, len, prot);
+       if (!prot_is_valid(prot)) {
+               set_error(EINVAL, "invalid prot 0x%x (%x)", prot,
+                         PROT_VALID_PROTS);
+               return -1;
+       }
        if (!len)
                return 0;
        len = ROUNDUP(len, PGSIZE);
@@ -1011,6 +1029,7 @@ int __do_mprotect(struct proc *p, uintptr_t addr, size_t len, int prot)
        int pte_prot = (prot & PROT_WRITE) ? PTE_USER_RW :
                       (prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : PTE_NONE;
 
+       assert(prot_is_valid(prot));
        /* TODO: this is aggressively splitting, when we might not need to if
         * the prots are the same as the previous.  Plus, there are three
         * excessive scans. */