Do not unmap reserved region on dlclose
dlclose used to unmap the part of the reserved region
for ANDROID_DLEXT_RESERVED_ADDRESS that was neccessary
to map PT_LOAD segments. With this change dlclose
replaces mapped PT_LOAD segments with a PROT_NONE,
MAP_ANONYMOUS | MAP_NORESERVE.
Previously caller was unmapping the reserved region after
the failed dlclose which led to race condition when someone
else reused the region freed by dlclose but before the unmap
by the chromium code.
Bug: http://code.google.com/p/chromium/issues/detail?id=568880
Change-Id: I0f5eaa2bf6641f83dde469b631c518482acc59a2
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 70c2ca5..1c1650e 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -356,7 +356,13 @@
}
if (si->base != 0 && si->size != 0) {
- munmap(reinterpret_cast<void*>(si->base), si->size);
+ if (!si->is_mapped_by_caller()) {
+ munmap(reinterpret_cast<void*>(si->base), si->size);
+ } else {
+ // remap the region as PROT_NONE, MAP_ANONYMOUS | MAP_NORESERVE
+ mmap(reinterpret_cast<void*>(si->base), si->size, PROT_NONE,
+ MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
+ }
}
soinfo *prev = nullptr, *trav;
@@ -1160,6 +1166,7 @@
si_->base = elf_reader.load_start();
si_->size = elf_reader.load_size();
+ si_->set_mapped_by_caller(elf_reader.is_mapped_by_caller());
si_->load_bias = elf_reader.load_bias();
si_->phnum = elf_reader.phdr_count();
si_->phdr = elf_reader.loaded_phdr();
@@ -3248,6 +3255,19 @@
return local_group_root_;
}
+
+void soinfo::set_mapped_by_caller(bool mapped_by_caller) {
+ if (mapped_by_caller) {
+ flags_ |= FLAG_MAPPED_BY_CALLER;
+ } else {
+ flags_ &= ~FLAG_MAPPED_BY_CALLER;
+ }
+}
+
+bool soinfo::is_mapped_by_caller() const {
+ return (flags_ & FLAG_MAPPED_BY_CALLER) != 0;
+}
+
// This function returns api-level at the time of
// dlopen/load. Note that libraries opened by system
// will always have 'current' api level.
diff --git a/linker/linker.h b/linker/linker.h
index 5a06853..389c5b3 100644
--- a/linker/linker.h
+++ b/linker/linker.h
@@ -79,11 +79,13 @@
#define ELF64_R_TYPE(info) (((info) >> 56) & 0xff)
#endif
-#define FLAG_LINKED 0x00000001
-#define FLAG_EXE 0x00000004 // The main executable
-#define FLAG_LINKER 0x00000010 // The linker itself
-#define FLAG_GNU_HASH 0x00000040 // uses gnu hash
-#define FLAG_NEW_SOINFO 0x40000000 // new soinfo format
+#define FLAG_LINKED 0x00000001
+#define FLAG_EXE 0x00000004 // The main executable
+#define FLAG_LINKER 0x00000010 // The linker itself
+#define FLAG_GNU_HASH 0x00000040 // uses gnu hash
+#define FLAG_MAPPED_BY_CALLER 0x00000080 // the map is reserved by the caller
+ // and should not be unmapped
+#define FLAG_NEW_SOINFO 0x40000000 // new soinfo format
#define SUPPORTED_DT_FLAGS_1 (DF_1_NOW | DF_1_GLOBAL | DF_1_NODELETE)
@@ -337,6 +339,9 @@
const std::vector<std::string>& get_dt_runpath() const;
android_namespace_t* get_namespace();
+ void set_mapped_by_caller(bool reserved_map);
+ bool is_mapped_by_caller() const;
+
private:
bool elf_lookup(SymbolName& symbol_name, const version_info* vi, uint32_t* symbol_index) const;
ElfW(Sym)* elf_addr_lookup(const void* addr);
diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index 4c4ce17..e81e325 100644
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -137,7 +137,8 @@
ElfReader::ElfReader()
: did_read_(false), did_load_(false), fd_(-1), file_offset_(0), file_size_(0), phdr_num_(0),
phdr_table_(nullptr), shdr_table_(nullptr), shdr_num_(0), dynamic_(nullptr), strtab_(nullptr),
- strtab_size_(0), load_start_(nullptr), load_size_(0), load_bias_(0), loaded_phdr_(nullptr) {
+ strtab_size_(0), load_start_(nullptr), load_size_(0), load_bias_(0), loaded_phdr_(nullptr),
+ mapped_by_caller_(false) {
}
bool ElfReader::Read(const char* name, int fd, off64_t file_offset, off64_t file_size) {
@@ -467,6 +468,7 @@
}
} else {
start = extinfo->reserved_addr;
+ mapped_by_caller_ = true;
}
load_start_ = start;
diff --git a/linker/linker_phdr.h b/linker/linker_phdr.h
index c359cca..89ec094 100644
--- a/linker/linker_phdr.h
+++ b/linker/linker_phdr.h
@@ -53,6 +53,7 @@
const ElfW(Phdr)* loaded_phdr() const { return loaded_phdr_; }
const ElfW(Dyn)* dynamic() const { return dynamic_; }
const char* get_string(ElfW(Word) index) const;
+ bool is_mapped_by_caller() const { return mapped_by_caller_; }
private:
bool ReadElfHeader();
@@ -99,6 +100,9 @@
// Loaded phdr.
const ElfW(Phdr)* loaded_phdr_;
+
+ // Is map owned by the caller
+ bool mapped_by_caller_;
};
size_t phdr_table_get_load_size(const ElfW(Phdr)* phdr_table, size_t phdr_count,
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 261aa55..d28e9cc 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -275,7 +275,8 @@
ASSERT_TRUE(fn != nullptr);
EXPECT_EQ(4, fn());
- uint32_t* taxicab_number = reinterpret_cast<uint32_t*>(dlsym(handle, "dlopen_testlib_taxicab_number"));
+ uint32_t* taxicab_number =
+ reinterpret_cast<uint32_t*>(dlsym(handle, "dlopen_testlib_taxicab_number"));
ASSERT_DL_NOTNULL(taxicab_number);
EXPECT_EQ(1729U, *taxicab_number);
@@ -284,8 +285,7 @@
TEST_F(DlExtTest, Reserved) {
- void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
android_dlextinfo extinfo;
extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS;
@@ -299,11 +299,17 @@
EXPECT_LT(reinterpret_cast<void*>(f),
reinterpret_cast<char*>(start) + LIBSIZE);
EXPECT_EQ(4, f());
+
+ // Check that after dlclose reserved address space is unmapped (and can be reused)
+ dlclose(handle_);
+ handle_ = nullptr;
+
+ void* new_start = mmap(start, PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ ASSERT_NE(start, new_start) << "dlclose unmapped reserved space";
}
TEST_F(DlExtTest, ReservedTooSmall) {
- void* start = mmap(nullptr, PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
android_dlextinfo extinfo;
extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS;
@@ -314,8 +320,7 @@
}
TEST_F(DlExtTest, ReservedHint) {
- void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
android_dlextinfo extinfo;
extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS_HINT;
@@ -332,8 +337,7 @@
}
TEST_F(DlExtTest, ReservedHintTooSmall) {
- void* start = mmap(nullptr, PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, PAGE_SIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
android_dlextinfo extinfo;
extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS_HINT;
@@ -350,8 +354,7 @@
}
TEST_F(DlExtTest, LoadAtFixedAddress) {
- void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
munmap(start, LIBSIZE);
@@ -367,6 +370,12 @@
EXPECT_LT(reinterpret_cast<void*>(f), reinterpret_cast<char*>(start) + LIBSIZE);
EXPECT_EQ(4, f());
+ dlclose(handle_);
+ handle_ = nullptr;
+
+ // Check that dlclose unmapped the file
+ void* addr = mmap(start, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ ASSERT_EQ(start, addr) << "dlclose did not unmap the memory";
}
TEST_F(DlExtTest, LoadAtFixedAddressTooSmall) {
@@ -390,8 +399,7 @@
protected:
virtual void SetUp() {
DlExtTest::SetUp();
- void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
- -1, 0);
+ void* start = mmap(nullptr, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
ASSERT_TRUE(start != MAP_FAILED);
extinfo_.flags = ANDROID_DLEXT_RESERVED_ADDRESS;
extinfo_.reserved_addr = start;
@@ -442,7 +450,8 @@
ASSERT_DL_NOTNULL(f);
EXPECT_EQ(4, f());
- uint32_t* taxicab_number = reinterpret_cast<uint32_t*>(dlsym(handle_, "dlopen_testlib_taxicab_number"));
+ uint32_t* taxicab_number =
+ reinterpret_cast<uint32_t*>(dlsym(handle_, "dlopen_testlib_taxicab_number"));
ASSERT_DL_NOTNULL(taxicab_number);
EXPECT_EQ(1729U, *taxicab_number);
}