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; }