Use file magic to determine file type, not file extension.
Bug: 10614658
Change-Id: I9156dfca78ac8cd1c62fb258825cc791629270a4
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 800fab0..5728bf6 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -794,7 +794,7 @@
const DexFile* ClassLinker::FindOrCreateOatFileForDexLocationLocked(const std::string& dex_location,
const std::string& oat_location) {
uint32_t dex_location_checksum;
- if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+ if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
LOG(ERROR) << "Failed to compute checksum '" << dex_location << "'";
return NULL;
}
@@ -915,7 +915,7 @@
UniquePtr<const OatFile> oat_file(FindOatFileFromOatLocationLocked(odex_filename));
if (oat_file.get() != NULL) {
uint32_t dex_location_checksum;
- if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+ if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
// If no classes.dex found in dex_location, it has been stripped, assume oat is up-to-date.
// This is the common case in user builds for jar's and apk's in the /system directory.
const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_location);
@@ -936,7 +936,7 @@
oat_file.reset(FindOatFileFromOatLocationLocked(cache_location));
if (oat_file.get() != NULL) {
uint32_t dex_location_checksum;
- if (!DexFile::GetChecksum(dex_location, dex_location_checksum)) {
+ if (!DexFile::GetChecksum(dex_location, &dex_location_checksum)) {
LOG(WARNING) << "Failed to compute checksum: " << dex_location;
return NULL;
}
@@ -1932,7 +1932,8 @@
void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file, SirtRef<mirror::DexCache>& dex_cache) {
dex_lock_.AssertExclusiveHeld(Thread::Current());
CHECK(dex_cache.get() != NULL) << dex_file.GetLocation();
- CHECK(dex_cache->GetLocation()->Equals(dex_file.GetLocation()));
+ CHECK(dex_cache->GetLocation()->Equals(dex_file.GetLocation()))
+ << dex_cache->GetLocation()->ToModifiedUtf8() << " " << dex_file.GetLocation();
dex_caches_.push_back(dex_cache.get());
dex_cache->SetDexFile(&dex_file);
dex_caches_dirty_ = true;
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index e81c456..9034628 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -62,9 +62,34 @@
reinterpret_cast<const DexFile::ClassDef*>(NULL));
}
-bool DexFile::GetChecksum(const std::string& filename, uint32_t& checksum) {
- if (IsValidZipFilename(filename)) {
- UniquePtr<ZipArchive> zip_archive(ZipArchive::Open(filename));
+int OpenAndReadMagic(const std::string& filename, uint32_t* magic) {
+ CHECK(magic != NULL);
+ int fd = open(filename.c_str(), O_RDONLY, 0);
+ if (fd == -1) {
+ PLOG(WARNING) << "Unable to open '" << filename << "'";
+ return -1;
+ }
+ int n = TEMP_FAILURE_RETRY(read(fd, magic, sizeof(*magic)));
+ if (n != sizeof(*magic)) {
+ PLOG(ERROR) << "Failed to find magic in '" << filename << "'";
+ return -1;
+ }
+ if (lseek(fd, 0, SEEK_SET) != 0) {
+ PLOG(ERROR) << "Failed to seek to beginning of file '" << filename << "'";
+ return -1;
+ }
+ return fd;
+}
+
+bool DexFile::GetChecksum(const std::string& filename, uint32_t* checksum) {
+ CHECK(checksum != NULL);
+ uint32_t magic;
+ int fd = OpenAndReadMagic(filename, &magic);
+ if (fd == -1) {
+ return false;
+ }
+ if (IsZipMagic(magic)) {
+ UniquePtr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd));
if (zip_archive.get() == NULL) {
return false;
}
@@ -73,30 +98,36 @@
LOG(ERROR) << "Zip archive '" << filename << "' doesn't contain " << kClassesDex;
return false;
}
- checksum = zip_entry->GetCrc32();
+ *checksum = zip_entry->GetCrc32();
return true;
}
- if (IsValidDexFilename(filename)) {
- UniquePtr<const DexFile> dex_file(DexFile::OpenFile(filename, filename, false));
+ if (IsDexMagic(magic)) {
+ UniquePtr<const DexFile> dex_file(DexFile::OpenFile(fd, filename, false));
if (dex_file.get() == NULL) {
return false;
}
- checksum = dex_file->GetHeader().checksum_;
+ *checksum = dex_file->GetHeader().checksum_;
return true;
}
- LOG(ERROR) << "Expected valid zip or dex file name: " << filename;
+ LOG(ERROR) << "Expected valid zip or dex file: " << filename;
return false;
}
const DexFile* DexFile::Open(const std::string& filename,
const std::string& location) {
- if (IsValidZipFilename(filename)) {
- return DexFile::OpenZip(filename, location);
+ uint32_t magic;
+ int fd = OpenAndReadMagic(filename, &magic);
+ if (fd == -1) {
+ return NULL;
}
- if (!IsValidDexFilename(filename)) {
- LOG(WARNING) << "Attempting to open dex file with unknown extension '" << filename << "'";
+ if (IsZipMagic(magic)) {
+ return DexFile::OpenZip(fd, location);
}
- return DexFile::OpenFile(filename, location, true);
+ if (IsDexMagic(magic)) {
+ return DexFile::OpenFile(fd, location, true);
+ }
+ LOG(ERROR) << "Expected valid zip or dex file: " << filename;
+ return NULL;
}
int DexFile::GetPermissions() const {
@@ -129,37 +160,32 @@
}
}
-const DexFile* DexFile::OpenFile(const std::string& filename,
+const DexFile* DexFile::OpenFile(int fd,
const std::string& location,
bool verify) {
- CHECK(!location.empty()) << filename;
- int fd = open(filename.c_str(), O_RDONLY); // TODO: scoped_fd
- if (fd == -1) {
- PLOG(ERROR) << "open(\"" << filename << "\", O_RDONLY) failed";
- return NULL;
- }
+ CHECK(!location.empty());
struct stat sbuf;
memset(&sbuf, 0, sizeof(sbuf));
if (fstat(fd, &sbuf) == -1) {
- PLOG(ERROR) << "fstat \"" << filename << "\" failed";
+ PLOG(ERROR) << "fstat \"" << location << "\" failed";
close(fd);
return NULL;
}
if (S_ISDIR(sbuf.st_mode)) {
- LOG(ERROR) << "attempt to mmap directory \"" << filename << "\"";
+ LOG(ERROR) << "attempt to mmap directory \"" << location << "\"";
return NULL;
}
size_t length = sbuf.st_size;
UniquePtr<MemMap> map(MemMap::MapFile(length, PROT_READ, MAP_PRIVATE, fd, 0));
if (map.get() == NULL) {
- LOG(ERROR) << "mmap \"" << filename << "\" failed";
+ LOG(ERROR) << "mmap \"" << location << "\" failed";
close(fd);
return NULL;
}
close(fd);
if (map->Size() < sizeof(DexFile::Header)) {
- LOG(ERROR) << "Failed to open dex file '" << filename << "' that is too short to have a header";
+ LOG(ERROR) << "Failed to open dex file '" << location << "' that is too short to have a header";
return NULL;
}
@@ -167,12 +193,12 @@
const DexFile* dex_file = OpenMemory(location, dex_header->checksum_, map.release());
if (dex_file == NULL) {
- LOG(ERROR) << "Failed to open dex file '" << filename << "' from memory";
+ LOG(ERROR) << "Failed to open dex file '" << location << "' from memory";
return NULL;
}
if (verify && !DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size())) {
- LOG(ERROR) << "Failed to verify dex file '" << filename << "'";
+ LOG(ERROR) << "Failed to verify dex file '" << location << "'";
return NULL;
}
@@ -181,11 +207,10 @@
const char* DexFile::kClassesDex = "classes.dex";
-const DexFile* DexFile::OpenZip(const std::string& filename,
- const std::string& location) {
- UniquePtr<ZipArchive> zip_archive(ZipArchive::Open(filename));
+const DexFile* DexFile::OpenZip(int fd, const std::string& location) {
+ UniquePtr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd));
if (zip_archive.get() == NULL) {
- LOG(ERROR) << "Failed to open " << filename << " when looking for classes.dex";
+ LOG(ERROR) << "Failed to open " << location << " when looking for classes.dex";
return NULL;
}
return DexFile::Open(*zip_archive.get(), location);
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 7be5cb8..346154c 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -346,7 +346,7 @@
// For .dex files, this is the header checksum.
// For zip files, this is the classes.dex zip entry CRC32 checksum.
// Return true if the checksum could be found, false otherwise.
- static bool GetChecksum(const std::string& filename, uint32_t& checksum)
+ static bool GetChecksum(const std::string& filename, uint32_t* checksum)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Opens .dex file, guessing the container format based on file extension
@@ -815,13 +815,12 @@
private:
// Opens a .dex file
- static const DexFile* OpenFile(const std::string& filename,
+ static const DexFile* OpenFile(int fd,
const std::string& location,
bool verify);
// Opens a dex file from within a .jar, .zip, or .apk file
- static const DexFile* OpenZip(const std::string& filename,
- const std::string& location);
+ static const DexFile* OpenZip(int fd, const std::string& location);
// Opens a .dex file at the given address backed by a MemMap
static const DexFile* OpenMemory(const std::string& location,
diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index 32a8354..272c42d 100644
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -120,7 +120,7 @@
TEST_F(DexFileTest, GetChecksum) {
uint32_t checksum;
ScopedObjectAccess soa(Thread::Current());
- EXPECT_TRUE(DexFile::GetChecksum(GetLibCoreDexFileName(), checksum));
+ EXPECT_TRUE(DexFile::GetChecksum(GetLibCoreDexFileName(), &checksum));
EXPECT_EQ(java_lang_dex_file_->GetLocationChecksum(), checksum);
}
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index e2e5bf7..1cd33ee 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -276,7 +276,7 @@
for (const OatFile::OatDexFile* oat_dex_file : oat_file_->GetOatDexFiles()) {
const std::string& dex_file_location = oat_dex_file->GetDexFileLocation();
uint32_t dex_file_location_checksum;
- if (!DexFile::GetChecksum(dex_file_location.c_str(), dex_file_location_checksum)) {
+ if (!DexFile::GetChecksum(dex_file_location.c_str(), &dex_file_location_checksum)) {
LOG(WARNING) << "ValidateOatFile could not find checksum for " << dex_file_location;
return false;
}
diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index d2a6c0e..4c60b8f 100644
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -228,7 +228,7 @@
} else {
uint32_t location_checksum;
// If we have no classes.dex checksum such as in a user build, assume up-to-date.
- if (!DexFile::GetChecksum(filename.c_str(), location_checksum)) {
+ if (!DexFile::GetChecksum(filename.c_str(), &location_checksum)) {
if (debug_logging) {
LOG(INFO) << "DexFile_isDexOptNeeded ignoring precompiled stripped file: "
<< filename.c_str();
@@ -278,7 +278,7 @@
ScopedObjectAccess soa(env);
uint32_t location_checksum;
- if (!DexFile::GetChecksum(filename.c_str(), location_checksum)) {
+ if (!DexFile::GetChecksum(filename.c_str(), &location_checksum)) {
LOG(ERROR) << "DexFile_isDexOptNeeded failed to compute checksum of " << filename.c_str();
return JNI_TRUE;
}
diff --git a/runtime/oat.h b/runtime/oat.h
index a5c6bed..a653cf8 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -27,6 +27,9 @@
class PACKED(4) OatHeader {
public:
+ static const uint8_t kOatMagic[4];
+ static const uint8_t kOatVersion[4];
+
OatHeader();
OatHeader(InstructionSet instruction_set,
const std::vector<const DexFile*>* dex_files,
@@ -78,9 +81,6 @@
std::string GetImageFileLocation() const;
private:
- static const uint8_t kOatMagic[4];
- static const uint8_t kOatVersion[4];
-
uint8_t magic_[4];
uint8_t version_[4];
uint32_t adler32_checksum_;
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 4c97017..abd1287 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -33,19 +33,18 @@
namespace art {
std::string OatFile::DexFilenameToOdexFilename(const std::string& location) {
- CHECK(IsValidDexFilename(location) || IsValidZipFilename(location));
+ CHECK_GE(location.size(), 4U) << location; // must be at least .123
+ size_t dot_index = location.size() - 3 - 1; // 3=dex or zip or apk
+ CHECK_EQ('.', location[dot_index]) << location;
std::string odex_location(location);
- odex_location.resize(odex_location.size() - 3); // 3=dex or zip or apk
- CHECK_EQ('.', odex_location[odex_location.size()-1]);
+ odex_location.resize(dot_index + 1);
+ CHECK_EQ('.', odex_location[odex_location.size()-1]) << location << " " << odex_location;
odex_location += "odex";
return odex_location;
}
void OatFile::CheckLocation(const std::string& location) {
CHECK(!location.empty());
- if (!IsValidOatFilename(location)) {
- LOG(WARNING) << "Attempting to open oat file with unknown extension '" << location << "'";
- }
}
OatFile* OatFile::OpenMemory(std::vector<uint8_t>& oat_contents,
diff --git a/runtime/utils.cc b/runtime/utils.cc
index dcfe8a7..8e810a7 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -1196,7 +1196,7 @@
LOG(FATAL) << "Expected path in location to be absolute: "<< location;
}
std::string cache_file(location, 1); // skip leading slash
- if (!IsValidDexFilename(location) && !IsValidImageFilename(location)) {
+ if (!EndsWith(location, ".dex") || !EndsWith(location, ".art")) {
cache_file += "/";
cache_file += DexFile::kClassesDex;
}
@@ -1204,27 +1204,19 @@
return dalvik_cache + "/" + cache_file;
}
-bool IsValidZipFilename(const std::string& filename) {
- if (filename.size() < 4) {
- return false;
- }
- std::string suffix(filename.substr(filename.size() - 4));
- return (suffix == ".zip" || suffix == ".jar" || suffix == ".apk");
+bool IsZipMagic(uint32_t magic) {
+ return (('P' == ((magic >> 0) & 0xff)) &&
+ ('K' == ((magic >> 8) & 0xff)));
}
-bool IsValidDexFilename(const std::string& filename) {
- return EndsWith(filename, ".dex");
+bool IsDexMagic(uint32_t magic) {
+ return DexFile::IsMagicValid(reinterpret_cast<const byte*>(&magic));
}
-bool IsValidImageFilename(const std::string& filename) {
- return EndsWith(filename, ".art");
-}
-
-bool IsValidOatFilename(const std::string& filename) {
- return (EndsWith(filename, ".odex") ||
- EndsWith(filename, ".dex") ||
- EndsWith(filename, ".oat") ||
- EndsWith(filename, DexFile::kClassesDex));
+bool IsOatMagic(uint32_t magic) {
+ return (memcmp(reinterpret_cast<const byte*>(magic),
+ OatHeader::kOatMagic,
+ sizeof(OatHeader::kOatMagic)) == 0);
}
} // namespace art
diff --git a/runtime/utils.h b/runtime/utils.h
index c506fba..812a581 100644
--- a/runtime/utils.h
+++ b/runtime/utils.h
@@ -353,11 +353,10 @@
// Returns the dalvik-cache location for a DexFile or OatFile, or dies trying.
std::string GetDalvikCacheFilenameOrDie(const std::string& location);
-// Check whether the given filename has a valid extension
-bool IsValidZipFilename(const std::string& filename);
-bool IsValidDexFilename(const std::string& filename);
-bool IsValidImageFilename(const std::string& filename);
-bool IsValidOatFilename(const std::string& filename);
+// Check whether the given magic matches a known file type.
+bool IsZipMagic(uint32_t magic);
+bool IsDexMagic(uint32_t magic);
+bool IsOatMagic(uint32_t magic);
class VoidFunctor {
public:
diff --git a/runtime/zip_archive.cc b/runtime/zip_archive.cc
index c3167e5..8e09e78 100644
--- a/runtime/zip_archive.cc
+++ b/runtime/zip_archive.cc
@@ -336,11 +336,11 @@
PLOG(WARNING) << "Unable to open '" << filename << "'";
return NULL;
}
- SetCloseOnExec(fd);
return OpenFromFd(fd);
}
ZipArchive* ZipArchive::OpenFromFd(int fd) {
+ SetCloseOnExec(fd);
UniquePtr<ZipArchive> zip_archive(new ZipArchive(fd));
if (zip_archive.get() == NULL) {
return NULL;