From e66190d2a97a713ae4b4786e60ca3d67ab8aa192 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 18 Dec 2012 15:57:55 -0800 Subject: [PATCH] Check for unknown flags passed to dlopen(3). Change-Id: I56f4aab0e5a1487bc32d2c4d231e8bd15c4ac8da --- linker/Android.mk | 18 ++++++++-------- linker/dlfcn.cpp | 4 ++-- linker/linker.cpp | 6 +++++- linker/linker.h | 2 +- tests/Android.mk | 2 +- tests/{dlopen_test.cpp => dlfcn_test.cpp} | 35 ++++++++++++++++++++++++------- 6 files changed, 46 insertions(+), 21 deletions(-) rename tests/{dlopen_test.cpp => dlfcn_test.cpp} (87%) diff --git a/linker/Android.mk b/linker/Android.mk index b757030ad..5fccf9bfa 100644 --- a/linker/Android.mk +++ b/linker/Android.mk @@ -8,21 +8,21 @@ else endif LOCAL_SRC_FILES:= \ - arch/$(TARGET_ARCH)/begin.$(linker_begin_extension) \ - debugger.cpp \ - dlfcn.cpp \ - linker.cpp \ - linker_environ.cpp \ - linker_format.cpp \ - linker_phdr.cpp \ - rt.cpp + arch/$(TARGET_ARCH)/begin.$(linker_begin_extension) \ + debugger.cpp \ + dlfcn.cpp \ + linker.cpp \ + linker_environ.cpp \ + linker_format.cpp \ + linker_phdr.cpp \ + rt.cpp LOCAL_LDFLAGS := -shared LOCAL_CFLAGS += -fno-stack-protector \ -Wstrict-overflow=5 \ -fvisibility=hidden \ - -Wall -Wextra + -Wall -Wextra -Werror # We need to access Bionic private headers in the linker... LOCAL_CFLAGS += -I$(LOCAL_PATH)/../libc/ diff --git a/linker/dlfcn.cpp b/linker/dlfcn.cpp index 24006e2be..2184f3a12 100644 --- a/linker/dlfcn.cpp +++ b/linker/dlfcn.cpp @@ -55,9 +55,9 @@ const char* dlerror() { return old_value; } -void* dlopen(const char* filename, int flag) { +void* dlopen(const char* filename, int flags) { ScopedPthreadMutexLocker locker(&gDlMutex); - soinfo* result = do_dlopen(filename); + soinfo* result = do_dlopen(filename, flags); if (result == NULL) { __bionic_format_dlerror("dlopen failed", linker_get_error()); return NULL; diff --git a/linker/linker.cpp b/linker/linker.cpp index 0d0a8a901..0bdff9975 100755 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -966,7 +966,11 @@ static int soinfo_unload(soinfo* si) { return 0; } -soinfo* do_dlopen(const char* name) { +soinfo* do_dlopen(const char* name, int flags) { + if ((flags & ~(RTLD_NOW|RTLD_LAZY|RTLD_LOCAL|RTLD_GLOBAL)) != 0) { + DL_ERR("invalid flags to dlopen: %x", flags); + return NULL; + } set_soinfo_pool_protection(PROT_READ | PROT_WRITE); soinfo* si = find_library(name); if (si != NULL) { diff --git a/linker/linker.h b/linker/linker.h index 51869e721..60c76fa91 100644 --- a/linker/linker.h +++ b/linker/linker.h @@ -207,7 +207,7 @@ extern soinfo libdl_info; #define DT_PREINIT_ARRAYSZ 33 #endif -soinfo* do_dlopen(const char* name); +soinfo* do_dlopen(const char* name, int flags); int do_dlclose(soinfo* si); Elf32_Sym* lookup(const char* name, soinfo** found, soinfo* start); diff --git a/tests/Android.mk b/tests/Android.mk index abc6f52d1..715338374 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -68,7 +68,7 @@ test_src_files = \ test_dynamic_ldflags = -Wl,--export-dynamic -Wl,-u,DlSymTestFunction test_dynamic_src_files = \ - dlopen_test.cpp \ + dlfcn_test.cpp \ # Build tests for the device (with bionic's .so). Run with: # adb shell /data/nativetest/bionic-unit-tests/bionic-unit-tests diff --git a/tests/dlopen_test.cpp b/tests/dlfcn_test.cpp similarity index 87% rename from tests/dlopen_test.cpp rename to tests/dlfcn_test.cpp index f042503ee..0c40eb728 100644 --- a/tests/dlopen_test.cpp +++ b/tests/dlfcn_test.cpp @@ -32,7 +32,7 @@ extern "C" void DlSymTestFunction() { gCalled = true; } -TEST(dlopen, dlsym_in_self) { +TEST(dlfcn, dlsym_in_self) { dlerror(); // Clear any pending errors. void* self = dlopen(NULL, RTLD_NOW); ASSERT_TRUE(self != NULL); @@ -50,7 +50,7 @@ TEST(dlopen, dlsym_in_self) { ASSERT_EQ(0, dlclose(self)); } -TEST(dlopen, dlopen_failure) { +TEST(dlfcn, dlopen_failure) { void* self = dlopen("/does/not/exist", RTLD_NOW); ASSERT_TRUE(self == NULL); #if __BIONIC__ @@ -65,7 +65,7 @@ static void* ConcurrentDlErrorFn(void*) { return reinterpret_cast(strdup(dlerror())); } -TEST(dlopen, dlerror_concurrent) { +TEST(dlfcn, dlerror_concurrent) { dlopen("/main/thread", RTLD_NOW); const char* main_thread_error = dlerror(); ASSERT_SUBSTR("/main/thread", main_thread_error); @@ -81,7 +81,7 @@ TEST(dlopen, dlerror_concurrent) { ASSERT_SUBSTR("/main/thread", main_thread_error); } -TEST(dlopen, dlsym_failures) { +TEST(dlfcn, dlsym_failures) { dlerror(); // Clear any pending errors. void* self = dlopen(NULL, RTLD_NOW); ASSERT_TRUE(self != NULL); @@ -114,7 +114,7 @@ TEST(dlopen, dlsym_failures) { ASSERT_EQ(0, dlclose(self)); } -TEST(dlopen, dladdr) { +TEST(dlfcn, dladdr) { dlerror(); // Clear any pending errors. void* self = dlopen(NULL, RTLD_NOW); ASSERT_TRUE(self != NULL); @@ -174,7 +174,7 @@ TEST(dlopen, dladdr) { ASSERT_EQ(0, dlclose(self)); } -TEST(dlopen, dladdr_invalid) { +TEST(dlfcn, dladdr_invalid) { Dl_info info; dlerror(); // Clear any pending errors. @@ -190,10 +190,31 @@ TEST(dlopen, dladdr_invalid) { #if __BIONIC__ // Our dynamic linker doesn't support GNU hash tables. -TEST(dlopen, library_with_only_gnu_hash) { +TEST(dlfcn, dlopen_library_with_only_gnu_hash) { dlerror(); // Clear any pending errors. void* handle = dlopen("no-elf-hash-table-library.so", RTLD_NOW); ASSERT_TRUE(handle == NULL); ASSERT_STREQ("dlopen failed: empty/missing DT_HASH in \"no-elf-hash-table-library.so\" (built with --hash-style=gnu?)", dlerror()); } #endif + +TEST(dlfcn, dlopen_bad_flags) { + dlerror(); // Clear any pending errors. + void* handle; + +#ifdef __GLIBC__ + // glibc was smart enough not to define RTLD_NOW as 0, so it can detect missing flags. + handle = dlopen(NULL, 0); + ASSERT_TRUE(handle == NULL); + ASSERT_SUBSTR("invalid", dlerror()); +#endif + + handle = dlopen(NULL, 0xffffffff); + ASSERT_TRUE(handle == NULL); + ASSERT_SUBSTR("invalid", dlerror()); + + // glibc actually allows you to choose both RTLD_NOW and RTLD_LAZY at the same time, and so do we. + handle = dlopen(NULL, RTLD_NOW|RTLD_LAZY); + ASSERT_TRUE(handle != NULL); + ASSERT_SUBSTR(NULL, dlerror()); +} -- 2.11.0