Merge "Reject .so files with no sysv hash table."
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 9e4138e..479d4b9 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -81,7 +81,7 @@
*/
-static int soinfo_link_image(soinfo *si);
+static bool soinfo_link_image(soinfo* si);
static int socount = 0;
static soinfo sopool[SO_MAX];
@@ -90,11 +90,17 @@
static soinfo *sonext = &libdl_info;
static soinfo *somain; /* main process, always the one after libdl_info */
-static char ldpaths_buf[LDPATH_BUFSIZE];
-static const char *ldpaths[LDPATH_MAX + 1];
+static const char* const gSoPaths[] = {
+ "/vendor/lib",
+ "/system/lib",
+ NULL
+};
-static char ldpreloads_buf[LDPRELOAD_BUFSIZE];
-static const char *ldpreload_names[LDPRELOAD_MAX + 1];
+static char gLdPathsBuffer[LDPATH_BUFSIZE];
+static const char* gLdPaths[LDPATH_MAX + 1];
+
+static char gLdPreloadsBuffer[LDPRELOAD_BUFSIZE];
+static const char* gLdPreloadNames[LDPRELOAD_MAX + 1];
static soinfo *preloads[LDPRELOAD_MAX + 1];
@@ -580,56 +586,36 @@
}
#endif
-static const char * const sopaths[] = {
- "/vendor/lib",
- "/system/lib",
- 0
-};
-
-static int _open_lib(const char* name) {
- // TODO: why not just call open?
- struct stat sb;
- if (stat(name, &sb) == -1 || !S_ISREG(sb.st_mode)) {
- return -1;
+static int open_library_on_path(const char* name, const char* const paths[]) {
+ char buf[512];
+ for (size_t i = 0; paths[i] != NULL; ++i) {
+ int n = format_buffer(buf, sizeof(buf), "%s/%s", paths[i], name);
+ if (n < 0 || n >= static_cast<int>(sizeof(buf))) {
+ WARN("Ignoring very long library path: %s/%s\n", paths[i], name);
+ continue;
}
- return TEMP_FAILURE_RETRY(open(name, O_RDONLY));
+ int fd = TEMP_FAILURE_RETRY(open(buf, O_RDONLY | O_CLOEXEC));
+ if (fd != -1) {
+ return fd;
+ }
+ }
+ return -1;
}
-static int open_library(const char *name)
-{
- int fd;
- char buf[512];
- const char * const*path;
- int n;
+static int open_library(const char* name) {
+ TRACE("[ %5d opening %s ]\n", pid, name);
- TRACE("[ %5d opening %s ]\n", pid, name);
+ // If the name contains a slash, we should attempt to open it directly and not search the paths.
+ if (strchr(name, '/') != NULL) {
+ return TEMP_FAILURE_RETRY(open(name, O_RDONLY | O_CLOEXEC));
+ }
- if(name == 0) return -1;
- if(strlen(name) > 256) return -1;
-
- if ((name[0] == '/') && ((fd = _open_lib(name)) >= 0))
- return fd;
-
- for (path = ldpaths; *path; path++) {
- n = format_buffer(buf, sizeof(buf), "%s/%s", *path, name);
- if (n < 0 || n >= (int)sizeof(buf)) {
- WARN("Ignoring very long library path: %s/%s\n", *path, name);
- continue;
- }
- if ((fd = _open_lib(buf)) >= 0)
- return fd;
- }
- for (path = sopaths; *path; path++) {
- n = format_buffer(buf, sizeof(buf), "%s/%s", *path, name);
- if (n < 0 || n >= (int)sizeof(buf)) {
- WARN("Ignoring very long library path: %s/%s\n", *path, name);
- continue;
- }
- if ((fd = _open_lib(buf)) >= 0)
- return fd;
- }
-
- return -1;
+ // Otherwise we try LD_LIBRARY_PATH first, and fall back to the built-in well known paths.
+ int fd = open_library_on_path(name, gLdPaths);
+ if (fd == -1) {
+ fd = open_library_on_path(name, gSoPaths);
+ }
+ return fd;
}
// Returns 'true' if the library is prelinked or on failure so we error out
@@ -727,8 +713,7 @@
Elf32_Addr phdr_size;
};
-static soinfo* load_library(const char* name)
-{
+static soinfo* load_library(const char* name) {
// Open the file.
scoped_fd fd;
fd.fd = open_library(name);
@@ -835,7 +820,7 @@
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)) {
+ if (!soinfo_link_image(si)) {
munmap((void *)si->base, si->size);
return NULL;
}
@@ -882,7 +867,7 @@
TRACE("[ %5d '%s' has not been loaded yet. Locating...]\n", pid, name);
si = load_library(name);
- if(si == NULL)
+ if (si == NULL)
return NULL;
return init_library(si);
}
@@ -1427,16 +1412,15 @@
return return_value;
}
-static int soinfo_link_image(soinfo *si)
-{
- unsigned *d;
+static bool soinfo_link_image(soinfo* si) {
+ si->flags |= FLAG_ERROR;
+
/* "base" might wrap around UINT32_MAX. */
Elf32_Addr base = si->load_bias;
const Elf32_Phdr *phdr = si->phdr;
int phnum = si->phnum;
int relocating_linker = (si->flags & FLAG_LINKER) != 0;
soinfo **needed, **pneeded;
- size_t dynamic_count;
/* We can't debug anything until the linker is relocated */
if (!relocating_linker) {
@@ -1446,13 +1430,14 @@
}
/* Extract dynamic section */
+ size_t dynamic_count;
phdr_table_get_dynamic_section(phdr, phnum, base, &si->dynamic,
&dynamic_count);
if (si->dynamic == NULL) {
if (!relocating_linker) {
- DL_ERR("missing PT_DYNAMIC?!");
+ DL_ERR("missing PT_DYNAMIC in \"%s\"", si->name);
}
- goto fail;
+ return false;
} else {
if (!relocating_linker) {
DEBUG("%5d dynamic = %p\n", pid, si->dynamic);
@@ -1465,7 +1450,7 @@
#endif
/* extract useful information from dynamic section */
- for(d = si->dynamic; *d; d++){
+ for (unsigned* d = si->dynamic; *d; ++d) {
DEBUG("%5d d = %p, d[0] = 0x%08x d[1] = 0x%08x\n", pid, d, d[0], d[1]);
switch(*d++){
case DT_HASH:
@@ -1482,8 +1467,8 @@
break;
case DT_PLTREL:
if(*d != DT_REL) {
- DL_ERR("DT_RELA not supported");
- goto fail;
+ DL_ERR("unsupported DT_RELA in \"%s\"", si->name);
+ return false;
}
break;
case DT_JMPREL:
@@ -1509,8 +1494,8 @@
#endif
break;
case DT_RELA:
- DL_ERR("DT_RELA not supported");
- goto fail;
+ DL_ERR("unsupported DT_RELA in \"%s\"", si->name);
+ return false;
case DT_INIT:
si->init_func = (void (*)(void))(base + *d);
DEBUG("%5d %s constructors (init func) found at %p\n",
@@ -1611,22 +1596,31 @@
DEBUG("%5d si->base = 0x%08x, si->strtab = %p, si->symtab = %p\n",
pid, si->base, si->strtab, si->symtab);
- if((si->strtab == 0) || (si->symtab == 0)) {
- DL_ERR("missing essential tables");
- goto fail;
+ // Sanity checks.
+ if (si->nbucket == 0) {
+ DL_ERR("empty/missing DT_HASH in \"%s\" (built with --hash-style=gnu?)", si->name);
+ return false;
+ }
+ if (si->strtab == 0) {
+ DL_ERR("empty/missing DT_STRTAB in \"%s\"", si->name);
+ return false;
+ }
+ if (si->symtab == 0) {
+ DL_ERR("empty/missing DT_SYMTAB in \"%s\"", si->name);
+ return false;
}
/* if this is the main executable, then load all of the preloads now */
if(si->flags & FLAG_EXE) {
int i;
memset(preloads, 0, sizeof(preloads));
- for(i = 0; ldpreload_names[i] != NULL; i++) {
- soinfo *lsi = find_library(ldpreload_names[i]);
+ for(i = 0; gLdPreloadNames[i] != NULL; i++) {
+ soinfo *lsi = find_library(gLdPreloadNames[i]);
if(lsi == 0) {
strlcpy(tmp_err_buf, linker_get_error(), sizeof(tmp_err_buf));
DL_ERR("could not load library \"%s\" needed by \"%s\"; caused by %s",
- ldpreload_names[i], si->name, tmp_err_buf);
- goto fail;
+ gLdPreloadNames[i], si->name, tmp_err_buf);
+ return false;
}
lsi->refcount++;
preloads[i] = lsi;
@@ -1636,7 +1630,7 @@
/* dynamic_count is an upper bound for the number of needed libs */
pneeded = needed = (soinfo**) alloca((1 + dynamic_count) * sizeof(soinfo*));
- for(d = si->dynamic; *d; d += 2) {
+ for (unsigned* d = si->dynamic; *d; d += 2) {
if(d[0] == DT_NEEDED){
DEBUG("%5d %s needs %s\n", pid, si->name, si->strtab + d[1]);
soinfo *lsi = find_library(si->strtab + d[1]);
@@ -1644,7 +1638,7 @@
strlcpy(tmp_err_buf, linker_get_error(), sizeof(tmp_err_buf));
DL_ERR("could not load library \"%s\" needed by \"%s\"; caused by %s",
si->strtab + d[1], si->name, tmp_err_buf);
- goto fail;
+ return false;
}
*pneeded++ = lsi;
lsi->refcount++;
@@ -1661,24 +1655,26 @@
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;
+ return false;
}
}
- if(si->plt_rel) {
+ if (si->plt_rel) {
DEBUG("[ %5d relocating %s plt ]\n", pid, si->name );
- if(soinfo_relocate(si, si->plt_rel, si->plt_rel_count, needed))
- goto fail;
+ if(soinfo_relocate(si, si->plt_rel, si->plt_rel_count, needed)) {
+ return false;
+ }
}
- if(si->rel) {
+ if (si->rel) {
DEBUG("[ %5d relocating %s ]\n", pid, si->name );
- if(soinfo_relocate(si, si->rel, si->rel_count, needed))
- goto fail;
+ if(soinfo_relocate(si, si->rel, si->rel_count, needed)) {
+ return false;
+ }
}
#ifdef ANDROID_MIPS_LINKER
- if(mips_relocate_got(si, needed)) {
- goto fail;
+ if (mips_relocate_got(si, needed)) {
+ return false;
}
#endif
@@ -1691,7 +1687,7 @@
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;
+ return false;
}
}
@@ -1699,25 +1695,17 @@
if (phdr_table_protect_gnu_relro(si->phdr, si->phnum, si->load_bias) < 0) {
DL_ERR("can't enable GNU RELRO protection for \"%s\": %s",
si->name, strerror(errno));
- goto fail;
+ return false;
}
- /* If this is a SET?ID program, dup /dev/null to opened stdin,
- stdout and stderr to close a security hole described in:
-
- ftp://ftp.freebsd.org/pub/FreeBSD/CERT/advisories/FreeBSD-SA-02:23.stdio.asc
-
- */
+ // If this is a setuid/setgid program, close the security hole described in
+ // ftp://ftp.freebsd.org/pub/FreeBSD/CERT/advisories/FreeBSD-SA-02:23.stdio.asc
if (get_AT_SECURE()) {
nullify_closed_stdio();
}
notify_gdb_of_load(si);
- return 0;
-
-fail:
- ERROR("failed to link %s\n", si->name);
- si->flags |= FLAG_ERROR;
- return -1;
+ si->flags &= ~FLAG_ERROR;
+ return true;
}
static void parse_path(const char* path, const char* delimiters,
@@ -1747,14 +1735,14 @@
}
static void parse_LD_LIBRARY_PATH(const char* path) {
- parse_path(path, ":", ldpaths,
- ldpaths_buf, sizeof(ldpaths_buf), LDPATH_MAX);
+ parse_path(path, ":", gLdPaths,
+ gLdPathsBuffer, sizeof(gLdPathsBuffer), LDPATH_MAX);
}
static void parse_LD_PRELOAD(const char* path) {
// We have historically supported ':' as well as ' ' in LD_PRELOAD.
- parse_path(path, " :", ldpreload_names,
- ldpreloads_buf, sizeof(ldpreloads_buf), LDPRELOAD_MAX);
+ parse_path(path, " :", gLdPreloadNames,
+ gLdPreloadsBuffer, sizeof(gLdPreloadsBuffer), LDPRELOAD_MAX);
}
/*
@@ -1894,7 +1882,7 @@
somain = si;
- if(soinfo_link_image(si)) {
+ 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));
@@ -2033,7 +2021,7 @@
linker_so.phnum = elf_hdr->e_phnum;
linker_so.flags |= FLAG_LINKER;
- if (soinfo_link_image(&linker_so)) {
+ 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).
diff --git a/tests/Android.mk b/tests/Android.mk
index 68dee10..9d5cd36 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -62,6 +62,21 @@
LOCAL_STATIC_LIBRARIES += libstlport_static libstdc++ libm libc
include $(BUILD_NATIVE_TEST)
+
+
+
+# Build no-elf-hash-table-library.so to test dlopen(3) on a library that
+# only has a GNU-style hash table.
+include $(CLEAR_VARS)
+LOCAL_MODULE := no-elf-hash-table-library
+LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_SRC_FILES := empty.cpp
+LOCAL_LDFLAGS := -Wl,--hash-style=gnu
+include $(BUILD_SHARED_LIBRARY)
+
+
+
+
# Build for the host (with glibc).
# Note that this will build against glibc, so it's not useful for testing
# bionic's implementation, but it does let you use glibc as a reference
diff --git a/tests/dlopen_test.cpp b/tests/dlopen_test.cpp
index d38d8c5..024df01 100644
--- a/tests/dlopen_test.cpp
+++ b/tests/dlopen_test.cpp
@@ -181,3 +181,13 @@
ASSERT_EQ(dladdr(&info, &info), 0); // Zero on error, non-zero on success.
ASSERT_TRUE(dlerror() == NULL); // dladdr(3) doesn't set dlerror(3).
}
+
+#if __BIONIC__
+// Our dynamic linker doesn't support GNU hash tables.
+TEST(dlopen, library_with_only_gnu_hash) {
+ dlerror(); // Clear any pending errors.
+ void* handle = dlopen("empty-library.so", RTLD_NOW);
+ ASSERT_TRUE(handle == NULL);
+ ASSERT_STREQ("dlopen failed: empty/missing DT_HASH in \"empty-library.so\" (built with --hash-style=gnu?)", dlerror());
+}
+#endif
diff --git a/tests/empty.cpp b/tests/empty.cpp
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/empty.cpp
