Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 14 Jun 2012 11:20:23 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r237058 - head/libexec/rtld-elf
Message-ID:  <201206141120.q5EBKNs0090889@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Thu Jun 14 11:20:22 2012
New Revision: 237058
URL: http://svn.freebsd.org/changeset/base/237058

Log:
  Eliminate the static buffer used to read the first page of the mapped
  object, and eliminate the pread(2) call as well [1]. Mmap the first
  page of the object temporaly, and unmap it on error or last use.
  Potentially, this leaves one-page gap between succeeding dlopen(3),
  but there are other mmap(2) consumers as well.
  
  Fix several cases were the whole mapping of the object leaked on error.
  
  Use MAP_PREFAULT_READ for mmap(2) calls which map real object pages [2].
  
  Insipired by the patch by:	Ian Lepore <freebsd damnhippie dyndns org> [1]
  Suggested by:	alc [2]
  MFC after:	2 weeks

Modified:
  head/libexec/rtld-elf/map_object.c

Modified: head/libexec/rtld-elf/map_object.c
==============================================================================
--- head/libexec/rtld-elf/map_object.c	Thu Jun 14 11:17:54 2012	(r237057)
+++ head/libexec/rtld-elf/map_object.c	Thu Jun 14 11:20:22 2012	(r237058)
@@ -38,7 +38,7 @@
 #include "debug.h"
 #include "rtld.h"
 
-static Elf_Ehdr *get_elf_header (int, const char *);
+static Elf_Ehdr *get_elf_header(int, const char *);
 static int convert_prot(int);	/* Elf flags -> mmap protection */
 static int convert_flags(int); /* Elf flags -> mmap flags */
 
@@ -121,7 +121,7 @@ map_object(int fd, const char *path, con
     	    if ((segs[nsegs]->p_align & (PAGE_SIZE - 1)) != 0) {
 		_rtld_error("%s: PT_LOAD segment %d not page-aligned",
 		    path, nsegs);
-		return NULL;
+		goto error;
 	    }
 	    break;
 
@@ -161,12 +161,12 @@ map_object(int fd, const char *path, con
     }
     if (phdyn == NULL) {
 	_rtld_error("%s: object is not dynamically-linked", path);
-	return NULL;
+	goto error;
     }
 
     if (nsegs < 0) {
 	_rtld_error("%s: too few PT_LOAD segments", path);
-	return NULL;
+	goto error;
     }
 
     /*
@@ -183,13 +183,12 @@ map_object(int fd, const char *path, con
     if (mapbase == (caddr_t) -1) {
 	_rtld_error("%s: mmap of entire address space failed: %s",
 	  path, rtld_strerror(errno));
-	return NULL;
+	goto error;
     }
     if (base_addr != NULL && mapbase != base_addr) {
 	_rtld_error("%s: mmap returned wrong address: wanted %p, got %p",
 	  path, base_addr, mapbase);
-	munmap(mapbase, mapsize);
-	return NULL;
+	goto error1;
     }
 
     for (i = 0; i <= nsegs; i++) {
@@ -201,10 +200,10 @@ map_object(int fd, const char *path, con
 	data_prot = convert_prot(segs[i]->p_flags);
 	data_flags = convert_flags(segs[i]->p_flags) | MAP_FIXED;
 	if (mmap(data_addr, data_vlimit - data_vaddr, data_prot,
-	  data_flags, fd, data_offset) == (caddr_t) -1) {
+	  data_flags | MAP_PREFAULT_READ, fd, data_offset) == (caddr_t) -1) {
 	    _rtld_error("%s: mmap of data failed: %s", path,
 		rtld_strerror(errno));
-	    return NULL;
+	    goto error1;
 	}
 
 	/* Do BSS setup */
@@ -221,7 +220,7 @@ map_object(int fd, const char *path, con
 		     mprotect(clear_page, PAGE_SIZE, data_prot|PROT_WRITE)) {
 			_rtld_error("%s: mprotect failed: %s", path,
 			    rtld_strerror(errno));
-			return NULL;
+			goto error1;
 		}
 
 		memset(clear_addr, 0, nclear);
@@ -240,7 +239,7 @@ map_object(int fd, const char *path, con
 		    data_flags | MAP_ANON, -1, 0) == (caddr_t)-1) {
 		    _rtld_error("%s: mmap of bss failed: %s", path,
 			rtld_strerror(errno));
-		    return NULL;
+		    goto error1;
 		}
 	    }
 	}
