PaX: Heritage bug

[Maxime Villard <max%m00nbsd.net@localhost>]

Hi,
I've found a bug in the way the kernel handles the PaX flags when
loading an ELF binary.

-------------------------- kern/exec_elf.c --------------------------
#if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
	l->l_proc->p_pax = epp->ep_pax_flags;
#endif /* PAX_MPROTECT || PAX_SEGVGUARD || PAX_ASLR */

	if (is_dyn)
		elf_placedynexec(l, epp, eh, ph);
---------------------------------------------------------------------

'epp->ep_pax_flags' is set depending on information found in a section
of the binary being loaded. Here, the current proc PaX flag is
overwritten with the new one, and is then used in elf_placedynexec().

The problem here is that this flag is overwritten in the first part of
the loader, and not in the last one. If exec_elf_makecmds() fails, the
kernel returns an error to userland, and the calling process is still
alive. Therefore, it overwrote for free the PaX flag of the current
process.

It means that if the current process is not PaX'ed, and tries to execve
a PaX'ed binary and fails to, it gets tagged as PaX'ed while it is not.

This can lead to several inconsistencies. For example, the mmap syscall
calls pax_aslr(l), which reads the PaX flag on the fly and offsets the
memory.

To trigger the bug, launch a malformed PaX'ed binary with a normal
binary; and then, in this normal binary, call mmap(). With
PAX_ASLR_DEBUG enabled:

m00nbsd$ gcc -Wl,-dynamic-linker,/DEAD_INTERP_PATH -o paxed paxed.c
m00nbsd$ paxctl +A paxed
m00nbsd$ gcc -o launcher launcher.c
m00nbsd$ ./launcher
EXECVE FAILED!
CALLING MMAP
applying to 0x7f7ff7fff000 orig_addr=0x0 f=1002
result 0x7f7ff7fff000
applying to 0x7f7ff7fff000 orig_addr=0x0 f=1002
result 0x7f7ff7fff000
applying to 0x7f7ff7f00000 orig_addr=0x0 f=14001002
result 0x7f7ff7f00000
execve: -1
applying to 0x7f7ff7fff000 orig_addr=0x0 f=2
result 0x7f7ff7fff000

execve() fails, and the launcher becomes PaX'ed, as testifies the kernel
ASLR output.

Attached is a patch. However I have a doubt: if the kernel loads a
binary, and then its interpreter, the p_pax flag is not used when load-
ing this interpreter, right?

Thanks!
Maxime

Index: kern/exec_elf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/exec_elf.c,v
retrieving revision 1.70
diff -u -r1.70 exec_elf.c
--- kern/exec_elf.c	17 Aug 2014 23:03:58 -0000	1.70
+++ kern/exec_elf.c	24 Feb 2015 15:04:29 -0000
@@ -116,8 +116,7 @@
 #define	ELF_TRUNC(a, b)		((a) & ~((b) - 1))

 static void
-elf_placedynexec(struct lwp *l, struct exec_package *epp, Elf_Ehdr *eh,
-    Elf_Phdr *ph)
+elf_placedynexec(struct exec_package *epp, Elf_Ehdr *eh, Elf_Phdr *ph)
 {
 	Elf_Addr align, offset;
 	int i;
@@ -127,7 +126,7 @@
 			align = ph[i].p_align;

 #ifdef PAX_ASLR
-	if (pax_aslr_active(l)) {
+	if (pax_aslr_exec_active(epp)) {
 		size_t pax_align, l2, delta;
 		uint32_t r;

@@ -722,12 +721,8 @@
 		pos = (Elf_Addr)startp;
 	}

-#if defined(PAX_MPROTECT) || defined(PAX_SEGVGUARD) || defined(PAX_ASLR)
-	l->l_proc->p_pax = epp->ep_pax_flags;
-#endif /* PAX_MPROTECT || PAX_SEGVGUARD || PAX_ASLR */
-
 	if (is_dyn)
-		elf_placedynexec(l, epp, eh, ph);
+		elf_placedynexec(epp, eh, ph);

 	/*
 	 * Load all the necessary sections
Index: kern/kern_exec.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exec.c,v
retrieving revision 1.412
diff -u -r1.412 kern_exec.c
--- kern/kern_exec.c	14 Dec 2014 23:49:28 -0000	1.412
+++ kern/kern_exec.c	24 Feb 2015 15:04:29 -0000
@@ -705,7 +705,7 @@
 	 */

 #ifdef PAX_ASLR
-#define	ASLR_GAP(l)	(pax_aslr_active(l) ? (cprng_fast32() % PAGE_SIZE) : 0)
+#define	ASLR_GAP(l)	(pax_aslr_exec_active(epp) ? (cprng_fast32() %
PAGE_SIZE) : 0)
 #else
 #define	ASLR_GAP(l)	0
 #endif
@@ -1097,6 +1097,9 @@
 	/* Remove POSIX timers */
 	timers_free(p, TIMERS_POSIX);

+	/* Set the PaX flag. No lock needed. */
+	p->p_pax = epp->ep_pax_flags;
+
 	/*
 	 * Do whatever is necessary to prepare the address space
 	 * for remapping.  Note that this might replace the current
Index: kern/kern_pax.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_pax.c,v
retrieving revision 1.27
diff -u -r1.27 kern_pax.c
--- kern/kern_pax.c	25 Feb 2014 18:30:11 -0000	1.27
+++ kern/kern_pax.c	24 Feb 2015 15:04:29 -0000
@@ -284,21 +284,29 @@
 #endif /* PAX_MPROTECT */

 #ifdef PAX_ASLR
-bool
-pax_aslr_active(struct lwp *l)
+static bool
+pax_aslr_is_active(uint32_t f)
 {
-	uint32_t f;
-
 	if (!pax_aslr_enabled)
 		return false;
-
-	f = l->l_proc->p_pax;
 	if ((pax_aslr_global && (f & ELF_NOTE_PAX_NOASLR) != 0) ||
 	    (!pax_aslr_global && (f & ELF_NOTE_PAX_ASLR) == 0))
 		return false;
 	return true;
 }

+bool
+pax_aslr_active(struct lwp *l)
+{
+	return pax_aslr_is_active(l->l_proc->p_pax);
+}
+
+bool
+pax_aslr_exec_active(struct exec_package *epp)
+{
+	return pax_aslr_is_active(epp->ep_pax_flags);
+}
+
 void
 pax_aslr_init(struct lwp *l, struct vmspace *vm)
 {
Index: sys/pax.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pax.h,v
retrieving revision 1.11
diff -u -r1.11 pax.h
--- sys/pax.h	27 Dec 2007 15:21:53 -0000	1.11
+++ sys/pax.h	24 Feb 2015 15:04:29 -0000
@@ -54,6 +54,7 @@
 #define	PAX_ASLR_DELTA(delta, lsb, len)	\
     (((delta) & ((1UL << (len)) - 1)) << (lsb))
 bool pax_aslr_active(struct lwp *);
+bool pax_aslr_exec_active(struct exec_package *epp);
 void pax_aslr_init(struct lwp *, struct vmspace *);
 void pax_aslr_stack(struct lwp *, struct exec_package *, u_long *);
 void pax_aslr(struct lwp *, vaddr_t *, vaddr_t, int);