This is a multi-part message in MIME format.
--------------000803010308000209060707
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

YAMAMOTO Takashi wrote:

> - pax_mprotect_adjust seems no-op.
> - "*new_prot &= ~VM_PROT_EXECUTE;" in obj != NULL case of pax_mprotect
>   is no-op because the condition isn't met if VM_PROT_EXECUTE is set.
> - mprotect(2) is not only user of uvm_map_protect.
>   have you checked the rest of users?
> - uobj == NULL is not a good test for anonymous memory,
>   if you want to count eg. sysv shared memory or COW'ed memory as anonymous.
>   (i'm not sure why you want to check if anonymous or not here.)
> - the semantics of uvm_map_protect after your change is not clear to me.
>   eg. consider doing mprotect(100, 100, WRITE|EXEC) and
>   [100..150] is file-backed and [150..200] is anonymous memory.
>   if i read your change correctly, both of WRITE and EXEC bits will be
>   removed for the entire range.  is it an intended behaviour?
> - i couldn't find any code to restrict mmap.  does this make sense without it?

See if attached diff solves some of the above?

> have you had any public review of the patch?

Of an earlier version.. the screw-ups are all mine. :)

-e.

-- 
Elad Efrat

--------------000803010308000209060707
Content-Type: text/plain;
 name="pax_mprotect.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="pax_mprotect.diff"

Index: sys/pax.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pax.h,v
retrieving revision 1.1
diff -u -p -r1.1 pax.h
--- sys/pax.h	16 May 2006 00:08:25 -0000	1.1
+++ sys/pax.h	16 May 2006 21:42:53 -0000
@@ -33,7 +33,7 @@
 #ifndef __SYS_PAX_H__
 #define	__SYS_PAX_H__
 
-void pax_mprotect(struct lwp *, struct uvm_object *, vm_prot_t *);
+void pax_mprotect(struct lwp *, void *, vm_prot_t *, vm_prot_t *);
 void pax_mprotect_adjust(struct lwp *, int);
 
 #endif /* !__SYS_PAX_H__ */
Index: kern/kern_pax.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_pax.c,v
retrieving revision 1.1
diff -u -p -r1.1 kern_pax.c
--- kern/kern_pax.c	16 May 2006 00:08:25 -0000	1.1
+++ kern/kern_pax.c	16 May 2006 21:42:53 -0000
@@ -84,8 +84,7 @@ SYSCTL_SETUP(sysctl_security_pax_setup, 
 void
 pax_mprotect_adjust(struct lwp *l, int f)
 {
-	if (!pax_mprotect_enabled ||
-	    (f & (PF_PAXMPROTECT|PF_PAXNOMPROTECT)))
+	if (!pax_mprotect_enabled)
 		return;
 
 	if (f & PF_PAXMPROTECT)
@@ -95,22 +94,18 @@ pax_mprotect_adjust(struct lwp *l, int f
 }
 
 void
-pax_mprotect(struct lwp *l, struct uvm_object *obj, vm_prot_t *new_prot)
+pax_mprotect(struct lwp *l, void *handle, vm_prot_t *prot, vm_prot_t *maxprot)
 {
 	if (!pax_mprotect_enabled ||
 	    (pax_mprotect_global && (l->l_proc->p_flag & P_PAXNOMPROTECT)) ||
 	    (!pax_mprotect_global && !(l->l_proc->p_flag & P_PAXMPROTECT)))
 		return;
 
-	if (obj == NULL) {
-		/* Anonymous mappings always get their execute bit stripped. */
-		*new_prot &= ~VM_PROT_EXECUTE;
+	if ((*prot & (VM_PROT_WRITE|VM_PROT_EXECUTE)) != VM_PROT_EXECUTE) {
+		*prot &= ~VM_PROT_EXECUTE;
+		*maxprot &= ~VM_PROT_EXECUTE;
 	} else {
-		/* File mappings. */
-		if ((*new_prot & (VM_PROT_WRITE|VM_PROT_EXECUTE)) ==
-		    VM_PROT_WRITE)
-			*new_prot &= ~VM_PROT_EXECUTE;
-		else
-			*new_prot &= ~VM_PROT_WRITE;
+		*prot &= ~VM_PROT_WRITE;
+		*maxprot &= ~VM_PROT_WRITE;
 	}
 }
Index: uvm/uvm_map.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_map.c,v
retrieving revision 1.224
diff -u -p -r1.224 uvm_map.c
--- uvm/uvm_map.c	16 May 2006 00:08:25 -0000	1.224
+++ uvm/uvm_map.c	16 May 2006 21:42:59 -0000
@@ -77,7 +77,6 @@ __KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 
 #include "opt_uvmhist.h"
 #include "opt_uvm.h"
 #include "opt_sysv.h"
-#include "opt_pax.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -101,10 +100,6 @@ __KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 
 #include <uvm/uvm_ddb.h>
 #endif
 
-#ifdef PAX_MPROTECT
-#include <sys/pax.h>
-#endif /* PAX_MPROTECT */
-
 #if defined(UVMMAP_NOCOUNTERS)
 
 #define	UVMMAP_EVCNT_DEFINE(name)	/* nothing */
@@ -2834,10 +2829,6 @@ uvm_map_protect(struct vm_map *map, vadd
 			}
 		}
 
-#ifdef PAX_MPROTECT
-		pax_mprotect(curlwp, current->object.uvm_obj, &new_prot);
-#endif /* PAX_MPROTECT */
-
 		current = current->next;
 	}
 
Index: uvm/uvm_mmap.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_mmap.c,v
retrieving revision 1.96
diff -u -p -r1.96 uvm_mmap.c
--- uvm/uvm_mmap.c	14 May 2006 21:38:17 -0000	1.96
+++ uvm/uvm_mmap.c	16 May 2006 21:43:00 -0000
@@ -54,6 +54,7 @@
 __KERNEL_RCSID(0, "$NetBSD: uvm_mmap.c,v 1.96 2006/05/14 21:38:17 elad Exp $");
 
 #include "opt_compat_netbsd.h"
+#include "opt_pax.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -67,6 +68,10 @@ __KERNEL_RCSID(0, "$NetBSD: uvm_mmap.c,v
 #include <sys/vnode.h>
 #include <sys/conf.h>
 #include <sys/stat.h>
+ 
+#ifdef PAX_MPROTECT
+#include <sys/pax.h>
+#endif /* PAX_MPROTECT */
 
 #include <miscfs/specfs/specdev.h>
 
@@ -500,6 +505,10 @@ sys_mmap(l, v, retval)
 		}
 	}
 
+#ifdef PAX_MPROTECT
+	pax_mprotect(l, handle, &prot, &maxprot);
+#endif /* PAX_MPROTECT */
+
 	/*
 	 * now let kernel internal function uvm_mmap do the work.
 	 */

--------------000803010308000209060707--