From 66d1caf42c20efba8305efb3a819993126e8abbf Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Tue, 15 Jul 2014 23:56:47 +0100 Subject: [PATCH] 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 --- runtime/class_linker.cc | 10 +--------- runtime/dex_file.cc | 33 +++++++++++++++++++++++++++++++++ runtime/dex_file.h | 17 +++++++++++++++++ runtime/dex_file_test.cc | 27 +++++++++++++++++++++++++++ runtime/oat_file.cc | 21 ++++++++++++++++++--- 5 files changed, 96 insertions(+), 12 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index c8d1f5093..3a186f3e6 100644 --- 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* 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 e5bc7c8c8..e1a77714b 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -951,6 +951,38 @@ std::pair 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(nullptr)); + char* path = nullptr; + if (!IsMultiDexLocation(dex_location)) { + path = realpath(dex_location, nullptr); + } else { + std::pair 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(malloc(sizeof(char) * length)); + snprintf(multidex_canonical_location, length, "%s" kMultiDexSeparatorString "%s", dex_real_location, pair.second); + free(const_cast(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 d64a03005..2794af646 100644 --- 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 c1e00fcb0..1ae358bfe 100644 --- 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 86c1baef2..a61cd8149 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -18,6 +18,7 @@ #include #include +#include #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; } -- 2.11.0