3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-03-09 14:06:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

ios: vm_read module file path before calling strlen.

Browse Source 
Adds a new IOSIntermediateDumpWriter::AddPropertyCString method which
takes an address to a cstring of unknown length and page-by-page
searches for a NUL-byte terminator.

This is necessary because currently WriteModuleInfo calls strlen
directly on the dyld and module filePath without first using vm_read.
On iOS14 this occasionally crashes, and is generally unwise. Instead,
use AddPropertyCString.

This patch also removes WriteDyldErrorStringAnnotation, as it's no
longer used going forward with iOS 15.

Bug: 1332862
Change-Id: I3801693bc39259a0127e5175dccf286a1cd97ba7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3689516
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This commit is contained in:
Justin Cohen 2022-07-14 13:41:55 -04:00 committed by Crashpad LUCI CQ
parent 1424632592
commit b7db85b62d
4 changed files with 196 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

135
client/ios_handler/in_process_intermediate_dump_handler.cc
View File

@ -118,7 +118,7 @@ void WriteProperty(IOSIntermediateDumpWriter* writer,
//! \param[in] writer The dump writer
//! \param[in] key The key to write.
//! \param[in] value Memory to be written.
//! \param[in] count Length of \a data.
//! \param[in] value_length Length of \a data.
void WritePropertyBytes(IOSIntermediateDumpWriter* writer,
IntermediateDumpKey key,
const void* value,
@ -127,6 +127,20 @@ void WritePropertyBytes(IOSIntermediateDumpWriter* writer,
WriteError(key);
}
//! \brief Call AddPropertyCString with raw error log.
//!
//! \param[in] writer The dump writer
//! \param[in] key The key to write.
//! \param[in] max_length The maximum string length.
//! \param[in] value Memory to be written.
void WritePropertyCString(IOSIntermediateDumpWriter* writer,
IntermediateDumpKey key,
size_t max_length,
const char* value) {
if (!writer->AddPropertyCString(key, max_length, value))
WriteError(key);
}
kern_return_t MachVMRegionRecurseDeepest(task_t task,
vm_address_t* address,
vm_size_t* size,
@ -498,80 +512,6 @@ void WriteAppleCrashReporterAnnotations(
}
}
void WriteDyldErrorStringAnnotation(
IOSIntermediateDumpWriter* writer,
const uint64_t address,
const symtab_command* symtab_command_ptr,
const dysymtab_command* dysymtab_command_ptr,
const segment_command_64* text_seg_ptr,
const segment_command_64* linkedit_seg_ptr,
vm_size_t slide) {
if (text_seg_ptr == nullptr || linkedit_seg_ptr == nullptr ||
symtab_command_ptr == nullptr) {
return;
}
ScopedVMRead<symtab_command> symtab_command;
ScopedVMRead<dysymtab_command> dysymtab_command;
ScopedVMRead<segment_command_64> text_seg;
ScopedVMRead<segment_command_64> linkedit_seg;
if (!symtab_command.Read(symtab_command_ptr) ||
!text_seg.Read(text_seg_ptr) || !linkedit_seg.Read(linkedit_seg_ptr) ||
(dysymtab_command_ptr && !dysymtab_command.Read(dysymtab_command_ptr))) {
CRASHPAD_RAW_LOG("Unable to load dyld symbol table.");
}
uint64_t file_slide =
(linkedit_seg->vmaddr - text_seg->vmaddr) - linkedit_seg->fileoff;
uint64_t strings = address + (symtab_command->stroff + file_slide);
nlist_64* symbol_ptr = reinterpret_cast<nlist_64*>(
address + (symtab_command->symoff + file_slide));
// If a dysymtab is present, use it to filter the symtab for just the
// portion used for extdefsym. If no dysymtab is present, the entire symtab
// will need to be consulted.
uint32_t symbol_count = symtab_command->nsyms;
if (dysymtab_command_ptr) {
symbol_ptr += dysymtab_command->iextdefsym;
symbol_count = dysymtab_command->nextdefsym;
}
for (uint32_t i = 0; i < symbol_count; i++, symbol_ptr++) {
ScopedVMRead<nlist_64> symbol;
if (!symbol.Read(symbol_ptr)) {
CRASHPAD_RAW_LOG("Unable to load dyld symbol table symbol.");
return;
}
if (!symbol->n_value)
continue;
ScopedVMRead<const char> symbol_name;
if (!symbol_name.Read(strings + symbol->n_un.n_strx)) {
CRASHPAD_RAW_LOG("Unable to load dyld symbol name.");
}
if (strcmp(symbol_name.get(), "_error_string") == 0) {
ScopedVMRead<const char> symbol_value;
if (!symbol_value.Read(symbol->n_value + slide)) {
CRASHPAD_RAW_LOG("Unable to load dyld symbol value.");
}
// 1024 here is distinct from kMaxMessageSize above, because it refers to
// a precisely-sized buffer inside dyld.
const size_t value_len = strnlen(symbol_value.get(), 1024);
if (value_len) {
WriteProperty(writer,
IntermediateDumpKey::kAnnotationsDyldErrorString,
symbol_value.get(),
value_len);
}
return;
}
continue;
}
}
} // namespace
// static
@ -980,10 +920,8 @@ void InProcessIntermediateDumpHandler::WriteModuleInfo(
}
if (image->imageFilePath) {
WriteProperty(writer,
IntermediateDumpKey::kName,
image->imageFilePath,
strlen(image->imageFilePath));
WritePropertyCString(
writer, IntermediateDumpKey::kName, PATH_MAX, image->imageFilePath);
}
uint64_t address = FromPointerCast<uint64_t>(image->imageLoadAddress);
WriteProperty(writer, IntermediateDumpKey::kAddress, &address);
@ -995,10 +933,8 @@ void InProcessIntermediateDumpHandler::WriteModuleInfo(
{
IOSIntermediateDumpWriter::ScopedArrayMap modules(writer);
if (image_infos->dyldPath) {
WriteProperty(writer,
IntermediateDumpKey::kName,
image_infos->dyldPath,
strlen(image_infos->dyldPath));
WritePropertyCString(
writer, IntermediateDumpKey::kName, PATH_MAX, image_infos->dyldPath);
}
uint64_t address =
FromPointerCast<uint64_t>(image_infos->dyldImageLoadAddress);
@ -1129,10 +1065,6 @@ void InProcessIntermediateDumpHandler::WriteModuleInfoAtAddress(
// Make sure that the basic load command structure doesn’t overflow the
// space allotted for load commands, as well as iterating through ncmds.
vm_size_t slide = 0;
const symtab_command* symtab_command = nullptr;
const dysymtab_command* dysymtab_command = nullptr;
const segment_command_64* linkedit_seg = nullptr;
const segment_command_64* text_seg = nullptr;
for (uint32_t cmd_index = 0, cumulative_cmd_size = 0;
cmd_index <= header->ncmds && cumulative_cmd_size < header->sizeofcmds;
++cmd_index, cumulative_cmd_size += command->cmdsize) {
@ -1145,20 +1077,11 @@ void InProcessIntermediateDumpHandler::WriteModuleInfoAtAddress(
const segment_command_64* segment_ptr =
reinterpret_cast<const segment_command_64*>(command_ptr);
if (strcmp(segment->segname, SEG_TEXT) == 0) {
text_seg = segment_ptr;
WriteProperty(writer, IntermediateDumpKey::kSize, &segment->vmsize);
slide = address - segment->vmaddr;
} else if (strcmp(segment->segname, SEG_DATA) == 0) {
WriteDataSegmentAnnotations(writer, segment_ptr, slide);
} else if (strcmp(segment->segname, SEG_LINKEDIT) == 0) {
linkedit_seg = segment_ptr;
}
} else if (command->cmd == LC_SYMTAB) {
symtab_command =
reinterpret_cast<const struct symtab_command*>(command_ptr);
} else if (command->cmd == LC_DYSYMTAB) {
dysymtab_command =
reinterpret_cast<const struct dysymtab_command*>(command_ptr);
} else if (command->cmd == LC_ID_DYLIB) {
ScopedVMRead<dylib_command> dylib;
if (!dylib.Read(command_ptr)) {
@ -1195,16 +1118,6 @@ void InProcessIntermediateDumpHandler::WriteModuleInfoAtAddress(
}
WriteProperty(writer, IntermediateDumpKey::kFileType, &header->filetype);
if (is_dyld && header->filetype == MH_DYLINKER) {
WriteDyldErrorStringAnnotation(writer,
address,
symtab_command,
dysymtab_command,
text_seg,
linkedit_seg,
slide);
}
}
void InProcessIntermediateDumpHandler::WriteDataSegmentAnnotations(
@ -1286,12 +1199,10 @@ void InProcessIntermediateDumpHandler::WriteCrashpadAnnotationsList(
}
IOSIntermediateDumpWriter::ScopedArrayMap annotation_map(writer);
const size_t name_len = strnlen(reinterpret_cast<const char*>(node->name()),
Annotation::kNameMaxLength);
WritePropertyBytes(writer,
IntermediateDumpKey::kAnnotationName,
reinterpret_cast<const void*>(node->name()),
name_len);
WritePropertyCString(writer,
IntermediateDumpKey::kAnnotationName,
Annotation::kNameMaxLength,
reinterpret_cast<const char*>(node->name()));
WritePropertyBytes(writer,
IntermediateDumpKey::kAnnotationValue,
reinterpret_cast<const void*>(node->value()),

80
util/ios/ios_intermediate_dump_writer.cc
View File

@ -15,8 +15,10 @@
#include "util/ios/ios_intermediate_dump_writer.h"
#include <fcntl.h>
#include <mach/mach.h>
#include <unistd.h>
#include <algorithm>
#include <ostream>
#include "base/check.h"
@ -86,6 +88,73 @@ bool IOSIntermediateDumpWriter::Close() {
return RawLoggingCloseFile(fd);
}
bool IOSIntermediateDumpWriter::AddPropertyCString(IntermediateDumpKey key,
size_t max_length,
const char* value) {
constexpr size_t kMaxStringBytes = 1024;
if (max_length > kMaxStringBytes) {
CRASHPAD_RAW_LOG("AddPropertyCString max_length too large");
return false;
}
char buffer[kMaxStringBytes];
size_t string_length;
if (ReadCStringInternal(value, buffer, max_length, &string_length)) {
return Property(key, buffer, string_length);
}
return false;
}
bool IOSIntermediateDumpWriter::ReadCStringInternal(const char* value,
char* buffer,
size_t max_length,
size_t* string_length) {
size_t length = 0;
while (length < max_length) {
vm_address_t data_address = reinterpret_cast<vm_address_t>(value + length);
// Calculate bytes to read past `data_address`, either the number of bytes
// to the end of the page, or the remaining bytes in `buffer`, whichever is
// smaller.
size_t data_to_end_of_page =
getpagesize() - (data_address - trunc_page(data_address));
size_t remaining_bytes_in_buffer = max_length - length;
size_t bytes_to_read =
std::min(data_to_end_of_page, remaining_bytes_in_buffer);
char* buffer_start = buffer + length;
size_t bytes_read = 0;
kern_return_t kr =
vm_read_overwrite(mach_task_self(),
data_address,
bytes_to_read,
reinterpret_cast<vm_address_t>(buffer_start),
&bytes_read);
if (kr != KERN_SUCCESS || bytes_read <= 0) {
CRASHPAD_RAW_LOG("ReadCStringInternal vm_read_overwrite failed");
return false;
}
char* nul = static_cast<char*>(memchr(buffer_start, '\0', bytes_read));
if (nul != nullptr) {
length += nul - buffer_start;
*string_length = length;
return true;
}
length += bytes_read;
}
CRASHPAD_RAW_LOG("unterminated string");
return false;
}
bool IOSIntermediateDumpWriter::AddPropertyInternal(IntermediateDumpKey key,
const char* value,
size_t value_length) {
ScopedVMRead<char> vmread;
if (!vmread.Read(value, value_length))
return false;
return Property(key, vmread.get(), value_length);
}
bool IOSIntermediateDumpWriter::ArrayMapStart() {
const CommandType command_type = CommandType::kMapStart;
return RawLoggingWriteFile(fd_, &command_type, sizeof(command_type));
@ -123,17 +192,14 @@ bool IOSIntermediateDumpWriter::RootMapEnd() {
return RawLoggingWriteFile(fd_, &command_type, sizeof(command_type));
}
bool IOSIntermediateDumpWriter::AddPropertyInternal(IntermediateDumpKey key,
const char* value,
size_t value_length) {
ScopedVMRead<char> vmread;
if (!vmread.Read(value, value_length))
return false;
bool IOSIntermediateDumpWriter::Property(IntermediateDumpKey key,
const void* value,
size_t value_length) {
const CommandType command_type = CommandType::kProperty;
return RawLoggingWriteFile(fd_, &command_type, sizeof(command_type)) &&
RawLoggingWriteFile(fd_, &key, sizeof(key)) &&
RawLoggingWriteFile(fd_, &value_length, sizeof(size_t)) &&
RawLoggingWriteFile(fd_, vmread.get(), value_length);
RawLoggingWriteFile(fd_, value, value_length);
}
} // namespace internal

28
util/ios/ios_intermediate_dump_writer.h
View File

@ -175,9 +175,27 @@ class IOSIntermediateDumpWriter final {
key, reinterpret_cast<const char*>(value), value_length);
}
//! \return Returns `true` if able to vm_read a string of \a value and write
//! a kProperty command with the \a key \a value up to a NUL byte.
//! The string cannot be longer than \a max_length with a maximum string
//! length of 1024.
bool AddPropertyCString(IntermediateDumpKey key,
size_t max_length,
const char* value);
private:
//! \return Returns `true` if able to write a kProperty command with the
//! \a key \a value \a count tuple.
//! \return Returns `true` if able to vm_read_overwrite \a value into
//! \a buffer while only reading one page at a time up to a NUL byte.
//! Sets the final length of \a buffer to \a string_length.
//! Returns `false` if unable to vm_read \a value or when no NUL byte can
//! be found within /a max_length (unterminated).
bool ReadCStringInternal(const char* value,
char* buffer,
size_t max_length,
size_t* string_length);
//! \return Returns `true` if able to vm_read \a value \a count and write a
//! kProperty command with the \a key \a value \a count tuple.
bool AddPropertyInternal(IntermediateDumpKey key,
const char* value,
size_t value_length);
@ -205,6 +223,12 @@ class IOSIntermediateDumpWriter final {
//! \return Returns `true` if able to write a kRootMapEnd command.
bool RootMapEnd();
//! \return Returns `true` if able to write a kProperty command with the
//! \a key \a value \a value_length tuple.
bool Property(IntermediateDumpKey key,
const void* value,
size_t value_length);
int fd_;
};

74
util/ios/ios_intermediate_dump_writer_test.cc
View File

@ -15,8 +15,10 @@
#include "util/ios/ios_intermediate_dump_writer.h"
#include <fcntl.h>
#include <mach/mach.h>
#include "base/files/scoped_file.h"
#include "base/mac/scoped_mach_vm.h"
#include "base/posix/eintr_wrapper.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@ -102,6 +104,78 @@ TEST_F(IOSIntermediateDumpWriterTest, Property) {
ASSERT_EQ(contents, result);
}
TEST_F(IOSIntermediateDumpWriterTest, PropertyString) {
EXPECT_TRUE(writer_->Open(path()));
EXPECT_TRUE(writer_->AddPropertyCString(Key::kVersion, 64, "version"));
std::string contents;
ASSERT_TRUE(LoggingReadEntireFile(path(), &contents));
std::string result("\5\1\0\a\0\0\0\0\0\0\0version", 18);
ASSERT_EQ(contents, result);
}
TEST_F(IOSIntermediateDumpWriterTest, PropertyStringShort) {
EXPECT_TRUE(writer_->Open(path()));
EXPECT_FALSE(
writer_->AddPropertyCString(Key::kVersion, 7, "versionnnnnnnnnnnn"));
}
TEST_F(IOSIntermediateDumpWriterTest, PropertyStringLong) {
EXPECT_TRUE(writer_->Open(path()));
char* bad_string = nullptr;
EXPECT_FALSE(writer_->AddPropertyCString(Key::kVersion, 1025, bad_string));
}
TEST_F(IOSIntermediateDumpWriterTest, MissingPropertyString) {
char* region;
vm_size_t page_size = getpagesize();
vm_size_t region_size = page_size * 2;
ASSERT_EQ(vm_allocate(mach_task_self(),
reinterpret_cast<vm_address_t*>(&region),
region_size,
VM_FLAGS_ANYWHERE),
0);
base::mac::ScopedMachVM vm_owner(reinterpret_cast<vm_address_t>(region),
region_size);
// Fill first page with 'A' and second with 'B'.
memset(region, 'A', page_size);
memset(region + page_size, 'B', page_size);
// Drop a NUL 10 bytes from the end of the first page and into the second
// page.
region[page_size - 10] = '\0';
region[page_size + 10] = '\0';
// Read a string that spans two pages.
EXPECT_TRUE(writer_->Open(path()));
EXPECT_TRUE(
writer_->AddPropertyCString(Key::kVersion, 64, region + page_size - 5));
std::string contents;
ASSERT_TRUE(LoggingReadEntireFile(path(), &contents));
std::string result("\x5\x1\0\xF\0\0\0\0\0\0\0AAAAABBBBBBBBBB", 26);
ASSERT_EQ(contents, result);
// Dealloc second page.
ASSERT_EQ(vm_deallocate(mach_task_self(),
reinterpret_cast<vm_address_t>(region + page_size),
page_size),
0);
// Reading the same string should fail when the next page is dealloc-ed.
EXPECT_FALSE(
writer_->AddPropertyCString(Key::kVersion, 64, region + page_size - 5));
// Ensure we can read the first string without loading the second page.
EXPECT_TRUE(writer_->Open(path()));
EXPECT_TRUE(
writer_->AddPropertyCString(Key::kVersion, 64, region + page_size - 20));
ASSERT_TRUE(LoggingReadEntireFile(path(), &contents));
result.assign("\x5\x1\0\n\0\0\0\0\0\0\0AAAAAAAAAA", 21);
ASSERT_EQ(contents, result);
}
TEST_F(IOSIntermediateDumpWriterTest, BadProperty) {
EXPECT_TRUE(writer_->Open(path()));
ASSERT_FALSE(writer_->AddProperty(Key::kVersion, "version", -1));