[PATCH] dl-load: avoid excessive executable mappings for ET_DYN objects

[Mathias Krause <minipli at googlemail dot com>]

When we load an ET_DYN object the whole address range for the object
gets reserved with the first call to mmap(). The protection flags used
for this mapping are the flags of the first segment, which is, for
most ET_DYN objects, the code segment. This makes the whole address
range being marked as executable and opens up a short time frame where
the address mapping conflicts with the segments specified in the
ET_DYN object. By using PROT_NONE for the initial mapping this can be
avoided.

This patch changes the behaviour of ET_DYN loading by reserving the
address range for the object with a single PROT_NONE mapping. All
relevant sections are mapped afterwards using separate calls to
mmap().  Changing the protection flags of the first mapping using
mprotect() won't work on systems that enforce an W^X policy for every
mapping -- like any PaX enabled Linux kernel.

This, though, adds one syscall per ET_DYN object mapping not having
any holes. But it solves the problem of having an inconsistent view of
the sections from the object in memory.

Signed-off-by: Mathias Krause <minipli@googlemail.com>
---
 ChangeLog     |    5 +++++
 elf/dl-load.c |   22 ++++++++--------------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 23dab2e..53e454b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-10-26  Mathias Krause  <minipli@googlemail.com>
+
+	* elf/dl-load.c: Reserve address range of ET_DYN objects with PROT_NONE
+	instead of using the flags of the first segment.
+
 2011-10-25  Ulrich Drepper  <drepper@gmail.com>
 
 	* sysdeps/ieee754/dbl-64/e_rem_pio2.c: Comment everything out, the
diff --git a/elf/dl-load.c b/elf/dl-load.c
index f3717ea..cb70d97 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1279,9 +1279,13 @@ cannot allocate TLS data structures for initial thread");
 					  c->mapstart & GLRO(dl_use_load_bias))
 		   - MAP_BASE_ADDR (l));
 
-	/* Remember which part of the address space this object uses.  */
+	/* Remember which part of the address space this object uses.
+
+	   We're mapping the whole addres range PROT_NONE just to prevent it from
+	   being used for other mappings by other threads.  The segments get
+	   mapped using there appropriate protection flags later on. */
 	l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength,
-					      c->prot,
+					      PROT_NONE,
 					      MAP_COPY|MAP_FILE,
 					      fd, c->mapoff);
 	if (__builtin_expect ((void *) l->l_map_start == MAP_FAILED, 0))
@@ -1294,19 +1298,9 @@ cannot allocate TLS data structures for initial thread");
 	l->l_map_end = l->l_map_start + maplength;
 	l->l_addr = l->l_map_start - c->mapstart;
 
-	if (has_holes)
-	  /* Change protection on the excess portion to disallow all access;
-	     the portions we do not remap later will be inaccessible as if
-	     unallocated.  Then jump into the normal segment-mapping loop to
-	     handle the portion of the segment past the end of the file
-	     mapping.  */
-	  __mprotect ((caddr_t) (l->l_addr + c->mapend),
-		      loadcmds[nloadcmds - 1].mapstart - c->mapend,
-		      PROT_NONE);
-
 	l->l_contiguous = 1;
 
-	goto postmap;
+	goto beforemap;
       }
 
     /* This object is loaded at a fixed address.  This must never
@@ -1329,6 +1323,7 @@ cannot allocate TLS data structures for initial thread");
 
     while (c < &loadcmds[nloadcmds])
       {
+      beforemap:
 	if (c->mapend > c->mapstart
 	    /* Map the segment contents from the file.  */
 	    && (__mmap ((void *) (l->l_addr + c->mapstart),
@@ -1338,7 +1333,6 @@ cannot allocate TLS data structures for initial thread");
 		== MAP_FAILED))
 	  goto map_error;
 
-      postmap:
 	if (c->prot & PROT_EXEC)
 	  l->l_text_end = l->l_addr + c->mapend;
 
-- 
1.5.6.5