generic-library/bionic
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

linker: don't perform unnecessary mprotects

Browse Source 
The linker only needs to mark the text segment as
writable iff the file has text relocations. Unnecessarily
calling mprotect when it isn't necessary is slow, and some
security enhanced kernels don't like it. Pages which are
simultaneously writable and executable are considered a no-no.

The vast majority of executables / shared libraries on Android
do NOT have text relocations.

Change-Id: Ic38ce30a99b7e33ecf21efd9c108547a58eafa35
This commit is contained in:
Nick Kralevich 2012-08-10 21:08:42 -07:00
parent 0bf5cf87c6
commit 5135b3ae6e
2 changed files with 28 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
linker/linker.cpp
View File

@ -81,7 +81,7 @@
*/
static int soinfo_link_image(soinfo *si, unsigned wr_offset);
static int soinfo_link_image(soinfo *si);
static int socount = 0;
static soinfo sopool[SO_MAX];
@ -817,20 +817,6 @@ static soinfo* load_library(const char* name)
return NULL;
}
/* Unprotect the segments, i.e. make them writable, to allow
* relocations to work properly. We will later call
* phdr_table_protect_segments() after all of them are applied
* and all constructors are run.
*/
ret = phdr_table_unprotect_segments(phdr_table,
phdr_count,
load_bias);
if (ret < 0) {
DL_ERR("can't unprotect loadable segments for \"%s\": %s",
name, strerror(errno));
return NULL;
}
soinfo_ptr si(name);
if (si.ptr == NULL) {
return NULL;
@ -855,14 +841,12 @@ static soinfo* load_library(const char* name)
static soinfo *
init_library(soinfo *si)
{
unsigned wr_offset = 0xffffffff;
/* At this point we know that whatever is loaded @ base is a valid ELF
* shared library whose segments are properly mapped in. */
TRACE("[ %5d init_library base=0x%08x sz=0x%08x name='%s') ]\n",
pid, si->base, si->size, si->name);
if(soinfo_link_image(si, wr_offset)) {
if(soinfo_link_image(si)) {
/* We failed to link. However, we can only restore libbase
** if no additional libraries have moved it since we updated it.
*/
@ -1432,7 +1416,7 @@ static int nullify_closed_stdio (void)
return return_value;
}
static int soinfo_link_image(soinfo *si, unsigned wr_offset)
static int soinfo_link_image(soinfo *si)
{
unsigned *d;
/* "base" might wrap around UINT32_MAX. */
@ -1466,20 +1450,6 @@ static int soinfo_link_image(soinfo *si, unsigned wr_offset)
&si->ARM_exidx, &si->ARM_exidx_count);
#endif
if (si->flags & (FLAG_EXE | FLAG_LINKER)) {
if (phdr_table_unprotect_segments(si->phdr,
si->phnum,
si->load_bias) < 0) {
/* We can't call DL_ERR if the linker's relocations haven't
* been performed yet */
if (!relocating_linker) {
DL_ERR("can't unprotect segments for \"%s\": %s",
si->name, strerror(errno));
}
goto fail;
}
}
/* extract useful information from dynamic section */
for(d = si->dynamic; *d; d++){
DEBUG("%5d d = %p, d[0] = 0x%08x d[1] = 0x%08x\n", pid, d, d[0], d[1]);
@ -1562,13 +1532,7 @@ static int soinfo_link_image(soinfo *si, unsigned wr_offset)
si->preinit_array_count = ((unsigned)*d) / sizeof(Elf32_Addr);
break;
case DT_TEXTREL:
/* TODO: make use of this. */
/* this means that we might have to write into where the text
* segment was loaded during relocation... Do something with
* it.
*/
DEBUG("%5d Text segment should be writable during relocation.\n",
pid);
si->has_text_relocations = true;
break;
#if defined(ANDROID_MIPS_LINKER)
case DT_NEEDED:
@ -1664,6 +1628,19 @@ static int soinfo_link_image(soinfo *si, unsigned wr_offset)
}
}
if (si->has_text_relocations) {
/* Unprotect the segments, i.e. make them writable, to allow
* text relocations to work properly. We will later call
* phdr_table_protect_segments() after all of them are applied
* and all constructors are run.
*/
if (phdr_table_unprotect_segments(si->phdr, si->phnum, si->load_bias) < 0) {
DL_ERR("can't unprotect loadable segments for \"%s\": %s",
si->name, strerror(errno));
goto fail;
}
}
if(si->plt_rel) {
DEBUG("[ %5d relocating %s plt ]\n", pid, si->name );
if(soinfo_relocate(si, si->plt_rel, si->plt_rel_count))
@ -1684,12 +1661,14 @@ static int soinfo_link_image(soinfo *si, unsigned wr_offset)
si->flags |= FLAG_LINKED;
DEBUG("[ %5d finished linking %s ]\n", pid, si->name);
/* All relocations are done, we can protect our segments back to
* read-only. */
if (phdr_table_protect_segments(si->phdr, si->phnum, si->load_bias) < 0) {
DL_ERR("can't protect segments for \"%s\": %s",
si->name, strerror(errno));
goto fail;
if (si->has_text_relocations) {
/* All relocations are done, we can protect our segments back to
* read-only. */
if (phdr_table_protect_segments(si->phdr, si->phnum, si->load_bias) < 0) {
DL_ERR("can't protect segments for \"%s\": %s",
si->name, strerror(errno));
goto fail;
}
}
/* We can also turn on GNU RELRO protection */
@ -1911,7 +1890,7 @@ sanitize:
parse_LD_LIBRARY_PATH(ldpath_env);
parse_LD_PRELOAD(ldpreload_env);
if(soinfo_link_image(si, 0)) {
if(soinfo_link_image(si)) {
char errmsg[] = "CANNOT LINK EXECUTABLE\n";
write(2, __linker_dl_err_buf, strlen(__linker_dl_err_buf));
write(2, errmsg, sizeof(errmsg));
@ -2050,7 +2029,7 @@ extern "C" unsigned __linker_init(unsigned **elfdata) {
linker_so.phnum = elf_hdr->e_phnum;
linker_so.flags |= FLAG_LINKER;
if (soinfo_link_image(&linker_so, 0)) {
if (soinfo_link_image(&linker_so)) {
// It would be nice to print an error message, but if the linker
// can't link itself, there's no guarantee that we'll be able to
// call write() (because it involves a GOT reference).

1
linker/linker.h
View File

@ -176,6 +176,7 @@ struct soinfo
/* When you read a virtual address from the ELF file, add this
* value to get the corresponding address in the process' address space */
Elf32_Addr load_bias;
int has_text_relocations;
};