The diff below makes the armv7 pmap use the XN (eXecure Never) bit.
This bit makes pages non-executable, making OpenBSD/armv7 catch up
with many of our architectures and provide this important security
measure.  The biggest change made by the diff is that pmap_protect()
now also deals with setting the XN bit if PROT_EXEC permission is
removed from a page.

As far as I can tell, this also gives us full kernel W^X.

ok?


Index: arch/arm/arm/pmap7.c
===================================================================
RCS file: /cvs/src/sys/arch/arm/arm/pmap7.c,v
retrieving revision 1.40
diff -u -p -r1.40 pmap7.c
--- arch/arm/arm/pmap7.c        19 Aug 2016 13:56:08 -0000      1.40
+++ arch/arm/arm/pmap7.c        19 Aug 2016 14:55:16 -0000
@@ -1553,41 +1553,27 @@ void
 pmap_protect(pmap_t pm, vaddr_t sva, vaddr_t eva, vm_prot_t prot)
 {
        struct l2_bucket *l2b;
-       pt_entry_t *ptep, opte;
+       pt_entry_t *ptep, opte, npte;
        vaddr_t next_bucket;
-       u_int flags;
        int flush;
 
        NPDEBUG(PDB_PROTECT,
            printf("pmap_protect: pm %p sva 0x%lx eva 0x%lx prot 0x%x",
            pm, sva, eva, prot));
 
-       if ((prot & PROT_READ) == 0) {
-NPDEBUG(PDB_PROTECT, printf("\n"));
-               pmap_remove(pm, sva, eva);
+       if ((prot & (PROT_WRITE | PROT_EXEC)) == (PROT_WRITE | PROT_EXEC))
                return;
-       }
 
-       if (prot & PROT_WRITE) {
-               /*
-                * If this is a read->write transition, just ignore it and let
-                * uvm_fault() take care of it later.
-                */
-NPDEBUG(PDB_PROTECT, printf("\n"));
-/* XXX WHAT IF RWX -> RW ??? */
+       if (prot == PROT_NONE) {
+               pmap_remove(pm, sva, eva);
                return;
        }
-
-       /*
-        * OK, at this point, we know we're doing write-protect operation.
-        */
-
+               
        /* XXX is that threshold of 4 the best choice for v7? */
        if (pmap_is_current(pm))
                flush = ((eva - sva) > (PAGE_SIZE * 4)) ? -1 : 0;
        else
                flush = -1;
-       flags = 0;
 
        while (sva < eva) {
                next_bucket = L2_NEXT_BUCKET(sva);
@@ -1603,27 +1589,27 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
                ptep = &l2b->l2b_kva[l2pte_index(sva)];
 
                while (sva < next_bucket) {
-                       opte = *ptep;
-                       if (opte != L2_TYPE_INV && l2pte_is_writeable(opte, 
pm)) {
+                       npte = opte = *ptep;
+                       if (opte != L2_TYPE_INV) {
                                struct vm_page *pg;
-                               u_int f;
 
-                               pg = PHYS_TO_VM_PAGE(l2pte_pa(opte));
-                               *ptep = opte | L2_V7_AP(0x4);
+                               if ((prot & PROT_WRITE) == 0)
+                                       npte |= L2_V7_AP(0x4);
+                               if ((prot & PROT_EXEC) == 0)
+                                       npte |= L2_V7_S_XN;
+                               *ptep = npte;
                                PTE_SYNC(ptep);
 
-                               if (pg != NULL) {
-                                       f = pmap_modify_pv(pg, pm, sva,
+                               pg = PHYS_TO_VM_PAGE(l2pte_pa(opte));
+                               if (pg != NULL && (prot & PROT_WRITE) == 0)
+                                       pmap_modify_pv(pg, pm, sva,
                                            PVF_WRITE, 0);
-                               } else
-                                       f = PVF_EXEC;
 
                                if (flush >= 0) {
                                        flush++;
                                        if (opte & L2_V7_AF)
                                                cpu_tlb_flushID_SE(sva);
-                               } else
-                                       flags |= f;
+                               }
                        }
 
                        sva += PAGE_SIZE;
@@ -1634,7 +1620,7 @@ NPDEBUG(PDB_PROTECT, printf("\n"));
        if (flush < 0)
                pmap_tlb_flushID(pm);
 
-NPDEBUG(PDB_PROTECT, printf("\n"));
+       NPDEBUG(PDB_PROTECT, printf("\n"));
 }
 
 void
@@ -1752,6 +1738,9 @@ pmap_fault_fixup(pmap_t pm, vaddr_t va, 
        if (pte == L2_TYPE_INV)
                goto out;
 
+       if ((ftype & PROT_EXEC) && (pte & L2_V7_S_XN))
+               goto out;
+
        /* only if vectors are low ?? */
        /*
         * Catch a userland access to the vector page mapped at 0x0
@@ -1763,15 +1752,6 @@ pmap_fault_fixup(pmap_t pm, vaddr_t va, 
 
        pa = l2pte_pa(pte);
 
-       if ((ftype & PROT_EXEC) && (pte & L2_V7_S_XN)) {
-printf("%s: va %08lx ftype %x %c pte %08x\n", __func__, va, ftype, user ? 'u' 
: 's', pte);
-printf("fault on exec\n");
-#ifdef DDB
-Debugger();
-#endif
-               /* XXX FIX THIS */
-               goto out;
-       }
        if ((ftype & PROT_WRITE) && !l2pte_is_writeable(pte, pm)) {
                /*
                 * This looks like a good candidate for "page modified"
Index: arch/arm/include/pmap.h
===================================================================
RCS file: /cvs/src/sys/arch/arm/include/pmap.h,v
retrieving revision 1.44
diff -u -p -r1.44 pmap.h
--- arch/arm/include/pmap.h     19 Aug 2016 14:05:23 -0000      1.44
+++ arch/arm/include/pmap.h     19 Aug 2016 14:55:16 -0000
@@ -703,14 +703,11 @@ L1_S_PROT(int ku, vm_prot_t pr)
                pte = (pr & PROT_WRITE) ? L1_S_PROT_UW : L1_S_PROT_UR;
        else
                pte = (pr & PROT_WRITE) ? L1_S_PROT_KW : L1_S_PROT_KR;
-       /*
-        * If we set the XN bit, the abort handlers or the vector page
-        * might be marked as such. Needs Debugging.
-        */
-       /*
+
+#ifdef CPU_ARMv7
        if ((pr & PROT_EXEC) == 0)
                pte |= L1_S_V7_XN;
-       */
+#endif
 
        return pte;
 }
@@ -723,14 +720,11 @@ L2_L_PROT(int ku, vm_prot_t pr)
                pte = (pr & PROT_WRITE) ? L2_L_PROT_UW : L2_L_PROT_UR;
        else
                pte = (pr & PROT_WRITE) ? L2_L_PROT_KW : L2_L_PROT_KR;
-       /*
-        * If we set the XN bit, the abort handlers or the vector page
-        * might be marked as such. Needs Debugging.
-        */
-       /*
+
+#ifdef CPU_ARMv7
        if ((pr & PROT_EXEC) == 0)
                pte |= L2_V7_L_XN;
-       */
+#endif
 
        return pte;
 }
@@ -743,14 +737,11 @@ L2_S_PROT(int ku, vm_prot_t pr)
                pte = (pr & PROT_WRITE) ? L2_S_PROT_UW : L2_S_PROT_UR;
        else
                pte = (pr & PROT_WRITE) ? L2_S_PROT_KW : L2_S_PROT_KR;
-       /*
-        * If we set the XN bit, the abort handlers or the vector page
-        * might be marked as such. Needs Debugging.
-        */
-       /*
+
+#ifdef CPU_ARMv7
        if ((pr & PROT_EXEC) == 0)
                pte |= L2_V7_S_XN;
-       */
+#endif
 
        return pte;
 }

Reply via email to