Add overflow checks in Memory objects.
Also change one of the reads to be explicitly ReadField instead of an
overloaded Read function.
Bug: 23762183
Test: Passes new unit tests.
Change-Id: Id848f7b632f67df0c5b7318d9e588942cfd2099a
diff --git a/libunwindstack/ElfInterface.cpp b/libunwindstack/ElfInterface.cpp
index d59e9d8..087457c 100644
--- a/libunwindstack/ElfInterface.cpp
+++ b/libunwindstack/ElfInterface.cpp
@@ -42,7 +42,7 @@
uint64_t offset = ehdr.e_phoff;
for (size_t i = 0; i < ehdr.e_phnum; i++, offset += ehdr.e_phentsize) {
PhdrType phdr;
- if (!memory_->Read(offset, &phdr, &phdr.p_type, sizeof(phdr.p_type))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_type, sizeof(phdr.p_type))) {
return false;
}
@@ -54,20 +54,20 @@
case PT_LOAD:
{
// Get the flags first, if this isn't an executable header, ignore it.
- if (!memory_->Read(offset, &phdr, &phdr.p_flags, sizeof(phdr.p_flags))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_flags, sizeof(phdr.p_flags))) {
return false;
}
if ((phdr.p_flags & PF_X) == 0) {
continue;
}
- if (!memory_->Read(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {
return false;
}
- if (!memory_->Read(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
return false;
}
- if (!memory_->Read(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
return false;
}
pt_loads_[phdr.p_offset] = LoadInfo{phdr.p_offset, phdr.p_vaddr,
@@ -79,22 +79,22 @@
}
case PT_GNU_EH_FRAME:
- if (!memory_->Read(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
return false;
}
eh_frame_offset_ = phdr.p_offset;
- if (!memory_->Read(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
return false;
}
eh_frame_size_ = phdr.p_memsz;
break;
case PT_DYNAMIC:
- if (!memory_->Read(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {
return false;
}
dynamic_offset_ = phdr.p_offset;
- if (!memory_->Read(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
return false;
}
dynamic_size_ = phdr.p_memsz;
@@ -116,8 +116,8 @@
ShdrType shdr;
if (ehdr.e_shstrndx < ehdr.e_shnum) {
uint64_t sh_offset = offset + ehdr.e_shstrndx * ehdr.e_shentsize;
- if (memory_->Read(sh_offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset))
- && memory_->Read(sh_offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
+ if (memory_->ReadField(sh_offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) &&
+ memory_->ReadField(sh_offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
sec_offset = shdr.sh_offset;
sec_size = shdr.sh_size;
}
@@ -125,27 +125,27 @@
// Skip the first header, it's always going to be NULL.
for (size_t i = 1; i < ehdr.e_shnum; i++, offset += ehdr.e_shentsize) {
- if (!memory_->Read(offset, &shdr, &shdr.sh_type, sizeof(shdr.sh_type))) {
+ if (!memory_->ReadField(offset, &shdr, &shdr.sh_type, sizeof(shdr.sh_type))) {
return false;
}
if (shdr.sh_type == SHT_PROGBITS) {
// Look for the .debug_frame and .gnu_debugdata.
- if (!memory_->Read(offset, &shdr, &shdr.sh_name, sizeof(shdr.sh_name))) {
+ if (!memory_->ReadField(offset, &shdr, &shdr.sh_name, sizeof(shdr.sh_name))) {
return false;
}
if (shdr.sh_name < sec_size) {
std::string name;
if (memory_->ReadString(sec_offset + shdr.sh_name, &name)) {
if (name == ".debug_frame") {
- if (memory_->Read(offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset))
- && memory_->Read(offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
+ if (memory_->ReadField(offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) &&
+ memory_->ReadField(offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
debug_frame_offset_ = shdr.sh_offset;
debug_frame_size_ = shdr.sh_size;
}
} else if (name == ".gnu_debugdata") {
- if (memory_->Read(offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset))
- && memory_->Read(offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
+ if (memory_->ReadField(offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) &&
+ memory_->ReadField(offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {
gnu_debugdata_offset_ = shdr.sh_offset;
gnu_debugdata_size_ = shdr.sh_size;
}
diff --git a/libunwindstack/ElfInterfaceArm.cpp b/libunwindstack/ElfInterfaceArm.cpp
index e157320..bab84cc 100644
--- a/libunwindstack/ElfInterfaceArm.cpp
+++ b/libunwindstack/ElfInterfaceArm.cpp
@@ -85,10 +85,10 @@
}
Elf32_Phdr phdr;
- if (!memory_->Read(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {
return true;
}
- if (!memory_->Read(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
+ if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {
return true;
}
// The load_bias_ should always be set by this time.
@@ -98,13 +98,15 @@
}
bool ElfInterfaceArm::Step(uint64_t pc, Regs* regs, Memory* process_memory) {
- return StepExidx(pc, regs, process_memory) ||
- ElfInterface32::Step(pc, regs, process_memory);
+ // Dwarf unwind information is precise about whether a pc is covered or not,
+ // but arm unwind information only has ranges of pc. In order to avoid
+ // incorrectly doing a bad unwind using arm unwind information for a
+ // different function, always try and unwind with the dwarf information first.
+ return ElfInterface32::Step(pc, regs, process_memory) || StepExidx(pc, regs, process_memory);
}
bool ElfInterfaceArm::StepExidx(uint64_t pc, Regs* regs, Memory* process_memory) {
RegsArm* regs_arm = reinterpret_cast<RegsArm*>(regs);
- // First try arm, then try dwarf.
uint64_t entry_offset;
if (!FindEntry(pc, &entry_offset)) {
return false;
diff --git a/libunwindstack/Memory.cpp b/libunwindstack/Memory.cpp
index 1fcf842..9e46509 100644
--- a/libunwindstack/Memory.cpp
+++ b/libunwindstack/Memory.cpp
@@ -96,10 +96,16 @@
offset_ = offset & (getpagesize() - 1);
uint64_t aligned_offset = offset & ~(getpagesize() - 1);
+ if (aligned_offset > static_cast<uint64_t>(buf.st_size) ||
+ offset > static_cast<uint64_t>(buf.st_size)) {
+ return false;
+ }
+
size_ = buf.st_size - aligned_offset;
- if (size < (UINT64_MAX - offset_) && size + offset_ < size_) {
+ uint64_t max_size;
+ if (!__builtin_add_overflow(size, offset_, &max_size) && max_size < size_) {
// Truncate the mapped size.
- size_ = size + offset_;
+ size_ = max_size;
}
void* map = mmap(nullptr, size_, PROT_READ, MAP_PRIVATE, fd, aligned_offset);
if (map == MAP_FAILED) {
@@ -113,14 +119,15 @@
}
bool MemoryFileAtOffset::Read(uint64_t addr, void* dst, size_t size) {
- if (addr + size > size_) {
+ uint64_t max_size;
+ if (__builtin_add_overflow(addr, size, &max_size) || max_size > size_) {
return false;
}
memcpy(dst, &data_[addr], size);
return true;
}
-static bool PtraceRead(pid_t pid, uint64_t addr, long* value) {
+bool MemoryRemote::PtraceRead(uint64_t addr, long* value) {
#if !defined(__LP64__)
// Cannot read an address greater than 32 bits.
if (addr > UINT32_MAX) {
@@ -130,7 +137,7 @@
// ptrace() returns -1 and sets errno when the operation fails.
// To disambiguate -1 from a valid result, we clear errno beforehand.
errno = 0;
- *value = ptrace(PTRACE_PEEKTEXT, pid, reinterpret_cast<void*>(addr), nullptr);
+ *value = ptrace(PTRACE_PEEKTEXT, pid_, reinterpret_cast<void*>(addr), nullptr);
if (*value == -1 && errno) {
return false;
}
@@ -138,11 +145,17 @@
}
bool MemoryRemote::Read(uint64_t addr, void* dst, size_t bytes) {
+ // Make sure that there is no overflow.
+ uint64_t max_size;
+ if (__builtin_add_overflow(addr, bytes, &max_size)) {
+ return false;
+ }
+
size_t bytes_read = 0;
long data;
size_t align_bytes = addr & (sizeof(long) - 1);
if (align_bytes != 0) {
- if (!PtraceRead(pid_, addr & ~(sizeof(long) - 1), &data)) {
+ if (!PtraceRead(addr & ~(sizeof(long) - 1), &data)) {
return false;
}
size_t copy_bytes = std::min(sizeof(long) - align_bytes, bytes);
@@ -154,7 +167,7 @@
}
for (size_t i = 0; i < bytes / sizeof(long); i++) {
- if (!PtraceRead(pid_, addr, &data)) {
+ if (!PtraceRead(addr, &data)) {
return false;
}
memcpy(dst, &data, sizeof(long));
@@ -165,7 +178,7 @@
size_t left_over = bytes & (sizeof(long) - 1);
if (left_over) {
- if (!PtraceRead(pid_, addr, &data)) {
+ if (!PtraceRead(addr, &data)) {
return false;
}
memcpy(dst, &data, left_over);
@@ -175,7 +188,13 @@
}
bool MemoryLocal::Read(uint64_t addr, void* dst, size_t size) {
- // The process_vm_readv call does will not always work on remote
+ // Make sure that there is no overflow.
+ uint64_t max_size;
+ if (__builtin_add_overflow(addr, size, &max_size)) {
+ return false;
+ }
+
+ // The process_vm_readv call will not always work on remote
// processes, so only use it for reads from the current pid.
// Use this method to avoid crashes if an address is invalid since
// unwind data could try to access any part of the address space.
@@ -208,9 +227,29 @@
}
bool MemoryOffline::Read(uint64_t addr, void* dst, size_t size) {
- if (addr < start_ || addr + size > start_ + offset_ + size_) {
+ uint64_t max_size;
+ if (__builtin_add_overflow(addr, size, &max_size)) {
+ return false;
+ }
+
+ uint64_t real_size;
+ if (__builtin_add_overflow(start_, offset_, &real_size) ||
+ __builtin_add_overflow(real_size, size_, &real_size)) {
+ return false;
+ }
+
+ if (addr < start_ || max_size > real_size) {
return false;
}
memcpy(dst, &data_[addr + offset_ - start_ + sizeof(start_)], size);
return true;
}
+
+bool MemoryRange::Read(uint64_t addr, void* dst, size_t size) {
+ uint64_t max_read;
+ if (__builtin_add_overflow(addr, size, &max_read) || max_read > length_) {
+ return false;
+ }
+ // The check above guarantees that addr + begin_ will not overflow.
+ return memory_->Read(addr + begin_, dst, size);
+}
diff --git a/libunwindstack/Memory.h b/libunwindstack/Memory.h
index c5316a1..f9f6d56 100644
--- a/libunwindstack/Memory.h
+++ b/libunwindstack/Memory.h
@@ -17,6 +17,7 @@
#ifndef _LIBUNWINDSTACK_MEMORY_H
#define _LIBUNWINDSTACK_MEMORY_H
+#include <assert.h>
#include <stdint.h>
#include <sys/types.h>
#include <unistd.h>
@@ -33,9 +34,16 @@
virtual bool Read(uint64_t addr, void* dst, size_t size) = 0;
- inline bool Read(uint64_t addr, void* start, void* field, size_t size) {
- return Read(addr + reinterpret_cast<uintptr_t>(field) - reinterpret_cast<uintptr_t>(start),
- field, size);
+ inline bool ReadField(uint64_t addr, void* start, void* field, size_t size) {
+ if (reinterpret_cast<uintptr_t>(field) < reinterpret_cast<uintptr_t>(start)) {
+ return false;
+ }
+ uint64_t offset = reinterpret_cast<uintptr_t>(field) - reinterpret_cast<uintptr_t>(start);
+ if (__builtin_add_overflow(addr, offset, &offset)) {
+ return false;
+ }
+ // The read will check if offset + size overflows.
+ return Read(offset, field, size);
}
inline bool Read32(uint64_t addr, uint32_t* dst) {
@@ -103,6 +111,9 @@
pid_t pid() { return pid_; }
+ protected:
+ virtual bool PtraceRead(uint64_t addr, long* value);
+
private:
pid_t pid_;
};
@@ -118,15 +129,12 @@
class MemoryRange : public Memory {
public:
MemoryRange(Memory* memory, uint64_t begin, uint64_t end)
- : memory_(memory), begin_(begin), length_(end - begin_) {}
+ : memory_(memory), begin_(begin), length_(end - begin) {
+ assert(end > begin);
+ }
virtual ~MemoryRange() { delete memory_; }
- inline bool Read(uint64_t addr, void* dst, size_t size) override {
- if (addr + size <= length_) {
- return memory_->Read(addr + begin_, dst, size);
- }
- return false;
- }
+ bool Read(uint64_t addr, void* dst, size_t size) override;
private:
Memory* memory_;
diff --git a/libunwindstack/tests/MemoryBuffer.cpp b/libunwindstack/tests/MemoryBuffer.cpp
new file mode 100644
index 0000000..af3d6b9
--- /dev/null
+++ b/libunwindstack/tests/MemoryBuffer.cpp
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2016 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 <vector>
+
+#include <gtest/gtest.h>
+
+#include "Memory.h"
+
+#include "LogFake.h"
+
+class MemoryBufferTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ ResetLogs();
+ memory_.reset(new MemoryBuffer);
+ }
+ std::unique_ptr<MemoryBuffer> memory_;
+};
+
+TEST_F(MemoryBufferTest, empty) {
+ ASSERT_EQ(0U, memory_->Size());
+ std::vector<uint8_t> buffer(1024);
+ ASSERT_FALSE(memory_->Read(0, buffer.data(), 1));
+ ASSERT_EQ(nullptr, memory_->GetPtr(0));
+ ASSERT_EQ(nullptr, memory_->GetPtr(1));
+}
+
+TEST_F(MemoryBufferTest, write_read) {
+ memory_->Resize(256);
+ ASSERT_EQ(256U, memory_->Size());
+ ASSERT_TRUE(memory_->GetPtr(0) != nullptr);
+ ASSERT_TRUE(memory_->GetPtr(1) != nullptr);
+ ASSERT_TRUE(memory_->GetPtr(255) != nullptr);
+ ASSERT_TRUE(memory_->GetPtr(256) == nullptr);
+
+ uint8_t* data = memory_->GetPtr(0);
+ for (size_t i = 0; i < memory_->Size(); i++) {
+ data[i] = i;
+ }
+
+ std::vector<uint8_t> buffer(memory_->Size());
+ ASSERT_TRUE(memory_->Read(0, buffer.data(), buffer.size()));
+ for (size_t i = 0; i < buffer.size(); i++) {
+ ASSERT_EQ(i, buffer[i]) << "Failed at byte " << i;
+ }
+}
+
+TEST_F(MemoryBufferTest, read_failures) {
+ memory_->Resize(100);
+ std::vector<uint8_t> buffer(200);
+ ASSERT_FALSE(memory_->Read(0, buffer.data(), 101));
+ ASSERT_FALSE(memory_->Read(100, buffer.data(), 1));
+ ASSERT_FALSE(memory_->Read(101, buffer.data(), 2));
+ ASSERT_FALSE(memory_->Read(99, buffer.data(), 2));
+ ASSERT_TRUE(memory_->Read(99, buffer.data(), 1));
+}
+
+TEST_F(MemoryBufferTest, read_failure_overflow) {
+ memory_->Resize(100);
+ std::vector<uint8_t> buffer(200);
+
+ ASSERT_FALSE(memory_->Read(UINT64_MAX - 100, buffer.data(), 200));
+}
diff --git a/libunwindstack/tests/MemoryFake.h b/libunwindstack/tests/MemoryFake.h
index e05736b..70ef30a 100644
--- a/libunwindstack/tests/MemoryFake.h
+++ b/libunwindstack/tests/MemoryFake.h
@@ -75,4 +75,16 @@
}
};
+class MemoryFakeRemote : public MemoryRemote {
+ public:
+ MemoryFakeRemote() : MemoryRemote(0) {}
+ virtual ~MemoryFakeRemote() = default;
+
+ protected:
+ bool PtraceRead(uint64_t, long* value) override {
+ *value = 0;
+ return true;
+ }
+};
+
#endif // _LIBUNWINDSTACK_TESTS_MEMORY_FAKE_H
diff --git a/libunwindstack/tests/MemoryFileTest.cpp b/libunwindstack/tests/MemoryFileTest.cpp
index 870ca19..aa7a23a 100644
--- a/libunwindstack/tests/MemoryFileTest.cpp
+++ b/libunwindstack/tests/MemoryFileTest.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#include <string>
+#include <vector>
+
#include <android-base/test_utils.h>
#include <android-base/file.h>
#include <gtest/gtest.h>
@@ -39,7 +42,7 @@
TemporaryFile* tf_ = nullptr;
};
-TEST_F(MemoryFileTest, offset_0) {
+TEST_F(MemoryFileTest, init_offset_0) {
WriteTestData();
ASSERT_TRUE(memory_.Init(tf_->path, 0));
@@ -49,7 +52,7 @@
ASSERT_STREQ("0123456789", buffer.data());
}
-TEST_F(MemoryFileTest, offset_non_zero) {
+TEST_F(MemoryFileTest, init_offset_non_zero) {
WriteTestData();
ASSERT_TRUE(memory_.Init(tf_->path, 10));
@@ -59,7 +62,7 @@
ASSERT_STREQ("abcdefghij", buffer.data());
}
-TEST_F(MemoryFileTest, offset_non_zero_larger_than_pagesize) {
+TEST_F(MemoryFileTest, init_offset_non_zero_larger_than_pagesize) {
size_t pagesize = getpagesize();
std::string large_string;
for (size_t i = 0; i < pagesize; i++) {
@@ -75,7 +78,7 @@
ASSERT_STREQ("abcdefgh", buffer.data());
}
-TEST_F(MemoryFileTest, offset_pagesize_aligned) {
+TEST_F(MemoryFileTest, init_offset_pagesize_aligned) {
size_t pagesize = getpagesize();
std::string data;
for (size_t i = 0; i < 2 * pagesize; i++) {
@@ -96,7 +99,7 @@
ASSERT_STREQ(expected_str.c_str(), buffer.data());
}
-TEST_F(MemoryFileTest, offset_pagesize_aligned_plus_extra) {
+TEST_F(MemoryFileTest, init_offset_pagesize_aligned_plus_extra) {
size_t pagesize = getpagesize();
std::string data;
for (size_t i = 0; i < 2 * pagesize; i++) {
@@ -117,6 +120,23 @@
ASSERT_STREQ(expected_str.c_str(), buffer.data());
}
+TEST_F(MemoryFileTest, init_offset_greater_than_filesize) {
+ size_t pagesize = getpagesize();
+ std::string data;
+ uint64_t file_size = 2 * pagesize + pagesize / 2;
+ for (size_t i = 0; i < file_size; i++) {
+ data += static_cast<char>((i / pagesize) + '0');
+ }
+ ASSERT_TRUE(android::base::WriteStringToFd(data, tf_->fd));
+
+ // Check offset > file size fails and aligned_offset > file size.
+ ASSERT_FALSE(memory_.Init(tf_->path, file_size + 2 * pagesize));
+ // Check offset == filesize fails.
+ ASSERT_FALSE(memory_.Init(tf_->path, file_size));
+ // Check aligned_offset < filesize, but offset > filesize fails.
+ ASSERT_FALSE(memory_.Init(tf_->path, 2 * pagesize + pagesize / 2 + pagesize / 4));
+}
+
TEST_F(MemoryFileTest, read_error) {
std::string data;
for (size_t i = 0; i < 5000; i++) {
@@ -137,32 +157,9 @@
ASSERT_TRUE(memory_.Read(4990, buffer.data(), 10));
ASSERT_FALSE(memory_.Read(4999, buffer.data(), 2));
ASSERT_TRUE(memory_.Read(4999, buffer.data(), 1));
-}
-TEST_F(MemoryFileTest, read_string) {
- std::string value("name_in_file");
- ASSERT_TRUE(android::base::WriteFully(tf_->fd, value.c_str(), value.size() + 1));
-
- std::string name;
- ASSERT_TRUE(memory_.Init(tf_->path, 0));
- ASSERT_TRUE(memory_.ReadString(0, &name));
- ASSERT_EQ("name_in_file", name);
- ASSERT_TRUE(memory_.ReadString(5, &name));
- ASSERT_EQ("in_file", name);
-}
-
-TEST_F(MemoryFileTest, read_string_error) {
- std::vector<uint8_t> buffer = { 0x23, 0x32, 0x45 };
- ASSERT_TRUE(android::base::WriteFully(tf_->fd, buffer.data(), buffer.size()));
-
- std::string name;
- ASSERT_TRUE(memory_.Init(tf_->path, 0));
-
- // Read from a non-existant address.
- ASSERT_FALSE(memory_.ReadString(100, &name));
-
- // This should fail because there is no terminating \0
- ASSERT_FALSE(memory_.ReadString(0, &name));
+ // Check that overflow fails properly.
+ ASSERT_FALSE(memory_.Read(UINT64_MAX - 100, buffer.data(), 200));
}
TEST_F(MemoryFileTest, read_past_file_within_mapping) {
diff --git a/libunwindstack/tests/MemoryLocalTest.cpp b/libunwindstack/tests/MemoryLocalTest.cpp
index 0ba5f1c..ab999da 100644
--- a/libunwindstack/tests/MemoryLocalTest.cpp
+++ b/libunwindstack/tests/MemoryLocalTest.cpp
@@ -47,25 +47,6 @@
}
}
-TEST(MemoryLocalTest, read_string) {
- std::string name("string_in_memory");
-
- MemoryLocal local;
-
- std::vector<uint8_t> dst(1024);
- std::string dst_name;
- ASSERT_TRUE(local.ReadString(reinterpret_cast<uint64_t>(name.c_str()), &dst_name));
- ASSERT_EQ("string_in_memory", dst_name);
-
- ASSERT_TRUE(local.ReadString(reinterpret_cast<uint64_t>(&name[7]), &dst_name));
- ASSERT_EQ("in_memory", dst_name);
-
- ASSERT_TRUE(local.ReadString(reinterpret_cast<uint64_t>(&name[7]), &dst_name, 10));
- ASSERT_EQ("in_memory", dst_name);
-
- ASSERT_FALSE(local.ReadString(reinterpret_cast<uint64_t>(&name[7]), &dst_name, 9));
-}
-
TEST(MemoryLocalTest, read_illegal) {
MemoryLocal local;
@@ -73,3 +54,13 @@
ASSERT_FALSE(local.Read(0, dst.data(), 1));
ASSERT_FALSE(local.Read(0, dst.data(), 100));
}
+
+TEST(MemoryLocalTest, read_overflow) {
+ MemoryLocal local;
+
+ // On 32 bit this test doesn't necessarily cause an overflow. The 64 bit
+ // version will always go through the overflow check.
+ std::vector<uint8_t> dst(100);
+ uint64_t value;
+ ASSERT_FALSE(local.Read(reinterpret_cast<uint64_t>(&value), dst.data(), SIZE_MAX));
+}
diff --git a/libunwindstack/tests/MemoryRangeTest.cpp b/libunwindstack/tests/MemoryRangeTest.cpp
index d636ec4..ee5ba01 100644
--- a/libunwindstack/tests/MemoryRangeTest.cpp
+++ b/libunwindstack/tests/MemoryRangeTest.cpp
@@ -17,6 +17,7 @@
#include <stdint.h>
#include <string.h>
+#include <memory>
#include <vector>
#include <gtest/gtest.h>
@@ -65,35 +66,14 @@
ASSERT_FALSE(range.Read(1020, dst.data(), 5));
ASSERT_FALSE(range.Read(1024, dst.data(), 1));
ASSERT_FALSE(range.Read(1024, dst.data(), 1024));
+
+ // Verify that reading up to the end works.
+ ASSERT_TRUE(range.Read(1020, dst.data(), 4));
}
-TEST_F(MemoryRangeTest, read_string_past_end) {
- std::string name("0123456789");
- memory_->SetMemory(0, name);
+TEST_F(MemoryRangeTest, read_overflow) {
+ std::vector<uint8_t> buffer(100);
- // Verify a read past the range fails.
- MemoryRange range(memory_, 0, 5);
- std::string dst_name;
- ASSERT_FALSE(range.ReadString(0, &dst_name));
-}
-
-TEST_F(MemoryRangeTest, read_string_to_end) {
- std::string name("0123456789");
- memory_->SetMemory(30, name);
-
- // Verify the range going to the end of the string works.
- MemoryRange range(memory_, 30, 30 + name.size() + 1);
- std::string dst_name;
- ASSERT_TRUE(range.ReadString(0, &dst_name));
- ASSERT_EQ("0123456789", dst_name);
-}
-
-TEST_F(MemoryRangeTest, read_string_fencepost) {
- std::string name("0123456789");
- memory_->SetMemory(10, name);
-
- // Verify the range set to one byte less than the end of the string fails.
- MemoryRange range(memory_, 10, 10 + name.size());
- std::string dst_name;
- ASSERT_FALSE(range.ReadString(0, &dst_name));
+ std::unique_ptr<MemoryRange> overflow(new MemoryRange(new MemoryFakeAlwaysReadZero, 100, 200));
+ ASSERT_FALSE(overflow->Read(UINT64_MAX - 10, buffer.data(), 100));
}
diff --git a/libunwindstack/tests/MemoryRemoteTest.cpp b/libunwindstack/tests/MemoryRemoteTest.cpp
index 7664c3e..e48edf7 100644
--- a/libunwindstack/tests/MemoryRemoteTest.cpp
+++ b/libunwindstack/tests/MemoryRemoteTest.cpp
@@ -33,6 +33,8 @@
#include "Memory.h"
+#include "MemoryFake.h"
+
class MemoryRemoteTest : public ::testing::Test {
protected:
static uint64_t NanoTime() {
@@ -121,6 +123,9 @@
ASSERT_TRUE(remote.Read(reinterpret_cast<uint64_t>(src) + pagesize - 1, dst.data(), 1));
ASSERT_FALSE(remote.Read(reinterpret_cast<uint64_t>(src) + pagesize - 4, dst.data(), 8));
+ // Check overflow condition is caught properly.
+ ASSERT_FALSE(remote.Read(UINT64_MAX - 100, dst.data(), 200));
+
ASSERT_EQ(0, munmap(src, pagesize));
ASSERT_TRUE(Detach(pid));
@@ -128,6 +133,14 @@
kill(pid, SIGKILL);
}
+TEST_F(MemoryRemoteTest, read_overflow) {
+ MemoryFakeRemote remote;
+
+ // Check overflow condition is caught properly.
+ std::vector<uint8_t> dst(200);
+ ASSERT_FALSE(remote.Read(UINT64_MAX - 100, dst.data(), 200));
+}
+
TEST_F(MemoryRemoteTest, read_illegal) {
pid_t pid;
if ((pid = fork()) == 0) {
diff --git a/libunwindstack/tests/MemoryTest.cpp b/libunwindstack/tests/MemoryTest.cpp
new file mode 100644
index 0000000..51b5d7d
--- /dev/null
+++ b/libunwindstack/tests/MemoryTest.cpp
@@ -0,0 +1,126 @@
+/*
+ * 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 <stdint.h>
+#include <string.h>
+
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "Memory.h"
+
+#include "MemoryFake.h"
+
+TEST(MemoryTest, read32) {
+ MemoryFakeAlwaysReadZero memory;
+
+ uint32_t data = 0xffffffff;
+ ASSERT_TRUE(memory.Read32(0, &data));
+ ASSERT_EQ(0U, data);
+}
+
+TEST(MemoryTest, read64) {
+ MemoryFakeAlwaysReadZero memory;
+
+ uint64_t data = 0xffffffffffffffffULL;
+ ASSERT_TRUE(memory.Read64(0, &data));
+ ASSERT_EQ(0U, data);
+}
+
+struct FakeStruct {
+ int one;
+ bool two;
+ uint32_t three;
+ uint64_t four;
+};
+
+TEST(MemoryTest, read_field) {
+ MemoryFakeAlwaysReadZero memory;
+
+ FakeStruct data;
+ memset(&data, 0xff, sizeof(data));
+ ASSERT_TRUE(memory.ReadField(0, &data, &data.one, sizeof(data.one)));
+ ASSERT_EQ(0, data.one);
+
+ memset(&data, 0xff, sizeof(data));
+ ASSERT_TRUE(memory.ReadField(0, &data, &data.two, sizeof(data.two)));
+ ASSERT_FALSE(data.two);
+
+ memset(&data, 0xff, sizeof(data));
+ ASSERT_TRUE(memory.ReadField(0, &data, &data.three, sizeof(data.three)));
+ ASSERT_EQ(0U, data.three);
+
+ memset(&data, 0xff, sizeof(data));
+ ASSERT_TRUE(memory.ReadField(0, &data, &data.four, sizeof(data.four)));
+ ASSERT_EQ(0U, data.four);
+}
+
+TEST(MemoryTest, read_field_fails) {
+ MemoryFakeAlwaysReadZero memory;
+
+ FakeStruct data;
+ memset(&data, 0xff, sizeof(data));
+
+ ASSERT_FALSE(memory.ReadField(UINT64_MAX, &data, &data.three, sizeof(data.three)));
+
+ // Field and start reversed, should fail.
+ ASSERT_FALSE(memory.ReadField(100, &data.two, &data, sizeof(data.two)));
+ ASSERT_FALSE(memory.ReadField(0, &data.two, &data, sizeof(data.two)));
+}
+
+TEST(MemoryTest, read_string) {
+ std::string name("string_in_memory");
+
+ MemoryFake memory;
+
+ memory.SetMemory(100, name.c_str(), name.size() + 1);
+
+ std::string dst_name;
+ ASSERT_TRUE(memory.ReadString(100, &dst_name));
+ ASSERT_EQ("string_in_memory", dst_name);
+
+ ASSERT_TRUE(memory.ReadString(107, &dst_name));
+ ASSERT_EQ("in_memory", dst_name);
+
+ // Set size greater than string.
+ ASSERT_TRUE(memory.ReadString(107, &dst_name, 10));
+ ASSERT_EQ("in_memory", dst_name);
+
+ ASSERT_FALSE(memory.ReadString(107, &dst_name, 9));
+}
+
+TEST(MemoryTest, read_string_error) {
+ std::string name("short");
+
+ MemoryFake memory;
+
+ // Save everything except the terminating '\0'.
+ memory.SetMemory(0, name.c_str(), name.size());
+
+ std::string dst_name;
+ // Read from a non-existant address.
+ ASSERT_FALSE(memory.ReadString(100, &dst_name));
+
+ // This should fail because there is no terminating '\0'.
+ ASSERT_FALSE(memory.ReadString(0, &dst_name));
+
+ // This should pass because there is a terminating '\0'.
+ memory.SetData8(name.size(), '\0');
+ ASSERT_TRUE(memory.ReadString(0, &dst_name));
+ ASSERT_EQ("short", dst_name);
+}