commit 5d10374947824c60d524e812172ae60f30a51fde
author Torne (Richard Coles) <torne@google.com> Thu Apr 11 12:25:06 2019 -0400
committer Torne (Richard Coles) <torne@google.com> Thu Apr 11 13:57:31 2019 -0400
tree d6ae67a2d04a626afdabbe546d01378bd0636eca
parent e6fd53b751c253ce9315875bed838a6d549132f3

Only write main library's RELRO by default.

ANDROID_DLEXT_WRITE_RELRO was inadvertently writing out the RELRO
section of all libraries loaded during a given dlopen() call instead of
only the main library; since the other libraries are loaded at
unpredictable addresses this additional data is rarely useful.

Fix this to only happen when the
ANDROID_DLEXT_RESERVED_ADDRESS_RECURSIVE flag is being used.

Bug: 128623590
Test: DlExtRelroSharingTest.CheckRelroSizes
Change-Id: I05e8651d06ce2de77b8c85fe2b6238f9c09691ad

linker/linker.cpp

diff --git a/linker/linker.cpp b/linker/linker.cpp
index c60ab6a..219ae79 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -1865,11 +1865,17 @@
 
     soinfo_list_t global_group = local_group_ns->get_global_group();
     bool linked = local_group.visit([&](soinfo* si) {
-      // Even though local group may contain accessible soinfos from other namesapces
+      // Even though local group may contain accessible soinfos from other namespaces
       // we should avoid linking them (because if they are not linked -> they
       // are in the local_group_roots and will be linked later).
       if (!si->is_linked() && si->get_primary_namespace() == local_group_ns) {
-        if (!si->link_image(global_group, local_group, extinfo, &relro_fd_offset) ||
+        const android_dlextinfo* link_extinfo = nullptr;
+        if (si == soinfos[0] || reserved_address_recursive) {
+          // Only forward extinfo for the first library unless the recursive
+          // flag is set.
+          link_extinfo = extinfo;
+        }
+        if (!si->link_image(global_group, local_group, link_extinfo, &relro_fd_offset) ||
             !get_cfi_shadow()->AfterLoad(si, solist_get_head())) {
           return false;
         }

tests/dlext_test.cpp

diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 1bc5030..7262fac 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -524,6 +524,26 @@
   tf.fd = extinfo_.relro_fd;
 }
 
+TEST_F(DlExtRelroSharingTest, CheckRelroSizes) {
+  TemporaryFile tf1, tf2;
+  ASSERT_NOERROR(close(tf1.fd));
+  ASSERT_NOERROR(close(tf2.fd));
+
+  ASSERT_NO_FATAL_FAILURE(CreateRelroFile(kLibNameRecursive, tf1.path, false));
+  struct stat no_recursive;
+  ASSERT_NOERROR(fstat(extinfo_.relro_fd, &no_recursive));
+  tf1.fd = extinfo_.relro_fd;
+
+  ASSERT_NO_FATAL_FAILURE(CreateRelroFile(kLibNameRecursive, tf2.path, true));
+  struct stat with_recursive;
+  ASSERT_NOERROR(fstat(extinfo_.relro_fd, &with_recursive));
+  tf2.fd = extinfo_.relro_fd;
+
+  // RELRO file should end up bigger when we use the recursive flag, since it
+  // includes data for more than one library.
+  ASSERT_GT(with_recursive.st_size, no_recursive.st_size);
+}
+
 TEST_F(DlExtRelroSharingTest, ChildWritesNoRelro) {
   TemporaryFile tf; // // Use tf to get an unique filename.
   ASSERT_NOERROR(close(tf.fd));