Fixed user_mem_check()
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 3 Sep 2009 15:50:25 +0000 (08:50 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 3 Sep 2009 15:50:25 +0000 (08:50 -0700)
It was only considering the permissions on the last level of the
pagetable walk, instead of the cumulative permissions.  The sparc
version still uses the old style (the result of pgdir_walk).

kern/arch/i386/pmap.c
kern/arch/sparc/pmap.c
kern/include/pmap.h
kern/src/pmap.c

index 1293ffd..057723f 100644 (file)
@@ -628,6 +628,23 @@ pgdir_walk(pde_t *pgdir, const void *SNT va, int create)
        return &((pde_t*COUNT(NPTENTRIES))KADDR(PTE_ADDR(*the_pde)))[PTX(va)];
 }
 
+/* Returns the effective permissions for PTE_U, PTE_W, and PTE_P on a given
+ * virtual address.  Note we need to consider the composition of every PTE in
+ * the page table walk. */
+int get_va_perms(pde_t *pgdir, const void *SNT va)
+{
+       pde_t the_pde = pgdir[PDX(va)];
+       pte_t the_pte = 0;
+
+       if (!(the_pde & PTE_P))
+               return 0;
+       if (the_pde & PTE_PS)
+               return the_pde & (PTE_U | PTE_W | PTE_P);
+       // else
+       the_pte = ((pde_t*COUNT(NPTENTRIES))KADDR(PTE_ADDR(the_pde)))[PTX(va)];
+       return the_pte & the_pde & (PTE_U | PTE_W | PTE_P);
+}
+
 /* Flushes a TLB, including global pages.  We should always have the CR4_PGE
  * flag set, but just in case, we'll check.  Toggling this bit flushes the TLB.
  */
index cbaf19d..8a76031 100644 (file)
@@ -114,6 +114,14 @@ pgdir_walk(pde_t* l1pt, const void*SNT va, int create)
        l3pte = &l3pt[L3X(va)];
        return l3pte;
 }
+
+/* TODO: this is probably wrong, since it only returns the pte as if it were the
+ * perms. */
+int get_va_perms(pde_t *pgdir, const void *SNT va)
+{
+       return (int)pgdir_walk(pgdir, va, 0);
+}
+
 void
 page_check(void)
 {
index e59d218..b5fae68 100644 (file)
@@ -82,7 +82,8 @@ error_t
 memcpy_from_user(env_t* env, void* COUNT(len) dest,
                  const void *DANGEROUS va, size_t len);
                  
-pte_t *pgdir_walk(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va, int create);                 
+pte_t *pgdir_walk(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va, int create);
+int get_va_perms(pde_t *pgdir, const void *SNT va);
 
 static inline page_t *SAFE ppn2page(size_t ppn)
 {
index 4b38b4f..ec712b5 100644 (file)
@@ -238,7 +238,9 @@ void tlb_invalidate(pde_t *pgdir, void *va)
  * @brief Check that an environment is allowed to access the range of memory
  * [va, va+len) with permissions 'perm | PTE_P'.
  *
- * Normally 'perm' will contain PTE_U at least, but this is not required.
+ * Normally 'perm' will contain PTE_U at least, but this is not required.  The
+ * function get_va_perms only checks for PTE_U, PTE_W, and PTE_P.  It won't
+ * check for things like PTE_PS, PTE_A, etc.
  * 'va' and 'len' need not be page-aligned;
  *
  * A user program can access a virtual address if:
@@ -266,7 +268,7 @@ void* user_mem_check(env_t *env, const void *DANGEROUS va, size_t len, int perm)
        // kernel is uninterruptible
        void *DANGEROUS start, *DANGEROUS end;
        size_t num_pages, i;
-       pte_t *pte;
+       int page_perms = 0;
 
        perm |= PTE_P;
        start = ROUNDDOWN((void*DANGEROUS)va, PGSIZE);
@@ -277,9 +279,9 @@ void* user_mem_check(env_t *env, const void *DANGEROUS va, size_t len, int perm)
        }
        num_pages = PPN(end - start);
        for (i = 0; i < num_pages; i++, start += PGSIZE) {
-               pte = pgdir_walk(env->env_pgdir, start, 0);
+               page_perms = get_va_perms(env->env_pgdir, start);
                // ensures the bits we want on are turned on.  if not, error out
-               if ( !pte || ((*pte & perm) != perm) ) {
+               if ((page_perms & perm) != perm) {
                        if (i == 0)
                                user_mem_check_addr = (void*DANGEROUS)va;
                        else