Handle empty relro segment or incorrectly sized file.

If the file has no relro segment, the generated relro file will have
length 0, which caused mmap to fail. If the relro file has nonzero size,
but is too short (e.g. because it's for the wrong version of the
library), the linker would segfault while comparing the data. Fix both
these issues: don't try to map a zero length file, and don't try to
compare data that would be beyond the end of the file.

Improve test to explicitly generate two versions of the library: one
with -z relro, and one with -z norelro, so we can test both cases; also
explicitly test the case where the relro file has length 0.

Bug: 14299541
Change-Id: Id8b95585edda90e8bb5de452a35b70ed2d224934

diff --git a/linker/linker_phdr.cpp b/linker/linker_phdr.cpp
index cfeab96..12e9779 100644 (file)
--- a/linker/linker_phdr.cpp
+++ b/linker/linker_phdr.cpp
@@ -591,9 +591,12 @@ int phdr_table_map_gnu_relro(const ElfW(Phdr)* phdr_table, size_t phdr_count, El
     return -1;
   }
   off_t file_size = file_stat.st_size;
-  void* temp_mapping = mmap(NULL, file_size, PROT_READ, MAP_PRIVATE, fd, 0);
-  if (temp_mapping == MAP_FAILED) {
-    return -1;
+  void* temp_mapping = NULL;
+  if (file_size > 0) {
+    temp_mapping = mmap(NULL, file_size, PROT_READ, MAP_PRIVATE, fd, 0);
+    if (temp_mapping == MAP_FAILED) {
+      return -1;
+    }
   }
   size_t file_offset = 0;
 
@@ -614,6 +617,13 @@ int phdr_table_map_gnu_relro(const ElfW(Phdr)* phdr_table, size_t phdr_count, El
     size_t match_offset = 0;
     size_t size = seg_page_end - seg_page_start;
 
+    if (file_size - file_offset < size) {
+      // File is too short to compare to this segment. The contents are likely
+      // different as well (it's probably for a different library version) so
+      // just don't bother checking.
+      break;
+    }
+
     while (match_offset < size) {
       // Skip over dissimilar pages.
       while (match_offset < size &&

diff --git a/tests/Android.mk b/tests/Android.mk
index 0014af6..c6fa54e 100644 (file)
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -190,17 +190,32 @@ include $(LOCAL_PATH)/Android.build.mk
 endif
 
 # -----------------------------------------------------------------------------
-# Library used by dlext tests.
+# Library used by dlext tests - with/without GNU RELRO program header
 # -----------------------------------------------------------------------------
 libdlext_test_src_files := \
     dlext_test_library.cpp \
 
+libdlext_test_ldflags := \
+    -Wl,-z,relro \
+
 module := libdlext_test
 module_tag := optional
 build_type := target
 build_target := SHARED_LIBRARY
 include $(LOCAL_PATH)/Android.build.mk
 
+libdlext_test_norelro_src_files := \
+    dlext_test_library.cpp \
+
+libdlext_test_norelro_ldflags := \
+    -Wl,-z,norelro \
+
+module := libdlext_test_norelro
+module_tag := optional
+build_type := target
+build_target := SHARED_LIBRARY
+include $(LOCAL_PATH)/Android.build.mk
+
 # -----------------------------------------------------------------------------
 # Tests for the device using bionic's .so. Run with:
 #   adb shell /data/nativetest/bionic-unit-tests/bionic-unit-tests

diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 872cc5b..4b2a5e2 100644 (file)
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -39,6 +39,7 @@
 
 typedef int (*fn)(void);
 #define LIBNAME "libdlext_test.so"
+#define LIBNAME_NORELRO "libdlext_test_norelro.so"
 #define LIBSIZE 1024*1024 // how much address space to reserve for it
 
 
@@ -144,52 +145,88 @@ TEST_F(DlExtTest, ReservedHintTooSmall) {
   EXPECT_EQ(4, f());
 }
 
-TEST_F(DlExtTest, RelroShareChildWrites) {
-  void* start = mmap(NULL, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
-                     -1, 0);
-  ASSERT_TRUE(start != MAP_FAILED);
-  android_dlextinfo extinfo;
-  extinfo.reserved_addr = start;
-  extinfo.reserved_size = LIBSIZE;
+class DlExtRelroSharingTest : public DlExtTest {
+protected:
+  virtual void SetUp() {
+    DlExtTest::SetUp();
+    void* start = mmap(NULL, LIBSIZE, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
+                       -1, 0);
+    ASSERT_TRUE(start != MAP_FAILED);
+    extinfo_.flags = ANDROID_DLEXT_RESERVED_ADDRESS;
+    extinfo_.reserved_addr = start;
+    extinfo_.reserved_size = LIBSIZE;
+    extinfo_.relro_fd = -1;
+
+    const char* android_data = getenv("ANDROID_DATA");
+    ASSERT_TRUE(android_data != NULL);
+    snprintf(relro_file_, sizeof(relro_file_), "%s/local/tmp/libdlext_test.relro", android_data);
+  }
 
-  int relro_fd;
-  char relro_file[PATH_MAX];
-  const char* android_data = getenv("ANDROID_DATA");
-  ASSERT_TRUE(android_data != NULL);
-  snprintf(relro_file, sizeof(relro_file), "%s/local/tmp/libdlext_test.relro", android_data);
-  relro_fd = open(relro_file, O_CREAT | O_RDWR | O_TRUNC, 0644);
-  extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS | ANDROID_DLEXT_WRITE_RELRO;
-  ASSERT_NOERROR(relro_fd);
-  extinfo.relro_fd = relro_fd;
-
-  pid_t pid = fork();
-  if (pid == 0) {
-    // child process
-    void* handle = android_dlopen_ext(LIBNAME, RTLD_NOW, &extinfo);
-    if (handle == NULL) {
-      fprintf(stderr, "in child: %s\n", dlerror());
-      exit(1);
+  virtual void TearDown() {
+    DlExtTest::TearDown();
+    if (extinfo_.relro_fd != -1) {
+      ASSERT_NOERROR(close(extinfo_.relro_fd));
     }
-    exit(0);
   }
 
-  // continuing in parent
-  ASSERT_NOERROR(close(relro_fd));
-  ASSERT_NOERROR(pid);
-  int status;
-  ASSERT_EQ(pid, waitpid(pid, &status, 0));
-  ASSERT_TRUE(WIFEXITED(status));
-  ASSERT_EQ(0, WEXITSTATUS(status));
+  void CreateRelroFile(const char* lib) {
+    int relro_fd = open(relro_file_, O_CREAT | O_RDWR | O_TRUNC, 0644);
+    ASSERT_NOERROR(relro_fd);
+
+    pid_t pid = fork();
+    if (pid == 0) {
+      // child process
+      extinfo_.flags |= ANDROID_DLEXT_WRITE_RELRO;
+      extinfo_.relro_fd = relro_fd;
+      void* handle = android_dlopen_ext(lib, RTLD_NOW, &extinfo_);
+      if (handle == NULL) {
+        fprintf(stderr, "in child: %s\n", dlerror());
+        exit(1);
+      }
+      exit(0);
+    }
+
+    // continuing in parent
+    ASSERT_NOERROR(close(relro_fd));
+    ASSERT_NOERROR(pid);
+    int status;
+    ASSERT_EQ(pid, waitpid(pid, &status, 0));
+    ASSERT_TRUE(WIFEXITED(status));
+    ASSERT_EQ(0, WEXITSTATUS(status));
+
+    // reopen file for reading so it can be used
+    relro_fd = open(relro_file_, O_RDONLY);
+    ASSERT_NOERROR(relro_fd);
+    extinfo_.flags |= ANDROID_DLEXT_USE_RELRO;
+    extinfo_.relro_fd = relro_fd;
+  }
+
+  void TryUsingRelro(const char* lib) {
+    handle_ = android_dlopen_ext(lib, RTLD_NOW, &extinfo_);
+    ASSERT_DL_NOTNULL(handle_);
+    fn f = reinterpret_cast<fn>(dlsym(handle_, "getRandomNumber"));
+    ASSERT_DL_NOTNULL(f);
+    EXPECT_EQ(4, f());
+  }
+
+  android_dlextinfo extinfo_;
+  char relro_file_[PATH_MAX];
+};
+
+TEST_F(DlExtRelroSharingTest, ChildWritesGoodData) {
+  ASSERT_NO_FATAL_FAILURE(CreateRelroFile(LIBNAME));
+  ASSERT_NO_FATAL_FAILURE(TryUsingRelro(LIBNAME));
+}
 
-  relro_fd = open(relro_file, O_RDONLY);
+TEST_F(DlExtRelroSharingTest, ChildWritesNoRelro) {
+  ASSERT_NO_FATAL_FAILURE(CreateRelroFile(LIBNAME_NORELRO));
+  ASSERT_NO_FATAL_FAILURE(TryUsingRelro(LIBNAME_NORELRO));
+}
+
+TEST_F(DlExtRelroSharingTest, RelroFileEmpty) {
+  int relro_fd = open(relro_file_, O_CREAT | O_RDWR | O_TRUNC, 0644);
   ASSERT_NOERROR(relro_fd);
-  extinfo.flags = ANDROID_DLEXT_RESERVED_ADDRESS | ANDROID_DLEXT_USE_RELRO;
-  extinfo.relro_fd = relro_fd;
-  handle_ = android_dlopen_ext(LIBNAME, RTLD_NOW, &extinfo);
   ASSERT_NOERROR(close(relro_fd));
 
-  ASSERT_DL_NOTNULL(handle_);
-  fn f = reinterpret_cast<fn>(dlsym(handle_, "getRandomNumber"));
-  ASSERT_DL_NOTNULL(f);
-  EXPECT_EQ(4, f());
+  ASSERT_NO_FATAL_FAILURE(TryUsingRelro(LIBNAME));
 }