Date: Wed, 13 Jun 2012 22:12:05 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Ian Lepore <freebsd@damnhippie.dyndns.org> Cc: Wojciech Puchar <wojtek@wojtek.tensor.gdynia.pl>, freebsd-hackers@freebsd.org Subject: Rtld object tasting [Was: Re: wired memory - again!] Message-ID: <20120613191205.GU2337@deviant.kiev.zoral.com.ua> In-Reply-To: <1339593249.73426.5.camel@revolution.hippie.lan> References: <alpine.BSF.2.00.1206090920030.84632@wojtek.tensor.gdynia.pl> <1339259223.36051.328.camel@revolution.hippie.lan> <20120609165217.GO85127@deviant.kiev.zoral.com.ua> <alpine.BSF.2.00.1206092244550.9248@wojtek.tensor.gdynia.pl> <1339512694.36051.362.camel@revolution.hippie.lan> <20120612204508.GP2337@deviant.kiev.zoral.com.ua> <1339593249.73426.5.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
--Ivd6Nf4GAZ12BQqh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 13, 2012 at 07:14:09AM -0600, Ian Lepore wrote: > http://lists.freebsd.org/pipermail/freebsd-arm/2012-January/003288.html The map_object.c patch is step in the almost right direction, I wanted to remove the static page-sized buffer from get_elf_header for long time. It works because rtld always holds bind_lock exclusively while loading an object. There is no need to copy the first page after it is mapped. commit 0f6f8629af1345acded7c0c685d3ff7b4d9180d6 Author: Konstantin Belousov <kib@freebsd.org> Date: Wed Jun 13 22:04:18 2012 +0300 Eliminate the static buffer used to read the first page of the mapped object, and eliminate the pread(2) call as well. Mmap the first page of the object temporaly, and unmap it on error or last use. =20 Fix several cases were the whole mapping of the object leaked on error. =20 Potentially, this leaves one-page gap between succeeding dlopen(3), but there are other mmap(2) consumers as well. diff --git a/libexec/rtld-elf/map_object.c b/libexec/rtld-elf/map_object.c index d74ef15..7068433 100644 --- a/libexec/rtld-elf/map_object.c +++ b/libexec/rtld-elf/map_object.c @@ -38,7 +38,7 @@ #include "debug.h" #include "rtld.h" =20 -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 */ =20 @@ -121,7 +121,7 @@ map_object(int fd, const char *path, const struct stat = *sb) if ((segs[nsegs]->p_align & (PAGE_SIZE - 1)) !=3D 0) { _rtld_error("%s: PT_LOAD segment %d not page-aligned", path, nsegs); - return NULL; + goto error; } break; =20 @@ -161,12 +161,12 @@ map_object(int fd, const char *path, const struct sta= t *sb) } if (phdyn =3D=3D NULL) { _rtld_error("%s: object is not dynamically-linked", path); - return NULL; + goto error; } =20 if (nsegs < 0) { _rtld_error("%s: too few PT_LOAD segments", path); - return NULL; + goto error; } =20 /* @@ -183,13 +183,12 @@ map_object(int fd, const char *path, const struct sta= t *sb) if (mapbase =3D=3D (caddr_t) -1) { _rtld_error("%s: mmap of entire address space failed: %s", path, rtld_strerror(errno)); - return NULL; + goto error; } if (base_addr !=3D NULL && mapbase !=3D base_addr) { _rtld_error("%s: mmap returned wrong address: wanted %p, got %p", path, base_addr, mapbase); - munmap(mapbase, mapsize); - return NULL; + goto error1; } =20 for (i =3D 0; i <=3D nsegs; i++) { @@ -204,7 +203,7 @@ map_object(int fd, const char *path, const struct stat = *sb) data_flags, fd, data_offset) =3D=3D (caddr_t) -1) { _rtld_error("%s: mmap of data failed: %s", path, rtld_strerror(errno)); - return NULL; + goto error1; } =20 /* Do BSS setup */ @@ -221,7 +220,7 @@ map_object(int fd, const char *path, const struct stat = *sb) mprotect(clear_page, PAGE_SIZE, data_prot|PROT_WRITE)) { _rtld_error("%s: mprotect failed: %s", path, rtld_strerror(errno)); - return NULL; + goto error1; } =20 memset(clear_addr, 0, nclear); @@ -240,7 +239,7 @@ map_object(int fd, const char *path, const struct stat = *sb) data_flags | MAP_ANON, -1, 0) =3D=3D (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, const struct stat = *sb) if (obj->phdr =3D=3D 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 =3D true; @@ -293,63 +292,72 @@ map_object(int fd, const char *path, const struct sta= t *sb) obj->relro_page =3D obj->relocbase + trunc_page(relro_page); obj->relro_size =3D round_page(relro_size); =20 - return obj; + munmap(hdr, PAGE_SIZE); + return (obj); + +error1: + munmap(mapbase, mapsize); +error: + munmap(hdr, PAGE_SIZE); + return (NULL); } =20 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 =3D pread(fd, u.buf, PAGE_SIZE, 0)) =3D=3D -1) { - _rtld_error("%s: read error: %s", path, rtld_strerror(errno)); - return NULL; - } + Elf_Ehdr *hdr; =20 - /* 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] !=3D ELF_TARG_CLASS - || u.hdr.e_ident[EI_DATA] !=3D ELF_TARG_DATA) { - _rtld_error("%s: unsupported file layout", path); - return NULL; - } - if (u.hdr.e_ident[EI_VERSION] !=3D EV_CURRENT - || u.hdr.e_version !=3D EV_CURRENT) { - _rtld_error("%s: unsupported file version", path); - return NULL; - } - if (u.hdr.e_type !=3D ET_EXEC && u.hdr.e_type !=3D ET_DYN) { - _rtld_error("%s: unsupported file type", path); - return NULL; - } - if (u.hdr.e_machine !=3D ELF_TARG_MACH) { - _rtld_error("%s: unsupported machine", path); - return NULL; - } + hdr =3D mmap(NULL, PAGE_SIZE, PROT_READ, 0, fd, 0); + if (hdr =3D=3D (Elf_Ehdr *)MAP_FAILED) { + _rtld_error("%s: read error: %s", path, rtld_strerror(errno)); + return NULL; + } =20 - /* - * 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 !=3D sizeof(Elf_Phdr)) { - _rtld_error( - "%s: invalid shared object: e_phentsize !=3D 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; - } + /* 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] !=3D ELF_TARG_CLASS || + hdr->e_ident[EI_DATA] !=3D ELF_TARG_DATA) { + _rtld_error("%s: unsupported file layout", path); + goto error; + } + if (hdr->e_ident[EI_VERSION] !=3D EV_CURRENT || + hdr->e_version !=3D EV_CURRENT) { + _rtld_error("%s: unsupported file version", path); + goto error; + } + if (hdr->e_type !=3D ET_EXEC && hdr->e_type !=3D ET_DYN) { + _rtld_error("%s: unsupported file type", path); + goto error; + } + if (hdr->e_machine !=3D ELF_TARG_MACH) { + _rtld_error("%s: unsupported machine", path); + goto error; + } + + /* + * 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 !=3D sizeof(Elf_Phdr)) { + _rtld_error( + "%s: invalid shared object: e_phentsize !=3D 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); =20 - return (&u.hdr); +error: + munmap(hdr, PAGE_SIZE); + return (NULL); } =20 void --Ivd6Nf4GAZ12BQqh Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk/Y5gUACgkQC3+MBN1Mb4hoagCeOXwjxBuWFDTlywn8fJcLlpUq apwAoPduJnKqW0X9U9KjZiMMIP1LbhzP =OcdG -----END PGP SIGNATURE----- --Ivd6Nf4GAZ12BQqh--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120613191205.GU2337>