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

linux: Make MemoryMap retry when duplicates are detected

Browse Source 
When the /proc/pid/maps file is not read atomically and the target
process is actively mapping memory, entries can be read multiple times
or missed entirely. This change makes MemoryMap read the whole contents
of the maps file before attempting to parse it as well as check for
duplication/overlap errors, retrying on failure. This change also adds
ptrace attachements to unit tests to reflect actual intended usage.

Bug: crashpad:30
Change-Id: Ie8549548e25c47baa418ee7439d82743f84ff41e
Reviewed-on: https://chromium-review.googlesource.com/491950
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Joshua Peraza 2017-05-02 10:07:21 -07:00 committed by Commit Bot
parent 8af3203d81
commit 51779ce639
2 changed files with 166 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

265
util/linux/memory_map.cc
View File

@ -22,7 +22,8 @@
#include "base/logging.h"
#include "build/build_config.h"
#include "util/file/delimited_file_reader.h"
#include "util/file/file_reader.h"
#include "util/file/file_io.h"
#include "util/file/string_file.h"
#include "util/stdlib/string_number_conversion.h"
namespace crashpad {
@ -56,6 +57,134 @@ bool HexStringToNumber(const std::string& string, Type* number) {
return LocalStringToNumber("0x" + string, number);
}
// The result from parsing a line from the maps file.
enum class ParseResult {
// A line was successfully parsed.
kSuccess = 0,
// The end of the file was successfully reached.
kEndOfFile,
// There was an error in the file, likely because it was read non-atmoically.
// We should try to read it again.
kRetry,
// An error with a message logged.
kError
};
// Reads a line from a maps file being read by maps_file_reader and extends
// mappings with a new MemoryMap::Mapping describing the line.
ParseResult ParseMapsLine(DelimitedFileReader* maps_file_reader,
std::vector<MemoryMap::Mapping>* mappings) {
std::string field;
LinuxVMAddress start_address;
switch (maps_file_reader->GetDelim('-', &field)) {
case DelimitedFileReader::Result::kError:
return ParseResult::kError;
case DelimitedFileReader::Result::kEndOfFile:
return ParseResult::kEndOfFile;
case DelimitedFileReader::Result::kSuccess:
field.pop_back();
if (!HexStringToNumber(field, &start_address)) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
if (!mappings->empty() && start_address < mappings->back().range.End()) {
return ParseResult::kRetry;
}
}
LinuxVMAddress end_address;
if (maps_file_reader->GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !HexStringToNumber(field, &end_address))) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
if (end_address <= start_address) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
// TODO(jperaza): set bitness properly
#if defined(ARCH_CPU_64_BITS)
const bool is_64_bit = true;
#else
const bool is_64_bit = false;
#endif
MemoryMap::Mapping mapping;
mapping.range.SetRange(is_64_bit, start_address, end_address - start_address);
if (maps_file_reader->GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), field.size() != 4)) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
#define SET_FIELD(actual_c, outval, true_chars, false_chars) \
do { \
if (strchr(true_chars, actual_c)) { \
*outval = true; \
} else if (strchr(false_chars, actual_c)) { \
*outval = false; \
} else { \
LOG(ERROR) << "format error"; \
return ParseResult::kError; \
} \
} while (false)
SET_FIELD(field[0], &mapping.readable, "r", "-");
SET_FIELD(field[1], &mapping.writable, "w", "-");
SET_FIELD(field[2], &mapping.executable, "x", "-");
SET_FIELD(field[3], &mapping.shareable, "sS", "p");
#undef SET_FIELD
if (maps_file_reader->GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !HexStringToNumber(field, &mapping.offset))) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
int major, minor;
if (maps_file_reader->GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), field.size() != 5) ||
!HexStringToNumber(field.substr(0, 2), &major) ||
!HexStringToNumber(field.substr(3, 2), &minor)) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
mapping.device = MKDEV(major, minor);
if (maps_file_reader->GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !LocalStringToNumber(field, &mapping.inode))) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
if (maps_file_reader->GetDelim('\n', &field) !=
DelimitedFileReader::Result::kSuccess) {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
if (field.back() != '\n') {
LOG(ERROR) << "format error";
return ParseResult::kError;
}
field.pop_back();
mappings->push_back(mapping);
size_t path_start = field.find_first_not_of(' ');
if (path_start != std::string::npos) {
mappings->back().name = field.substr(path_start);
}
return ParseResult::kSuccess;
}
} // namespace
MemoryMap::Mapping::Mapping()
@ -76,122 +205,44 @@ MemoryMap::~MemoryMap() {}
bool MemoryMap::Initialize(pid_t pid) {
INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
// If the maps file is not read atomically, entries can be read multiple times
// or missed entirely. The kernel reads entries from this file into a page
// sized buffer, so maps files larger than a page require multiple reads.
// Attempt to reduce the time between reads by reading the entire file into a
// StringFile before attempting to parse it. If ParseMapsLine detects
// duplicate, overlapping, or out-of-order entries, it will trigger restarting
// the read up to |attempts| times.
int attempts = 3;
do {
std::string contents;
char path[32];
snprintf(path, sizeof(path), "/proc/%d/maps", pid);
FileReader maps_file;
if (!maps_file.Open(base::FilePath(path))) {
if (!LoggingReadEntireFile(base::FilePath(path), &contents)) {
return false;
}
StringFile maps_file;
maps_file.SetString(contents);
DelimitedFileReader maps_file_reader(&maps_file);
DelimitedFileReader::Result result;
std::string field;
while ((result = maps_file_reader.GetDelim('-', &field)) ==
DelimitedFileReader::Result::kSuccess) {
field.pop_back();
LinuxVMAddress start_address;
if (!HexStringToNumber(field, &start_address)) {
LOG(ERROR) << "format error";
return false;
ParseResult result;
while ((result = ParseMapsLine(&maps_file_reader, &mappings_)) ==
ParseResult::kSuccess) {
}
LinuxVMAddress end_address;
if (maps_file_reader.GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !HexStringToNumber(field, &end_address))) {
LOG(ERROR) << "format error";
return false;
}
if (end_address <= start_address) {
LOG(ERROR) << "format error";
return false;
}
// TODO(jperaza): set bitness properly
#if defined(ARCH_CPU_64_BITS)
const bool is_64_bit = true;
#else
const bool is_64_bit = false;
#endif
Mapping mapping;
mapping.range.SetRange(
is_64_bit, start_address, end_address - start_address);
if (maps_file_reader.GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), field.size() != 4)) {
LOG(ERROR) << "format error";
return false;
}
#define SET_FIELD(actual_c, outval, true_chars, false_chars) \
do { \
if (strchr(true_chars, actual_c)) { \
*outval = true; \
} else if (strchr(false_chars, actual_c)) { \
*outval = false; \
} else { \
LOG(ERROR) << "format error"; \
return false; \
} \
} while (false)
SET_FIELD(field[0], &mapping.readable, "r", "-");
SET_FIELD(field[1], &mapping.writable, "w", "-");
SET_FIELD(field[2], &mapping.executable, "x", "-");
SET_FIELD(field[3], &mapping.shareable, "sS", "p");
#undef SET_FIELD
if (maps_file_reader.GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !HexStringToNumber(field, &mapping.offset))) {
LOG(ERROR) << "format error";
return false;
}
int major, minor;
if (maps_file_reader.GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), field.size() != 5) ||
!HexStringToNumber(field.substr(0, 2), &major) ||
!HexStringToNumber(field.substr(3, 2), &minor)) {
LOG(ERROR) << "format error";
return false;
}
mapping.device = MKDEV(major, minor);
if (maps_file_reader.GetDelim(' ', &field) !=
DelimitedFileReader::Result::kSuccess ||
(field.pop_back(), !LocalStringToNumber(field, &mapping.inode))) {
LOG(ERROR) << "format error";
return false;
}
if (maps_file_reader.GetDelim('\n', &field) !=
DelimitedFileReader::Result::kSuccess) {
LOG(ERROR) << "format error";
return false;
}
if (field.back() != '\n') {
LOG(ERROR) << "format error";
return false;
}
field.pop_back();
mappings_.push_back(mapping);
size_t path_start = field.find_first_not_of(' ');
if (path_start != std::string::npos) {
mappings_.back().name = field.substr(path_start);
}
}
if (result != DelimitedFileReader::Result::kEndOfFile) {
return false;
}
if (result == ParseResult::kEndOfFile) {
INITIALIZATION_STATE_SET_VALID(initialized_);
return true;
}
if (result == ParseResult::kError) {
return false;
}
DCHECK(result == ParseResult::kRetry);
} while (--attempts > 0);
LOG(ERROR) << "retry count exceeded";
return false;
}
const MemoryMap::Mapping* MemoryMap::FindMapping(LinuxVMAddress address) const {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);

7
util/linux/memory_map_test.cc
View File

@ -28,6 +28,7 @@
#include "test/multiprocess.h"
#include "test/scoped_temp_dir.h"
#include "util/file/file_io.h"
#include "util/linux/scoped_ptrace_attach.h"
#include "util/misc/clock.h"
#include "util/misc/from_pointer_cast.h"
#include "util/posix/scoped_mmap.h"
@ -104,6 +105,9 @@ class MapChildTest : public Multiprocess {
std::string mapped_file_name(path_length, std::string::value_type());
CheckedReadFileExactly(ReadPipeHandle(), &mapped_file_name[0], path_length);
ScopedPtraceAttach attachment;
attachment.ResetAttach(ChildPID());
MemoryMap map;
ASSERT_TRUE(map.Initialize(ChildPID()));
@ -279,6 +283,9 @@ class MapRunningChildTest : public Multiprocess {
// Let the child get back to its work
SleepNanoseconds(1000);
ScopedPtraceAttach attachment;
attachment.ResetAttach(ChildPID());
MemoryMap map;
ASSERT_TRUE(map.Initialize(ChildPID()));