@@ -273,7 +272,7 @@ map_object(int fd, const char *path, con
 	if (obj->phdr == NULL) {
 	    obj_free(obj);
 	    _rtld_error("%s: cannot allocate program header", path);
-	     return NULL;
+	    goto error1;
 	}
 	memcpy((char *)obj->phdr, (char *)hdr + hdr->e_phoff, phsize);
 	obj->phdr_alloc = true;
@@ -293,63 +292,72 @@ map_object(int fd, const char *path, con
     obj->relro_page = obj->relocbase + trunc_page(relro_page);
     obj->relro_size = round_page(relro_size);
 
-    return obj;
+    munmap(hdr, PAGE_SIZE);
+    return (obj);
+
+error1:
+    munmap(mapbase, mapsize);
+error:
+    munmap(hdr, PAGE_SIZE);
+    return (NULL);
 }
 
 static Elf_Ehdr *
-get_elf_header (int fd, const char *path)
+get_elf_header(int fd, const char *path)
 {
-    static union {
-	Elf_Ehdr hdr;
-	char buf[PAGE_SIZE];
-    } u;
-    ssize_t nbytes;
-
-    if ((nbytes = pread(fd, u.buf, PAGE_SIZE, 0)) == -1) {
-	_rtld_error("%s: read error: %s", path, rtld_strerror(errno));
-	return NULL;
-    }
-
-    /* Make sure the file is valid */
-    if (nbytes < (ssize_t)sizeof(Elf_Ehdr) || !IS_ELF(u.hdr)) {
-	_rtld_error("%s: invalid file format", path);
-	return NULL;
-    }
-    if (u.hdr.e_ident[EI_CLASS] != ELF_TARG_CLASS
-      || u.hdr.e_ident[EI_DATA] != ELF_TARG_DATA) {
-	_rtld_error("%s: unsupported file layout", path);
-	return NULL;
-    }
-    if (u.hdr.e_ident[EI_VERSION] != EV_CURRENT
-      || u.hdr.e_version != EV_CURRENT) {
-	_rtld_error("%s: unsupported file version", path);
-	return NULL;
-    }
-    if (u.hdr.e_type != ET_EXEC && u.hdr.e_type != ET_DYN) {
-	_rtld_error("%s: unsupported file type", path);
-	return NULL;
-    }
-    if (u.hdr.e_machine != ELF_TARG_MACH) {
-	_rtld_error("%s: unsupported machine", path);
-	return NULL;
-    }
+	Elf_Ehdr *hdr;
 
-    /*
-     * We rely on the program header being in the first page.  This is
-     * not strictly required by the ABI specification, but it seems to
-     * always true in practice.  And, it simplifies things considerably.
-     */
-    if (u.hdr.e_phentsize != sizeof(Elf_Phdr)) {
-	_rtld_error(
-	  "%s: invalid shared object: e_phentsize != sizeof(Elf_Phdr)", path);
-	return NULL;
-    }
-    if (u.hdr.e_phoff + u.hdr.e_phnum * sizeof(Elf_Phdr) > (size_t)nbytes) {
-	_rtld_error("%s: program header too large", path);
-	return NULL;
-    }
+	hdr = mmap(NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE | MAP_PREFAULT_READ,
+	    fd, 0);
+	if (hdr == (Elf_Ehdr *)MAP_FAILED) {
+		_rtld_error("%s: read error: %s", path, rtld_strerror(errno));
+		return (NULL);
+	}
+
+	/* Make sure the file is valid */
+	if (!IS_ELF(*hdr)) {
+		_rtld_error("%s: invalid file format", path);
+		goto error;
+	}
+	if (hdr->e_ident[EI_CLASS] != ELF_TARG_CLASS ||
+	    hdr->e_ident[EI_DATA] != ELF_TARG_DATA) {
+		_rtld_error("%s: unsupported file layout", path);
+		goto error;
+	}
+	if (hdr->e_ident[EI_VERSION] != EV_CURRENT ||
+	    hdr->e_version != EV_CURRENT) {
+		_rtld_error("%s: unsupported file version", path);
+		goto error;
+	}
+	if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) {
+		_rtld_error("%s: unsupported file type", path);
+		goto error;
+	}
+	if (hdr->e_machine != ELF_TARG_MACH) {
+		_rtld_error("%s: unsupported machine", path);
+		goto error;
+	}
 
-    return (&u.hdr);
+	/*
+	 * We rely on the program header being in the first page.  This is
+	 * not strictly required by the ABI specification, but it seems to
+	 * always true in practice.  And, it simplifies things considerably.
+	 */
+	if (hdr->e_phentsize != sizeof(Elf_Phdr)) {
+		_rtld_error(
+	    "%s: invalid shared object: e_phentsize != sizeof(Elf_Phdr)", path);
+		goto error;
+	}
+	if (hdr->e_phoff + hdr->e_phnum * sizeof(Elf_Phdr) >
+	    (size_t)PAGE_SIZE) {
+		_rtld_error("%s: program header too large", path);
+		goto error;
+	}
+	return (hdr);
+
+error:
+	munmap(hdr, PAGE_SIZE);
+	return (NULL);
 }
 
 void

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201206141120.q5EBKNs0090889>