Put PG_EXEC and PG_FOE into the PG_PROT mask, and make sure the default
authormiod <miod@openbsd.org>
Mon, 6 Jan 2014 20:27:44 +0000 (20:27 +0000)
committermiod <miod@openbsd.org>
Mon, 6 Jan 2014 20:27:44 +0000 (20:27 +0000)
pte protection masks, as initialized in alpha_protection_init(), set PG_FOE
by default when VM_PROT_EXECUTE is not set.

Also, change pmap_emulate_reference() to only clear PG_FOE if the affected
pte has executable permission.

This allows various pmap_pte_exec() checks (added to explicitely set PG_FOE)
to be removed.

All tests of regress/sys/kern/noexec now reliably pass on EV5. EV6 systems
still see spurious (but no longer 100% reproduceable) failures of the `catch
a signal' tests, which is likely caused by the effect of mprotect() removing
execute permission not taking effect correctly, despite PAL IMB being issued
(and no, this is not caused by the previous pmap_changebit() change), to be
investigated.

sys/arch/alpha/alpha/pmap.c
sys/arch/alpha/alpha/trap.c
sys/arch/alpha/include/pte.h

index bdd3b40..cc7c6f8 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: pmap.c,v 1.68 2014/01/06 20:21:32 miod Exp $ */
+/* $OpenBSD: pmap.c,v 1.69 2014/01/06 20:27:44 miod Exp $ */
 /* $NetBSD: pmap.c,v 1.154 2000/12/07 22:18:55 thorpej Exp $ */
 
 /*-
@@ -1408,6 +1408,7 @@ pmap_page_protect(struct vm_page *pg, vm_prot_t prot)
        case VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE:
        case VM_PROT_READ|VM_PROT_WRITE:
                return;
+
        /* copy_on_write */
        case VM_PROT_READ|VM_PROT_EXECUTE:
        case VM_PROT_READ:
@@ -1415,6 +1416,7 @@ pmap_page_protect(struct vm_page *pg, vm_prot_t prot)
 /* XXX */      pmap_changebit(pg, 0, ~(PG_KWE | PG_UWE), cpu_id);
                PMAP_HEAD_TO_MAP_UNLOCK();
                return;
+
        /* remove_all */
        default:
                break;
@@ -1494,8 +1496,6 @@ pmap_protect(pmap_t pmap, vaddr_t sva, vaddr_t eva, vm_prot_t prot)
        PMAP_LOCK(pmap);
 
        bits = pte_prot(pmap, prot);
-       if (!pmap_pte_exec(&bits))
-               bits |= PG_FOE;
        isactive = PMAP_ISACTIVE(pmap, cpu_id);
 
        l1pte = pmap_l1pte(pmap, sva);
@@ -1811,10 +1811,6 @@ pmap_enter(pmap_t pmap, vaddr_t va, paddr_t pa, vm_prot_t prot, int flags)
                else if ((attrs & PGA_MODIFIED) == 0)
                        npte |= PG_FOW;
 
-               /* Always force FOE on non-exec mappings. */
-               if (!pmap_pte_exec(pte))
-                       npte |= PG_FOE;
-
                /*
                 * Mapping was entered on PV list.
                 */
@@ -2410,39 +2406,23 @@ alpha_protection_init(void)
        up = protection_codes[1];
 
        for (prot = 0; prot < 8; prot++) {
-               kp[prot] = 0; up[prot] = 0;
-               switch (prot) {
-               case VM_PROT_NONE | VM_PROT_NONE | VM_PROT_NONE:
-                       kp[prot] |= PG_ASM;
-                       up[prot] |= 0;
-                       break;
-
-               case VM_PROT_READ | VM_PROT_NONE | VM_PROT_EXECUTE:
-               case VM_PROT_NONE | VM_PROT_NONE | VM_PROT_EXECUTE:
-                       kp[prot] |= PG_EXEC;            /* software */
-                       up[prot] |= PG_EXEC;            /* software */
-                       /* FALLTHROUGH */
+               kp[prot] = PG_ASM;
+               up[prot] = 0;
 
-               case VM_PROT_READ | VM_PROT_NONE | VM_PROT_NONE:
-                       kp[prot] |= PG_ASM | PG_KRE;
-                       up[prot] |= PG_URE | PG_KRE;
-                       break;
-
-               case VM_PROT_NONE | VM_PROT_WRITE | VM_PROT_NONE:
-                       kp[prot] |= PG_ASM | PG_KWE;
-                       up[prot] |= PG_UWE | PG_KWE;
-                       break;
-
-               case VM_PROT_NONE | VM_PROT_WRITE | VM_PROT_EXECUTE:
-               case VM_PROT_READ | VM_PROT_WRITE | VM_PROT_EXECUTE:
-                       kp[prot] |= PG_EXEC;            /* software */
-                       up[prot] |= PG_EXEC;            /* software */
-                       /* FALLTHROUGH */
-
-               case VM_PROT_READ | VM_PROT_WRITE | VM_PROT_NONE:
-                       kp[prot] |= PG_ASM | PG_KWE | PG_KRE;
-                       up[prot] |= PG_UWE | PG_URE | PG_KWE | PG_KRE;
-                       break;
+               if (prot & VM_PROT_READ) {
+                       kp[prot] |= PG_KRE;
+                       up[prot] |= PG_KRE | PG_URE;
+               }
+               if (prot & VM_PROT_WRITE) {
+                       kp[prot] |= PG_KWE;
+                       up[prot] |= PG_KWE | PG_UWE;
+               }
+               if (prot & VM_PROT_EXECUTE) {
+                       kp[prot] |= PG_EXEC | PG_KRE;
+                       up[prot] |= PG_EXEC | PG_KRE | PG_URE;
+               } else {
+                       kp[prot] |= PG_FOE;
+                       up[prot] |= PG_FOE;
                }
        }
 }
@@ -2567,9 +2547,6 @@ pmap_remove_mapping(pmap_t pmap, vaddr_t va, pt_entry_t *pte,
  *     Note: we assume that the pv_head is already locked, and that
  *     the caller has acquired a PV->pmap mutex so that we can lock
  *     the pmaps as we encounter them.
- *
- *     XXX This routine could stand to have some I-stream
- *     XXX optimization done.
  */
 void
 pmap_changebit(struct vm_page *pg, u_long set, u_long mask, cpuid_t cpu_id)
@@ -2612,17 +2589,18 @@ pmap_changebit(struct vm_page *pg, u_long set, u_long mask, cpuid_t cpu_id)
  * pmap_emulate_reference:
  *
  *     Emulate reference and/or modified bit hits.
- *
- *     return non-zero if this was a FOE fault and the pte is not
- *     executable.
+ *     Return non-zero if this was an execute fault on a non-exec mapping,
+ *     otherwise return 0.
  */
 int
 pmap_emulate_reference(struct proc *p, vaddr_t v, int user, int type)
 {
+       struct pmap *pmap;
        pt_entry_t faultoff, *pte;
        struct vm_page *pg;
        paddr_t pa;
        boolean_t didlock = FALSE;
+       boolean_t exec = FALSE;
        cpuid_t cpu_id = cpu_number();
 
 #ifdef DEBUG
@@ -2648,16 +2626,18 @@ pmap_emulate_reference(struct proc *p, vaddr_t v, int user, int type)
                if (p->p_vmspace == NULL)
                        panic("pmap_emulate_reference: bad p_vmspace");
 #endif
-               PMAP_LOCK(p->p_vmspace->vm_map.pmap);
+               pmap = p->p_vmspace->vm_map.pmap;
+               PMAP_LOCK(pmap);
                didlock = TRUE;
-               pte = pmap_l3pte(p->p_vmspace->vm_map.pmap, v, NULL);
+               pte = pmap_l3pte(pmap, v, NULL);
                /*
                 * We'll unlock below where we're done with the PTE.
                 */
        }
-       if (!pmap_pte_exec(pte) && type == ALPHA_MMCSR_FOE) {
+       exec = pmap_pte_exec(pte);
+       if (!exec && type == ALPHA_MMCSR_FOE) {
                if (didlock)
-                       PMAP_UNLOCK(p->p_vmspace->vm_map.pmap);
+                       PMAP_UNLOCK(pmap);
                return (1);
        }
 #ifdef DEBUG
@@ -2695,7 +2675,7 @@ pmap_emulate_reference(struct proc *p, vaddr_t v, int user, int type)
         * it now.
         */
        if (didlock)
-               PMAP_UNLOCK(p->p_vmspace->vm_map.pmap);
+               PMAP_UNLOCK(pmap);
 
 #ifdef DEBUG
        if (pmapdebug & PDB_FOLLOW)
@@ -2722,16 +2702,14 @@ pmap_emulate_reference(struct proc *p, vaddr_t v, int user, int type)
 
        if (type == ALPHA_MMCSR_FOW) {
                pg->mdpage.pvh_attrs |= (PGA_REFERENCED|PGA_MODIFIED);
-               faultoff = PG_FOR | PG_FOW | PG_FOE;
+               faultoff = PG_FOR | PG_FOW;
        } else {
                pg->mdpage.pvh_attrs |= PGA_REFERENCED;
-               faultoff = PG_FOR | PG_FOE;
+               faultoff = PG_FOR;
+               if (exec) {
+                       faultoff |= PG_FOE;
+               }
        }
-       /*
-        * If the page is not PG_EXEC, pmap_changebit will automagically
-        * set PG_FOE (gross, but necessary if I don't want to change the
-        * whole API).
-        */
        pmap_changebit(pg, 0, ~faultoff, cpu_id);
 
        PMAP_HEAD_TO_MAP_UNLOCK();
index e886239..878ca3f 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: trap.c,v 1.64 2012/12/31 06:46:13 guenther Exp $ */
+/* $OpenBSD: trap.c,v 1.65 2014/01/06 20:27:44 miod Exp $ */
 /* $NetBSD: trap.c,v 1.52 2000/05/24 16:48:33 thorpej Exp $ */
 
 /*-
@@ -365,8 +365,7 @@ trap(a0, a1, a2, entry, framep)
                case ALPHA_MMCSR_FOE:
                case ALPHA_MMCSR_FOW:
                        if (pmap_emulate_reference(p, a0, user, a1)) {
-                               /* XXX - stupid API right now. */
-                               ftype = VM_PROT_EXECUTE|VM_PROT_READ;
+                               ftype = VM_PROT_EXECUTE;
                                goto do_fault;
                        }
                        goto out;
@@ -382,7 +381,7 @@ trap(a0, a1, a2, entry, framep)
 
                        switch (a2) {
                        case -1:                /* instruction fetch fault */
-                               ftype = VM_PROT_EXECUTE|VM_PROT_READ;
+                               ftype = VM_PROT_EXECUTE;
                                break;
                        case 0:                 /* load instruction */
                                ftype = VM_PROT_READ;
index 32179c7..a1dbfae 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: pte.h,v 1.10 2011/03/23 16:54:34 pirofti Exp $ */
+/* $OpenBSD: pte.h,v 1.11 2014/01/06 20:27:44 miod Exp $ */
 /* $NetBSD: pte.h,v 1.26 1999/04/09 00:38:11 thorpej Exp $ */
 
 /*-
@@ -93,7 +93,7 @@ typedef       alpha_pt_entry_t        pt_entry_t;
 #define        PG_URE          ALPHA_PTE_UR
 #define        PG_KWE          ALPHA_PTE_KW
 #define        PG_UWE          ALPHA_PTE_UW
-#define        PG_PROT         ALPHA_PTE_PROT
+#define        PG_PROT         (ALPHA_PTE_PROT | PG_EXEC | PG_FOE)
 #define        PG_RSVD         0x000000000000cc80      /* Reserved for hardware */
 #define        PG_WIRED        0x0000000000010000      /* Wired. [SOFTWARE] */
 #define        PG_PVLIST       0x0000000000020000      /* on pv list [SOFTWARE] */