Current-Users archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: W^X mmap
Hi,
I have simplified the patch, changed it to return EACCES upon errors,
adapted it to -current, and tested it there (both with PAX_MPROTECT set
and not set). It is still not 100% elegant though (adds an #ifdef) so I
will welcome ideas on how to improve it some more.
Cheers,
-- khorben
On 26/12/2016 00:10, Pierre Pronchery wrote:
On 10/12/2016 14:02, Michael van Elst wrote:
coypu%SDF.ORG@localhost writes:
Why doesn't the following code get rejected by pax mprotect?
a = mmap(NULL, BUFSIZ, PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_ANON, -1, 2);
It gets 'rejected' by silently dropping the PROT_EXEC flag.
I find this awful: programs trying to use e.g JIT will fail to detect
that it really is not supported, and crash later instead.
I am attaching here a patch returning errors instead.
Thanks to this patch, www/firefox works without having to set the "m:
mprotect(2) restrictions, explicit disable" flag on its executable
binaries (tested on netbsd-7/amd64).
POSIX would require mmap to fail with errno = EACCES.
In the patch attached I have used ENOTSUP, because this is what OpenBSD
seems to be using:
http://man.openbsd.org/mmap.2
I also think EACCES (or EPERM?) would be better though, so I will be
happy to replace it if considered more appropriate.
I have changed the logic deciding which flags to drop. It used to be,
independently of whether PROT_READ is set:
- if PROT_WRITE, or PROT_WRITE and PROT_EXECUTE are set, then execution
is silently denied;
- otherwise, writing is silently denied.
(which doesn't make much sense to me)
Now there would be only one case instead:
- if PROT_WRITE and PROT_EXECUTE are set, execution is denied and an
error is returned.
Another thing I will really need to know before committing this, is
whether the changes should really be applied to sys_mmap() only.
Finally, I left a XXX where there might be a side-effect, if applied in
sys/kern/exec_subr.c, after calling vn_rdwr().
Cheers,
--
khorben
Index: sys/kern/kern_pax.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_pax.c,v
retrieving revision 1.57
diff -p -u -r1.57 kern_pax.c
--- sys/kern/kern_pax.c 17 Sep 2016 02:29:11 -0000 1.57
+++ sys/kern/kern_pax.c 26 Dec 2016 13:25:40 -0000
@@ -423,7 +423,7 @@ pax_mprotect_elf_flags_active(uint32_t f
return true;
}
-void
+int
pax_mprotect_adjust(
#ifdef PAX_MPROTECT_DEBUG
const char *file, size_t line,
@@ -434,9 +434,10 @@ pax_mprotect_adjust(
flags = l->l_proc->p_pax;
if (!pax_flags_active(flags, P_PAX_MPROTECT))
- return;
+ return 0;
- if ((*prot & (VM_PROT_WRITE|VM_PROT_EXECUTE)) != VM_PROT_EXECUTE) {
+ if ((*prot & (VM_PROT_WRITE|VM_PROT_EXECUTE))
+ == (VM_PROT_WRITE|VM_PROT_EXECUTE)) {
#ifdef PAX_MPROTECT_DEBUG
struct proc *p = l->l_proc;
if ((*prot & VM_PROT_EXECUTE) && pax_mprotect_debug) {
@@ -447,18 +448,10 @@ pax_mprotect_adjust(
#endif
*prot &= ~VM_PROT_EXECUTE;
*maxprot &= ~VM_PROT_EXECUTE;
- } else {
-#ifdef PAX_MPROTECT_DEBUG
- struct proc *p = l->l_proc;
- if ((*prot & VM_PROT_WRITE) && pax_mprotect_debug) {
- printf("%s: %s,%zu: %d.%d (%s): -w\n",
- __func__, file, line,
- p->p_pid, l->l_lid, p->p_comm);
- }
-#endif
- *prot &= ~VM_PROT_WRITE;
- *maxprot &= ~VM_PROT_WRITE;
+ return EACCES;
}
+
+ return 0;
}
/*
Index: sys/sys/pax.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pax.h,v
retrieving revision 1.25
diff -p -u -r1.25 pax.h
--- sys/sys/pax.h 3 Sep 2016 12:20:58 -0000 1.25
+++ sys/sys/pax.h 26 Dec 2016 13:25:41 -0000
@@ -63,7 +63,7 @@ void pax_setup_elf_flags(struct exec_pac
# define pax_setup_elf_flags(e, flags) __USE(flags)
#endif
-void pax_mprotect_adjust(
+int pax_mprotect_adjust(
#ifdef PAX_MPROTECT_DEBUG
const char *, size_t,
#endif
Index: sys/uvm/uvm_mmap.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_mmap.c,v
retrieving revision 1.162
diff -p -u -r1.162 uvm_mmap.c
--- sys/uvm/uvm_mmap.c 9 Aug 2016 12:17:04 -0000 1.162
+++ sys/uvm/uvm_mmap.c 26 Dec 2016 13:25:41 -0000
@@ -411,7 +411,12 @@ sys_mmap(struct lwp *l, const struct sys
pos = 0;
}
- PAX_MPROTECT_ADJUST(l, &prot, &maxprot);
+#ifdef PAX_MPROTECT
+ error = PAX_MPROTECT_ADJUST(l, &prot, &maxprot);
+ if (error) {
+ goto out;
+ }
+#endif
pax_aslr_mmap(l, &addr, orig_addr, flags);
Index: sys/uvm/uvm_unix.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_unix.c,v
retrieving revision 1.47
diff -p -u -r1.47 uvm_unix.c
--- sys/uvm/uvm_unix.c 7 Apr 2016 12:07:36 -0000 1.47
+++ sys/uvm/uvm_unix.c 26 Dec 2016 13:25:41 -0000
@@ -100,6 +100,7 @@ sys_obreak(struct lwp *l, const struct s
vm_prot_t prot = UVM_PROT_READ | UVM_PROT_WRITE;
vm_prot_t maxprot = UVM_PROT_ALL;
+ /* currently never fails with these flags */
PAX_MPROTECT_ADJUST(l, &prot, &maxprot);
error = uvm_map(&vm->vm_map, &obreak, nbreak - obreak, NULL,
Home |
Main Index |
Thread Index |
Old Index