Check vdex file is valid in VdexFile::Open.
Test: m test-art-host, including new vdex_file_test.
Bug: 34339100
Change-Id: I0fe01760779f3876d6790a5443785a4f7289a717
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 19f0f1c..196d8d4 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1282,13 +1282,10 @@
DCHECK_EQ(input_vdex_fd_, -1);
if (!input_vdex_.empty()) {
std::string error_msg;
- input_vdex_file_.reset(VdexFile::Open(input_vdex_,
- /* writable */ false,
- /* low_4gb */ false,
- &error_msg));
- if (input_vdex_file_ != nullptr && !input_vdex_file_->IsValid()) {
- input_vdex_file_.reset(nullptr);
- }
+ input_vdex_file_ = VdexFile::Open(input_vdex_,
+ /* writable */ false,
+ /* low_4gb */ false,
+ &error_msg);
}
DCHECK_EQ(output_vdex_fd_, -1);
@@ -1330,19 +1327,16 @@
PLOG(WARNING) << "Failed getting length of vdex file";
} else {
std::string error_msg;
- input_vdex_file_.reset(VdexFile::Open(input_vdex_fd_,
- s.st_size,
- "vdex",
- /* writable */ false,
- /* low_4gb */ false,
- &error_msg));
+ input_vdex_file_ = VdexFile::Open(input_vdex_fd_,
+ s.st_size,
+ "vdex",
+ /* writable */ false,
+ /* low_4gb */ false,
+ &error_msg);
// If there's any problem with the passed vdex, just warn and proceed
// without it.
if (input_vdex_file_ == nullptr) {
- PLOG(WARNING) << "Failed opening vdex file " << error_msg;
- } else if (!input_vdex_file_->IsValid()) {
- PLOG(WARNING) << "Existing vdex file is invalid";
- input_vdex_file_.reset(nullptr);
+ PLOG(WARNING) << "Failed opening vdex file: " << error_msg;
}
}
}
diff --git a/runtime/Android.bp b/runtime/Android.bp
index 196c65e..304e157 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -575,6 +575,7 @@
"type_lookup_table_test.cc",
"utf_test.cc",
"utils_test.cc",
+ "vdex_file_test.cc",
"verifier/method_verifier_test.cc",
"verifier/reg_type_test.cc",
"zip_archive_test.cc",
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index d47f1b5..31eb1cc 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -193,7 +193,7 @@
bool writable,
bool low_4gb,
std::string* error_msg) {
- vdex_.reset(VdexFile::Open(vdex_filename, writable, low_4gb, error_msg));
+ vdex_ = VdexFile::Open(vdex_filename, writable, low_4gb, error_msg);
if (vdex_.get() == nullptr) {
*error_msg = StringPrintf("Failed to load vdex file '%s' %s",
vdex_filename.c_str(),
diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc
index dabf8c8..2481c8b 100644
--- a/runtime/vdex_file.cc
+++ b/runtime/vdex_file.cc
@@ -49,10 +49,10 @@
DCHECK(IsVersionValid());
}
-VdexFile* VdexFile::Open(const std::string& vdex_filename,
- bool writable,
- bool low_4gb,
- std::string* error_msg) {
+std::unique_ptr<VdexFile> VdexFile::Open(const std::string& vdex_filename,
+ bool writable,
+ bool low_4gb,
+ std::string* error_msg) {
if (!OS::FileExists(vdex_filename.c_str())) {
*error_msg = "File " + vdex_filename + " does not exist.";
return nullptr;
@@ -79,12 +79,12 @@
return Open(vdex_file->Fd(), vdex_length, vdex_filename, writable, low_4gb, error_msg);
}
-VdexFile* VdexFile::Open(int file_fd,
- size_t vdex_length,
- const std::string& vdex_filename,
- bool writable,
- bool low_4gb,
- std::string* error_msg) {
+std::unique_ptr<VdexFile> VdexFile::Open(int file_fd,
+ size_t vdex_length,
+ const std::string& vdex_filename,
+ bool writable,
+ bool low_4gb,
+ std::string* error_msg) {
std::unique_ptr<MemMap> mmap(MemMap::MapFile(vdex_length,
writable ? PROT_READ | PROT_WRITE : PROT_READ,
MAP_SHARED,
@@ -98,8 +98,14 @@
return nullptr;
}
+ std::unique_ptr<VdexFile> vdex(new VdexFile(mmap.release()));
+ if (!vdex->IsValid()) {
+ *error_msg = "Vdex file is not valid";
+ return nullptr;
+ }
+
*error_msg = "Success";
- return new VdexFile(mmap.release());
+ return vdex;
}
const uint8_t* VdexFile::GetNextDexFileData(const uint8_t* cursor) const {
diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h
index 330b955..7daf2f8 100644
--- a/runtime/vdex_file.h
+++ b/runtime/vdex_file.h
@@ -73,17 +73,19 @@
typedef uint32_t VdexChecksum;
- static VdexFile* Open(const std::string& vdex_filename,
- bool writable,
- bool low_4gb,
- std::string* error_msg);
+ // Returns nullptr if the vdex file cannot be opened or is not valid.
+ static std::unique_ptr<VdexFile> Open(const std::string& vdex_filename,
+ bool writable,
+ bool low_4gb,
+ std::string* error_msg);
- static VdexFile* Open(int file_fd,
- size_t vdex_length,
- const std::string& vdex_filename,
- bool writable,
- bool low_4gb,
- std::string* error_msg);
+ // Returns nullptr if the vdex file cannot be opened or is not valid.
+ static std::unique_ptr<VdexFile> Open(int file_fd,
+ size_t vdex_length,
+ const std::string& vdex_filename,
+ bool writable,
+ bool low_4gb,
+ std::string* error_msg);
const uint8_t* Begin() const { return mmap_->Begin(); }
const uint8_t* End() const { return mmap_->End(); }
diff --git a/runtime/vdex_file_test.cc b/runtime/vdex_file_test.cc
new file mode 100644
index 0000000..909e117
--- /dev/null
+++ b/runtime/vdex_file_test.cc
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "vdex_file.h"
+
+#include <string>
+
+#include <gtest/gtest.h>
+
+#include "common_runtime_test.h"
+
+namespace art {
+
+class VdexFileTest : public CommonRuntimeTest {
+};
+
+TEST_F(VdexFileTest, OpenEmptyVdex) {
+ // Verify we fail to open an empty vdex file.
+ ScratchFile tmp;
+ std::string error_msg;
+ std::unique_ptr<VdexFile> vdex = VdexFile::Open(tmp.GetFd(),
+ 0,
+ tmp.GetFilename(),
+ /*writable*/false,
+ /*low_4gb*/false,
+ &error_msg);
+ EXPECT_TRUE(vdex == nullptr);
+
+ vdex = VdexFile::Open(tmp.GetFilename(), /*writable*/false, /*low_4gb*/false, &error_msg);
+ EXPECT_TRUE(vdex == nullptr);
+}
+
+} // namespace art