summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--dexlayout/dex_ir.cc13
-rw-r--r--dexlayout/dexlayout_test.cc77
-rw-r--r--runtime/dex_instruction.cc20
-rw-r--r--runtime/dex_instruction.h9
-rw-r--r--runtime/dex_instruction_iterator.h147
-rw-r--r--runtime/verifier/method_verifier.cc20
6 files changed, 232 insertions, 54 deletions
diff --git a/dexlayout/dex_ir.cc b/dexlayout/dex_ir.cc
index 8c4ee6e9a1..23c3a5ca93 100644
--- a/dexlayout/dex_ir.cc
+++ b/dexlayout/dex_ir.cc
@@ -167,10 +167,17 @@ static bool GetIdsFromByteCode(Collections& collections,
std::vector<MethodId*>* method_ids,
std::vector<FieldId*>* field_ids) {
bool has_id = false;
- for (const Instruction& instruction : code->Instructions()) {
- CHECK_GT(instruction.SizeInCodeUnits(), 0u);
+ IterationRange<DexInstructionIterator> instructions = code->Instructions();
+ SafeDexInstructionIterator it(instructions.begin(), instructions.end());
+ for (; !it.IsErrorState() && it < instructions.end(); ++it) {
+ // In case the instruction goes past the end of the code item, make sure to not process it.
+ SafeDexInstructionIterator next = it;
+ ++next;
+ if (next.IsErrorState() || next > instructions.end()) {
+ break;
+ }
has_id |= GetIdFromInstruction(collections,
- &instruction,
+ it.Inst(),
type_ids,
string_ids,
method_ids,
diff --git a/dexlayout/dexlayout_test.cc b/dexlayout/dexlayout_test.cc
index f34e7ecd4b..cdd0219ef3 100644
--- a/dexlayout/dexlayout_test.cc
+++ b/dexlayout/dexlayout_test.cc
@@ -317,6 +317,30 @@ class DexLayoutTest : public CommonRuntimeTest {
return true;
}
+ template <typename Mutator>
+ bool MutateDexFile(File* output_dex, const std::string& input_jar, const Mutator& mutator) {
+ std::vector<std::unique_ptr<const DexFile>> dex_files;
+ std::string error_msg;
+ CHECK(DexFileLoader::Open(input_jar.c_str(),
+ input_jar.c_str(),
+ /*verify*/ true,
+ /*verify_checksum*/ true,
+ &error_msg,
+ &dex_files)) << error_msg;
+ EXPECT_EQ(dex_files.size(), 1u) << "Only one input dex is supported";
+ for (const std::unique_ptr<const DexFile>& dex : dex_files) {
+ CHECK(dex->EnableWrite()) << "Failed to enable write";
+ mutator(const_cast<DexFile*>(dex.get()));
+ if (!output_dex->WriteFully(dex->Begin(), dex->Size())) {
+ return false;
+ }
+ }
+ if (output_dex->Flush() != 0) {
+ PLOG(FATAL) << "Could not flush the output file.";
+ }
+ return true;
+ }
+
// Create a profile with some subset of methods and classes.
void CreateProfile(const std::string& input_dex,
const std::string& out_profile,
@@ -518,8 +542,10 @@ class DexLayoutTest : public CommonRuntimeTest {
const char* dex_filename,
ScratchFile* profile_file,
std::vector<std::string>& dexlayout_exec_argv) {
- WriteBase64ToFile(dex_filename, dex_file->GetFile());
- EXPECT_EQ(dex_file->GetFile()->Flush(), 0);
+ if (dex_filename != nullptr) {
+ WriteBase64ToFile(dex_filename, dex_file->GetFile());
+ EXPECT_EQ(dex_file->GetFile()->Flush(), 0);
+ }
if (profile_file != nullptr) {
CreateProfile(dex_file->GetFilename(), profile_file->GetFilename(), dex_file->GetFilename());
}
@@ -673,4 +699,51 @@ TEST_F(DexLayoutTest, DuplicateCodeItem) {
dexlayout_exec_argv));
}
+// Test that instructions that go past the end of the code items don't cause crashes.
+TEST_F(DexLayoutTest, CodeItemOverrun) {
+ ScratchFile temp_dex;
+ MutateDexFile(temp_dex.GetFile(), GetTestDexFileName("ManyMethods"), [] (DexFile* dex) {
+ bool mutated_successfully = false;
+ // Change the dex instructions to make an opcode that spans past the end of the code item.
+ for (size_t i = 0; i < dex->NumClassDefs(); ++i) {
+ const DexFile::ClassDef& def = dex->GetClassDef(i);
+ const uint8_t* data = dex->GetClassData(def);
+ if (data == nullptr) {
+ continue;
+ }
+ ClassDataItemIterator it(*dex, data);
+ it.SkipAllFields();
+ while (it.HasNextDirectMethod() || it.HasNextVirtualMethod()) {
+ DexFile::CodeItem* item = const_cast<DexFile::CodeItem*>(it.GetMethodCodeItem());
+ if (item != nullptr) {
+ IterationRange<DexInstructionIterator> instructions = item->Instructions();
+ if (instructions.begin() != instructions.end()) {
+ DexInstructionIterator last_instruction = instructions.begin();
+ for (auto dex_it = instructions.begin(); dex_it != instructions.end(); ++dex_it) {
+ last_instruction = dex_it;
+ }
+ if (last_instruction->SizeInCodeUnits() == 1) {
+ // Set the opcode to something that will go past the end of the code item.
+ const_cast<Instruction*>(last_instruction.Inst())->SetOpcode(
+ Instruction::CONST_STRING_JUMBO);
+ mutated_successfully = true;
+ }
+ }
+ }
+ it.Next();
+ }
+ }
+ CHECK(mutated_successfully)
+ << "Failed to find candidate code item with only one code unit in last instruction.";
+ });
+ std::string dexlayout = GetTestAndroidRoot() + "/bin/dexlayout";
+ EXPECT_TRUE(OS::FileExists(dexlayout.c_str())) << dexlayout << " should be a valid file path";
+ std::vector<std::string> dexlayout_exec_argv =
+ { dexlayout, "-i", "-o", "/dev/null", temp_dex.GetFilename() };
+ ASSERT_TRUE(DexLayoutExec(&temp_dex,
+ /*dex_filename*/ nullptr,
+ nullptr /* profile_file */,
+ dexlayout_exec_argv));
+}
+
} // namespace art
diff --git a/runtime/dex_instruction.cc b/runtime/dex_instruction.cc
index 99fe53bb71..e64c0f62c2 100644
--- a/runtime/dex_instruction.cc
+++ b/runtime/dex_instruction.cc
@@ -119,6 +119,26 @@ size_t Instruction::SizeInCodeUnitsComplexOpcode() const {
}
}
+size_t Instruction::CodeUnitsRequiredForSizeOfComplexOpcode() const {
+ const uint16_t* insns = reinterpret_cast<const uint16_t*>(this);
+ // Handle special NOP encoded variable length sequences.
+ switch (*insns) {
+ case kPackedSwitchSignature:
+ FALLTHROUGH_INTENDED;
+ case kSparseSwitchSignature:
+ return 2;
+ case kArrayDataSignature:
+ return 4;
+ default:
+ if ((*insns & 0xFF) == 0) {
+ return 1; // NOP.
+ } else {
+ LOG(FATAL) << "Unreachable: " << DumpString(nullptr);
+ UNREACHABLE();
+ }
+ }
+}
+
std::string Instruction::DumpHex(size_t code_units) const {
size_t inst_length = SizeInCodeUnits();
if (inst_length > code_units) {
diff --git a/runtime/dex_instruction.h b/runtime/dex_instruction.h
index 2f28dffa2b..09c78b2428 100644
--- a/runtime/dex_instruction.h
+++ b/runtime/dex_instruction.h
@@ -225,6 +225,12 @@ class Instruction {
}
}
+ // Code units required to calculate the size of the instruction.
+ size_t CodeUnitsRequiredForSizeComputation() const {
+ const int8_t result = kInstructionDescriptors[Opcode()].size_in_code_units;
+ return UNLIKELY(result < 0) ? CodeUnitsRequiredForSizeOfComplexOpcode() : 1;
+ }
+
// Reads an instruction out of the stream at the specified address.
static const Instruction* At(const uint16_t* code) {
DCHECK(code != nullptr);
@@ -638,6 +644,9 @@ class Instruction {
private:
size_t SizeInCodeUnitsComplexOpcode() const;
+ // Return how many code unit words are required to compute the size of the opcode.
+ size_t CodeUnitsRequiredForSizeOfComplexOpcode() const;
+
uint32_t Fetch32(size_t offset) const {
return (Fetch16(offset) | ((uint32_t) Fetch16(offset + 1) << 16));
}
diff --git a/runtime/dex_instruction_iterator.h b/runtime/dex_instruction_iterator.h
index 280746e9dc..afa8d4aad3 100644
--- a/runtime/dex_instruction_iterator.h
+++ b/runtime/dex_instruction_iterator.h
@@ -24,19 +24,68 @@
namespace art {
-class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, Instruction> {
+// Base helper class to prevent duplicated comparators.
+class DexInstructionIteratorBase : public std::iterator<std::forward_iterator_tag, Instruction> {
public:
using value_type = std::iterator<std::forward_iterator_tag, Instruction>::value_type;
using difference_type = std::iterator<std::forward_iterator_tag, value_type>::difference_type;
- DexInstructionIterator() = default;
- DexInstructionIterator(const DexInstructionIterator&) = default;
- DexInstructionIterator(DexInstructionIterator&&) = default;
- DexInstructionIterator& operator=(const DexInstructionIterator&) = default;
- DexInstructionIterator& operator=(DexInstructionIterator&&) = default;
+ DexInstructionIteratorBase() = default;
+ explicit DexInstructionIteratorBase(const value_type* inst) : inst_(inst) {}
- explicit DexInstructionIterator(const value_type* inst) : inst_(inst) {}
- explicit DexInstructionIterator(const uint16_t* inst) : inst_(value_type::At(inst)) {}
+ const value_type* Inst() const {
+ return inst_;
+ }
+
+ // Return the dex pc for an iterator compared to the code item begin.
+ uint32_t GetDexPC(const DexInstructionIteratorBase& code_item_begin) {
+ return reinterpret_cast<const uint16_t*>(inst_) -
+ reinterpret_cast<const uint16_t*>(code_item_begin.inst_);
+ }
+
+ protected:
+ const value_type* inst_ = nullptr;
+};
+
+
+static ALWAYS_INLINE inline bool operator==(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return lhs.Inst() == rhs.Inst();
+}
+
+static inline bool operator!=(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return !(lhs == rhs);
+}
+
+static inline bool operator<(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return lhs.Inst() < rhs.Inst();
+}
+
+static inline bool operator>(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return rhs < lhs;
+}
+
+static inline bool operator<=(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return !(rhs < lhs);
+}
+
+static inline bool operator>=(const DexInstructionIteratorBase& lhs,
+ const DexInstructionIteratorBase& rhs) {
+ return !(lhs < rhs);
+}
+
+class DexInstructionIterator : public DexInstructionIteratorBase {
+ public:
+ using value_type = std::iterator<std::forward_iterator_tag, Instruction>::value_type;
+ using difference_type = std::iterator<std::forward_iterator_tag, value_type>::difference_type;
+ using DexInstructionIteratorBase::DexInstructionIteratorBase;
+
+ explicit DexInstructionIterator(const uint16_t* inst)
+ : DexInstructionIteratorBase(value_type::At(inst)) {}
// Value after modification.
DexInstructionIterator& operator++() {
@@ -58,50 +107,62 @@ class DexInstructionIterator : public std::iterator<std::forward_iterator_tag, I
const value_type* operator->() const {
return &**this;
}
+};
- // Return the dex pc for an iterator compared to the code item begin.
- uint32_t GetDexPC(const DexInstructionIterator& code_item_begin) {
- return reinterpret_cast<const uint16_t*>(inst_) -
- reinterpret_cast<const uint16_t*>(code_item_begin.inst_);
+class SafeDexInstructionIterator : public DexInstructionIteratorBase {
+ public:
+ explicit SafeDexInstructionIterator(const DexInstructionIteratorBase& start,
+ const DexInstructionIteratorBase& end)
+ : DexInstructionIteratorBase(start.Inst())
+ , end_(end.Inst()) {}
+
+ // Value after modification, does not read past the end of the allowed region. May increment past
+ // the end of the code item though.
+ SafeDexInstructionIterator& operator++() {
+ AssertValid();
+ const size_t size_code_units = Inst()->CodeUnitsRequiredForSizeComputation();
+ const size_t available = reinterpret_cast<const uint16_t*>(end_) -
+ reinterpret_cast<const uint16_t*>(Inst());
+ if (size_code_units > available) {
+ error_state_ = true;
+ } else {
+ inst_ = inst_->Next();
+ }
+ return *this;
}
- const value_type* Inst() const {
- return inst_;
+ // Value before modification.
+ SafeDexInstructionIterator operator++(int) {
+ SafeDexInstructionIterator temp = *this;
+ ++*this;
+ return temp;
}
- private:
- const value_type* inst_ = nullptr;
-};
-
-static ALWAYS_INLINE inline bool operator==(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return lhs.Inst() == rhs.Inst();
-}
-
-static inline bool operator!=(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return !(lhs == rhs);
-}
+ const value_type& operator*() const {
+ AssertValid();
+ return *inst_;
+ }
-static inline bool operator<(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return lhs.Inst() < rhs.Inst();
-}
+ const value_type* operator->() const {
+ AssertValid();
+ return &**this;
+ }
-static inline bool operator>(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return rhs < lhs;
-}
+ // Returns true if the iterator is in an error state. This occurs when an instruction couldn't
+ // have its size computed without reading past the end iterator.
+ bool IsErrorState() const {
+ return error_state_;
+ }
-static inline bool operator<=(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return !(rhs < lhs);
-}
+ private:
+ ALWAYS_INLINE void AssertValid() const {
+ DCHECK(!IsErrorState());
+ DCHECK_LT(Inst(), end_);
+ }
-static inline bool operator>=(const DexInstructionIterator& lhs,
- const DexInstructionIterator& rhs) {
- return !(lhs < rhs);
-}
+ const value_type* end_ = nullptr;
+ bool error_state_ = false;
+};
} // namespace art
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 0033167160..6555e14358 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -987,9 +987,17 @@ bool MethodVerifier::ComputeWidthsAndCountOps() {
size_t monitor_enter_count = 0;
IterationRange<DexInstructionIterator> instructions = code_item_->Instructions();
- DexInstructionIterator inst = instructions.begin();
- for ( ; inst < instructions.end(); ++inst) {
- Instruction::Code opcode = inst->Opcode();
+ // We can't assume the instruction is well formed, handle the case where calculating the size
+ // goes past the end of the code item.
+ SafeDexInstructionIterator it(instructions.begin(), instructions.end());
+ for ( ; !it.IsErrorState() && it < instructions.end(); ++it) {
+ // In case the instruction goes past the end of the code item, make sure to not process it.
+ SafeDexInstructionIterator next = it;
+ ++next;
+ if (next.IsErrorState() || next > instructions.end()) {
+ break;
+ }
+ Instruction::Code opcode = it->Opcode();
switch (opcode) {
case Instruction::APUT_OBJECT:
case Instruction::CHECK_CAST:
@@ -1010,13 +1018,13 @@ bool MethodVerifier::ComputeWidthsAndCountOps() {
default:
break;
}
- GetInstructionFlags(inst.GetDexPC(instructions.begin())).SetIsOpcode();
+ GetInstructionFlags(it.GetDexPC(instructions.begin())).SetIsOpcode();
}
- if (inst != instructions.end()) {
+ if (it != instructions.end()) {
const size_t insns_size = code_item_->insns_size_in_code_units_;
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "code did not end where expected ("
- << inst.GetDexPC(instructions.begin()) << " vs. "
+ << it.GetDexPC(instructions.begin()) << " vs. "
<< insns_size << ")";
return false;
}
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-12-31 03:22:21 +0000