Use canonical paths when searching for dex files

Apps which use the DexPathClassLoader directly may
pass symlinks when trying to load dex files. This
will not work as we use string comparision to find
the dex in an oat file. The CL fixes this issue by
using using dex conical paths for comparisons.

Bug: 15313272

(cherry picked from commit 4e1d579d6401fef2dd57b16f8d406e33221a69d9)

Change-Id: I441f1ef18388c4a17c747a7e55b57f917724db85

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index c8d1f50..3a186f3 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -678,14 +678,6 @@ const OatFile* ClassLinker::FindOpenedOatFile(const char* oat_location, const ch
   return NULL;
 }
 
-static std::string GetMultiDexClassesDexName(size_t number, const char* dex_location) {
-  if (number == 0) {
-    return dex_location;
-  } else {
-    return StringPrintf("%s" kMultiDexSeparatorString "classes%zu.dex", dex_location, number + 1);
-  }
-}
-
 static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, const char* dex_location,
                                          bool generated,
                                          std::vector<std::string>* error_msgs,
@@ -698,7 +690,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, const char* de
 
   bool success = true;
   for (size_t i = 0; success; ++i) {
-    std::string next_name_str = GetMultiDexClassesDexName(i, dex_location);
+    std::string next_name_str = DexFile::GetMultiDexClassesDexName(i, dex_location);
     const char* next_name = next_name_str.c_str();
 
     uint32_t dex_location_checksum;

diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index e5bc7c8..e1a7771 100644 (file)
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -951,6 +951,38 @@ std::pair<const char*, const char*> DexFile::SplitMultiDexLocation(
   return std::make_pair(tmp, colon_ptr + 1);
 }
 
+std::string DexFile::GetMultiDexClassesDexName(size_t number, const char* dex_location) {
+  if (number == 0) {
+    return dex_location;
+  } else {
+    return StringPrintf("%s" kMultiDexSeparatorString "classes%zu.dex", dex_location, number + 1);
+  }
+}
+
+std::string DexFile::GetDexCanonicalLocation(const char* dex_location) {
+  CHECK_NE(dex_location, static_cast<const char*>(nullptr));
+  char* path = nullptr;
+  if (!IsMultiDexLocation(dex_location)) {
+    path = realpath(dex_location, nullptr);
+  } else {
+    std::pair<const char*, const char*> pair = DexFile::SplitMultiDexLocation(dex_location);
+    const char* dex_real_location(realpath(pair.first, nullptr));
+    delete pair.first;
+    if (dex_real_location != nullptr) {
+      int length = strlen(dex_real_location) + strlen(pair.second) + strlen(kMultiDexSeparatorString) + 1;
+      char* multidex_canonical_location = reinterpret_cast<char*>(malloc(sizeof(char) * length));
+      snprintf(multidex_canonical_location, length, "%s" kMultiDexSeparatorString "%s", dex_real_location, pair.second);
+      free(const_cast<char*>(dex_real_location));
+      path = multidex_canonical_location;
+    }
+  }
+
+  // If realpath fails then we just copy the argument.
+  std::string result(path == nullptr ? dex_location : path);
+  free(path);
+  return result;
+}
+
 std::ostream& operator<<(std::ostream& os, const DexFile& dex_file) {
   os << StringPrintf("[DexFile: %s dex-checksum=%08x location-checksum=%08x %p-%p]",
                      dex_file.GetLocation().c_str(),
@@ -958,6 +990,7 @@ std::ostream& operator<<(std::ostream& os, const DexFile& dex_file) {
                      dex_file.Begin(), dex_file.Begin() + dex_file.Size());
   return os;
 }
+
 std::string Signature::ToString() const {
   if (dex_file_ == nullptr) {
     CHECK(proto_id_ == nullptr);

diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index d64a030..2794af6 100644 (file)
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -841,6 +841,23 @@ class DexFile {
     return size_;
   }
 
+  static std::string GetMultiDexClassesDexName(size_t number, const char* dex_location);
+
+  // Returns the canonical form of the given dex location.
+  //
+  // There are different flavors of "dex locations" as follows:
+  // the file name of a dex file:
+  //     The actual file path that the dex file has on disk.
+  // dex_location:
+  //     This acts as a key for the class linker to know which dex file to load.
+  //     It may correspond to either an old odex file or a particular dex file
+  //     inside an oat file. In the first case it will also match the file name
+  //     of the dex file. In the second case (oat) it will include the file name
+  //     and possibly some multidex annotation to uniquely identify it.
+  // canonical_dex_location:
+  //     the dex_location where it's file name part has been made canonical.
+  static std::string GetDexCanonicalLocation(const char* dex_location);
+
  private:
   // Opens a .dex file
   static const DexFile* OpenFile(int fd, const char* location, bool verify, std::string* error_msg);

diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index c1e00fc..1ae358b 100644 (file)
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -340,4 +340,31 @@ TEST_F(DexFileTest, FindFieldId) {
   }
 }
 
+TEST_F(DexFileTest, GetMultiDexClassesDexName) {
+  std::string dex_location_str = "/system/app/framework.jar";
+  const char* dex_location = dex_location_str.c_str();
+  ASSERT_EQ("/system/app/framework.jar", DexFile::GetMultiDexClassesDexName(0, dex_location));
+  ASSERT_EQ("/system/app/framework.jar:classes2.dex", DexFile::GetMultiDexClassesDexName(1, dex_location));
+  ASSERT_EQ("/system/app/framework.jar:classes101.dex", DexFile::GetMultiDexClassesDexName(100, dex_location));
+}
+
+TEST_F(DexFileTest, GetDexCanonicalLocation) {
+  ScratchFile file;
+  std::string dex_location = file.GetFilename();
+
+  ASSERT_EQ(file.GetFilename(), DexFile::GetDexCanonicalLocation(dex_location.c_str()));
+  std::string multidex_location = DexFile::GetMultiDexClassesDexName(1, dex_location.c_str());
+  ASSERT_EQ(multidex_location, DexFile::GetDexCanonicalLocation(multidex_location.c_str()));
+
+  std::string dex_location_sym = dex_location + "symlink";
+  ASSERT_EQ(0, symlink(dex_location.c_str(), dex_location_sym.c_str()));
+
+  ASSERT_EQ(dex_location, DexFile::GetDexCanonicalLocation(dex_location_sym.c_str()));
+
+  std::string multidex_location_sym = DexFile::GetMultiDexClassesDexName(1, dex_location_sym.c_str());
+  ASSERT_EQ(multidex_location, DexFile::GetDexCanonicalLocation(multidex_location_sym.c_str()));
+
+  ASSERT_EQ(0, unlink(dex_location_sym.c_str()));
+}
+
 }  // namespace art

diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 86c1bae..a61cd81 100644 (file)
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -18,6 +18,7 @@
 
 #include <dlfcn.h>
 #include <sstream>
+#include <string.h>
 
 #include "base/bit_vector.h"
 #include "base/stl_util.h"
@@ -120,6 +121,9 @@ OatFile::OatFile(const std::string& location)
 }
 
 OatFile::~OatFile() {
+  for (auto it : oat_dex_files_) {
+     delete it.first.data();
+  }
   STLDeleteValues(&oat_dex_files_);
   if (dlopen_handle_ != NULL) {
     dlclose(dlopen_handle_);
@@ -300,8 +304,14 @@ bool OatFile::Setup(std::string* error_msg) {
                                               dex_file_checksum,
                                               dex_file_pointer,
                                               methods_offsets_pointer);
-    // Use a StringPiece backed by the oat_dex_file's internal std::string as the key.
-    StringPiece key(oat_dex_file->GetDexFileLocation());
+
+    std::string dex_canonical_location_str = DexFile::GetDexCanonicalLocation(dex_file_location.c_str());
+    // make a copy since we need to persist it as a key in the object's field.
+    int location_size = dex_canonical_location_str.size() + 1;
+    char* dex_canonical_location = new char[location_size ];
+    strncpy(dex_canonical_location, dex_canonical_location_str.c_str(), location_size);
+
+    StringPiece key(dex_canonical_location);
     oat_dex_files_.Put(key, oat_dex_file);
   }
   return true;
@@ -324,7 +334,9 @@ const byte* OatFile::End() const {
 const OatFile::OatDexFile* OatFile::GetOatDexFile(const char* dex_location,
                                                   const uint32_t* dex_location_checksum,
                                                   bool warn_if_not_found) const {
-  Table::const_iterator it = oat_dex_files_.find(dex_location);
+  std::string dex_canonical_location = DexFile::GetDexCanonicalLocation(dex_location);
+
+  Table::const_iterator it = oat_dex_files_.find(dex_canonical_location);
   if (it != oat_dex_files_.end()) {
     const OatFile::OatDexFile* oat_dex_file = it->second;
     if (dex_location_checksum == NULL ||
@@ -339,15 +351,18 @@ const OatFile::OatDexFile* OatFile::GetOatDexFile(const char* dex_location,
       checksum = StringPrintf("0x%08x", *dex_location_checksum);
     }
     LOG(WARNING) << "Failed to find OatDexFile for DexFile " << dex_location
+                 << " ( canonical path " << dex_canonical_location << ")"
                  << " with checksum " << checksum << " in OatFile " << GetLocation();
     if (kIsDebugBuild) {
       for (Table::const_iterator it = oat_dex_files_.begin(); it != oat_dex_files_.end(); ++it) {
         LOG(WARNING) << "OatFile " << GetLocation()
                      << " contains OatDexFile " << it->second->GetDexFileLocation()
+                     << " (canonical path " << it->first << ")"
                      << " with checksum 0x" << std::hex << it->second->GetDexFileLocationChecksum();
       }
     }
   }
+
   return NULL;
 }