commit f45b0e9edee72e97106c4b3d393b9bf1582303b3
author Dimitry Ivanov <dimitry@google.com> Fri Jan 15 11:13:35 2016 -0800
committer Dimitry Ivanov <dimitry@google.com> Wed Jan 20 20:57:47 2016 -0800
tree 4058875667899bec12774f4843a0e123aa7c4b6c
parent df2f5fbe6f3d3937c3caf95138e43ee15f6d8497

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

tests/dlext_test.cpp

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);
   }