Fix small ptrace reads.

The BacktracePtrace::Read function crashes if the number of bytes to
read is less than the number of bytes needed to align the read to
a word_t boundary.

Fix this and add a test for this case.

Change-Id: I50808849ece44928f65dba1d25309e3885c829a2
(cherry picked from commit 944f417ccb)
This commit is contained in:
Christopher Ferris 2015-05-06 16:36:34 -07:00
parent af67fb2475
commit 8bd4a4ecdc
2 changed files with 35 additions and 16 deletions

View file

@ -83,13 +83,12 @@ size_t BacktracePtrace::Read(uintptr_t addr, uint8_t* buffer, size_t bytes) {
if (!PtraceRead(Tid(), addr & ~(sizeof(word_t) - 1), &data_word)) { if (!PtraceRead(Tid(), addr & ~(sizeof(word_t) - 1), &data_word)) {
return 0; return 0;
} }
align_bytes = sizeof(word_t) - align_bytes; size_t copy_bytes = MIN(sizeof(word_t) - align_bytes, bytes);
memcpy(buffer, reinterpret_cast<uint8_t*>(&data_word) + sizeof(word_t) - align_bytes, memcpy(buffer, reinterpret_cast<uint8_t*>(&data_word) + align_bytes, copy_bytes);
align_bytes); addr += copy_bytes;
addr += align_bytes; buffer += copy_bytes;
buffer += align_bytes; bytes -= copy_bytes;
bytes -= align_bytes; bytes_read += copy_bytes;
bytes_read += align_bytes;
} }
size_t num_words = bytes / sizeof(word_t); size_t num_words = bytes / sizeof(word_t);

View file

@ -882,6 +882,17 @@ TEST(libbacktrace, verify_map_remote) {
ASSERT_EQ(waitpid(pid, nullptr, 0), pid); ASSERT_EQ(waitpid(pid, nullptr, 0), pid);
} }
void InitMemory(uint8_t* memory, size_t bytes) {
for (size_t i = 0; i < bytes; i++) {
memory[i] = i;
if (memory[i] == '\0') {
// Don't use '\0' in our data so we can verify that an overread doesn't
// occur by using a '\0' as the character after the read data.
memory[i] = 23;
}
}
}
void* ThreadReadTest(void* data) { void* ThreadReadTest(void* data) {
thread_t* thread_data = reinterpret_cast<thread_t*>(data); thread_t* thread_data = reinterpret_cast<thread_t*>(data);
@ -900,9 +911,7 @@ void* ThreadReadTest(void* data) {
} }
// Set up a simple pattern in memory. // Set up a simple pattern in memory.
for (size_t i = 0; i < pagesize; i++) { InitMemory(memory, pagesize);
memory[i] = i;
}
thread_data->data = memory; thread_data->data = memory;
@ -930,9 +939,8 @@ void RunReadTest(Backtrace* backtrace, uintptr_t read_addr) {
// Create a page of data to use to do quick compares. // Create a page of data to use to do quick compares.
uint8_t* expected = new uint8_t[pagesize]; uint8_t* expected = new uint8_t[pagesize];
for (size_t i = 0; i < pagesize; i++) { InitMemory(expected, pagesize);
expected[i] = i;
}
uint8_t* data = new uint8_t[2*pagesize]; uint8_t* data = new uint8_t[2*pagesize];
// Verify that we can only read one page worth of data. // Verify that we can only read one page worth of data.
size_t bytes_read = backtrace->Read(read_addr, data, 2 * pagesize); size_t bytes_read = backtrace->Read(read_addr, data, 2 * pagesize);
@ -946,6 +954,20 @@ void RunReadTest(Backtrace* backtrace, uintptr_t read_addr) {
ASSERT_TRUE(memcmp(data, &expected[i], 2 * sizeof(word_t)) == 0) ASSERT_TRUE(memcmp(data, &expected[i], 2 * sizeof(word_t)) == 0)
<< "Offset at " << i << " failed"; << "Offset at " << i << " failed";
} }
// Verify small unaligned reads.
for (size_t i = 1; i < sizeof(word_t); i++) {
for (size_t j = 1; j < sizeof(word_t); j++) {
// Set one byte past what we expect to read, to guarantee we don't overread.
data[j] = '\0';
bytes_read = backtrace->Read(read_addr + i, data, j);
ASSERT_EQ(j, bytes_read);
ASSERT_TRUE(memcmp(data, &expected[i], j) == 0)
<< "Offset at " << i << " length " << j << " miscompared";
ASSERT_EQ('\0', data[j])
<< "Offset at " << i << " length " << j << " wrote too much data";
}
}
delete data; delete data;
delete expected; delete expected;
} }
@ -989,9 +1011,7 @@ void ForkedReadTest() {
} }
// Set up a simple pattern in memory. // Set up a simple pattern in memory.
for (size_t i = 0; i < pagesize; i++) { InitMemory(memory, pagesize);
memory[i] = i;
}
g_addr = reinterpret_cast<uintptr_t>(memory); g_addr = reinterpret_cast<uintptr_t>(memory);
g_ready = 1; g_ready = 1;