linux: Use mmap for attachments in PtraceBroker
The broker attempts to use sbrk() to allocate memory to track ptrace
attachments. If the process failed due to an OOM, this system call might
fail, the broker falls back to saving attachments on the stack, and then
overruns the stack.
This change updates the broker to use sys_mmap() instead of sbrk(),
which is expected to work at least as well. If sys_mmap() fails or
the first mapped page is exhausted, further attachments fail without
attempting to save them to the stack.
Bug: chromium:1128441
Change-Id: I731fcde082ce82adc9028aec3ff65f13608135b0
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2488280
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
GitOrigin-RevId: 20cbfa49719cb3a295d51f43a55cf03624cb2bf1
diff --git a/util/linux/ptrace_broker.cc b/util/linux/ptrace_broker.cc
index b6b5bb1..68621f2 100644
--- a/util/linux/ptrace_broker.cc
+++ b/util/linux/ptrace_broker.cc
@@ -17,6 +17,7 @@
#include <fcntl.h>
#include <limits.h>
#include <string.h>
+#include <sys/mman.h>
#include <syscall.h>
#include <unistd.h>
@@ -24,7 +25,11 @@
#include "base/check_op.h"
#include "base/posix/eintr_wrapper.h"
+#include "base/process/process_metrics.h"
+#include "third_party/lss/lss.h"
+#include "util/linux/scoped_ptrace_attach.h"
#include "util/misc/memory_sanitizer.h"
+#include "util/posix/scoped_mmap.h"
namespace crashpad {
@@ -52,18 +57,58 @@
} // namespace
+class PtraceBroker::AttachmentsArray {
+ public:
+ AttachmentsArray() : allocation_(false), attach_count_(0) {}
+
+ ~AttachmentsArray() {
+ for (size_t index = 0; index < attach_count_; ++index) {
+ PtraceDetach(Attachments()[index], false);
+ }
+ }
+
+ bool Initialize() {
+ return allocation_.ResetMmap(nullptr,
+ base::GetPageSize(),
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS,
+ -1,
+ 0);
+ }
+
+ bool Attach(pid_t pid) {
+ pid_t* attach = AllocateAttachment();
+ if (!attach || !PtraceAttach(pid, false)) {
+ return false;
+ }
+
+ *attach = pid;
+ return true;
+ }
+
+ private:
+ pid_t* AllocateAttachment() {
+ if (attach_count_ >= (allocation_.len() / sizeof(pid_t))) {
+ return nullptr;
+ }
+ return &Attachments()[attach_count_++];
+ }
+
+ pid_t* Attachments() { return allocation_.addr_as<pid_t*>(); }
+
+ ScopedMmap allocation_;
+ size_t attach_count_;
+
+ DISALLOW_COPY_AND_ASSIGN(AttachmentsArray);
+};
+
PtraceBroker::PtraceBroker(int sock, pid_t pid, bool is_64_bit)
: ptracer_(is_64_bit, /* can_log= */ false),
file_root_(file_root_buffer_),
- attachments_(nullptr),
- attach_count_(0),
- attach_capacity_(0),
memory_file_(),
sock_(sock),
memory_pid_(pid),
tried_opening_mem_file_(false) {
- AllocateAttachments();
-
static constexpr char kProc[] = "/proc/";
size_t root_length = strlen(kProc);
memcpy(file_root_buffer_, kProc, root_length);
@@ -88,35 +133,12 @@
}
int PtraceBroker::Run() {
- int result = RunImpl();
- ReleaseAttachments();
- return result;
+ AttachmentsArray attachments;
+ attachments.Initialize();
+ return RunImpl(&attachments);
}
-bool PtraceBroker::AllocateAttachments() {
- constexpr size_t page_size = 4096;
- constexpr size_t alloc_size =
- (sizeof(ScopedPtraceAttach) + page_size - 1) & ~(page_size - 1);
- void* alloc = sbrk(alloc_size);
- if (reinterpret_cast<intptr_t>(alloc) == -1) {
- return false;
- }
-
- if (attachments_ == nullptr) {
- attachments_ = reinterpret_cast<ScopedPtraceAttach*>(alloc);
- }
-
- attach_capacity_ += alloc_size / sizeof(ScopedPtraceAttach);
- return true;
-}
-
-void PtraceBroker::ReleaseAttachments() {
- for (size_t index = 0; index < attach_count_; ++index) {
- attachments_[index].Reset();
- }
-}
-
-int PtraceBroker::RunImpl() {
+int PtraceBroker::RunImpl(AttachmentsArray* attachments) {
while (true) {
Request request = {};
if (!ReadFileExactly(sock_, &request, sizeof(request))) {
@@ -129,25 +151,10 @@
switch (request.type) {
case Request::kTypeAttach: {
- ScopedPtraceAttach* attach;
- ScopedPtraceAttach stack_attach;
- bool attach_on_stack = false;
-
- if (attach_capacity_ > attach_count_ || AllocateAttachments()) {
- attach = new (&attachments_[attach_count_]) ScopedPtraceAttach;
- } else {
- attach = &stack_attach;
- attach_on_stack = true;
- }
-
ExceptionHandlerProtocol::Bool status =
- ExceptionHandlerProtocol::kBoolFalse;
- if (attach->ResetAttach(request.tid)) {
- status = ExceptionHandlerProtocol::kBoolTrue;
- if (!attach_on_stack) {
- ++attach_count_;
- }
- }
+ attachments->Attach(request.tid)
+ ? ExceptionHandlerProtocol::kBoolTrue
+ : ExceptionHandlerProtocol::kBoolFalse;
if (!WriteFile(sock_, &status, sizeof(status))) {
return errno;
@@ -160,9 +167,6 @@
}
}
- if (attach_on_stack && status == ExceptionHandlerProtocol::kBoolTrue) {
- return RunImpl();
- }
continue;
}
diff --git a/util/linux/ptrace_broker.h b/util/linux/ptrace_broker.h
index 6a7bfb7..5d90cb2 100644
--- a/util/linux/ptrace_broker.h
+++ b/util/linux/ptrace_broker.h
@@ -24,7 +24,6 @@
#include "util/linux/exception_handler_protocol.h"
#include "util/linux/ptrace_connection.h"
#include "util/linux/ptracer.h"
-#include "util/linux/scoped_ptrace_attach.h"
#include "util/linux/thread_info.h"
#include "util/misc/address_types.h"
@@ -186,16 +185,13 @@
//! This method returns when a PtraceBrokerRequest with type kTypeExit is
//! received or an error is encountered on the socket.
//!
- //! This method calls `sbrk`, which may break other memory management tools,
- //! such as `malloc`.
- //!
//! \return 0 if Run() exited due to an exit request. Otherwise an error code.
int Run();
private:
- bool AllocateAttachments();
- void ReleaseAttachments();
- int RunImpl();
+ class AttachmentsArray;
+
+ int RunImpl(AttachmentsArray*);
int SendError(ExceptionHandlerProtocol::Errno err);
int SendReadError(ReadError err);
int SendOpenResult(OpenResult result);
@@ -210,9 +206,6 @@
char file_root_buffer_[32];
Ptracer ptracer_;
const char* file_root_;
- ScopedPtraceAttach* attachments_;
- size_t attach_count_;
- size_t attach_capacity_;
ScopedFileHandle memory_file_;
int sock_;
pid_t memory_pid_;
diff --git a/util/linux/scoped_ptrace_attach.cc b/util/linux/scoped_ptrace_attach.cc
index 09e3fba..eed8345 100644
--- a/util/linux/scoped_ptrace_attach.cc
+++ b/util/linux/scoped_ptrace_attach.cc
@@ -22,6 +22,32 @@
namespace crashpad {
+bool PtraceAttach(pid_t pid, bool can_log) {
+ if (ptrace(PTRACE_ATTACH, pid, nullptr, nullptr) != 0) {
+ PLOG_IF(ERROR, can_log) << "ptrace";
+ return false;
+ }
+
+ int status;
+ if (HANDLE_EINTR(waitpid(pid, &status, __WALL)) < 0) {
+ PLOG_IF(ERROR, can_log) << "waitpid";
+ return false;
+ }
+ if (!WIFSTOPPED(status)) {
+ LOG_IF(ERROR, can_log) << "process not stopped";
+ return false;
+ }
+ return true;
+}
+
+bool PtraceDetach(pid_t pid, bool can_log) {
+ if (pid >= 0 && ptrace(PTRACE_DETACH, pid, nullptr, nullptr) != 0) {
+ PLOG_IF(ERROR, can_log) << "ptrace";
+ return false;
+ }
+ return true;
+}
+
ScopedPtraceAttach::ScopedPtraceAttach()
: pid_(-1) {}
@@ -30,8 +56,7 @@
}
bool ScopedPtraceAttach::Reset() {
- if (pid_ >= 0 && ptrace(PTRACE_DETACH, pid_, nullptr, nullptr) != 0) {
- PLOG(ERROR) << "ptrace";
+ if (!PtraceDetach(pid_, true)) {
return false;
}
pid_ = -1;
@@ -41,21 +66,11 @@
bool ScopedPtraceAttach::ResetAttach(pid_t pid) {
Reset();
- if (ptrace(PTRACE_ATTACH, pid, nullptr, nullptr) != 0) {
- PLOG(ERROR) << "ptrace";
+ if (!PtraceAttach(pid, true)) {
return false;
}
- pid_ = pid;
- int status;
- if (HANDLE_EINTR(waitpid(pid_, &status, __WALL)) < 0) {
- PLOG(ERROR) << "waitpid";
- return false;
- }
- if (!WIFSTOPPED(status)) {
- LOG(ERROR) << "process not stopped";
- return false;
- }
+ pid_ = pid;
return true;
}
diff --git a/util/linux/scoped_ptrace_attach.h b/util/linux/scoped_ptrace_attach.h
index a3d9d69..f380d25 100644
--- a/util/linux/scoped_ptrace_attach.h
+++ b/util/linux/scoped_ptrace_attach.h
@@ -21,6 +21,24 @@
namespace crashpad {
+//! \brief Attaches to the process with process ID \a pid and blocks until the
+//! target process has stopped by calling `waitpid()`.
+//!
+//! \param pid The process ID of the process to attach to.
+//! \param can_log Whether this function may log messages on failure.
+//! \return `true` on success. `false` on failure with a message logged if \a
+//! can_log is `true`.
+bool PtraceAttach(pid_t pid, bool can_log = true);
+
+//! \brief Detaches the process with process ID \a pid. The process must
+//! already be ptrace attached.
+//!
+//! \param pid The process ID of the process to detach.
+//! \param can_log Whether this function may log messages on failure.
+//! \return `true` on success. `false` on failure with a message logged if \a
+//! ca_log is `true `true`
+bool PtraceDetach(pid_t pid, bool can_log = true);
+
//! \brief Maintains a `ptrace()` attachment to a process.
//!
//! On destruction, the process will be detached.
diff --git a/util/posix/scoped_mmap.cc b/util/posix/scoped_mmap.cc
index 0c98ba2..21533d2 100644
--- a/util/posix/scoped_mmap.cc
+++ b/util/posix/scoped_mmap.cc
@@ -22,12 +22,54 @@
#include "base/logging.h"
#include "base/numerics/safe_conversions.h"
#include "base/numerics/safe_math.h"
+#include "base/process/process_metrics.h"
+#include "build/build_config.h"
+
+#if defined(OS_LINUX)
+#include "third_party/lss/lss.h"
+#endif
namespace {
-bool Munmap(uintptr_t addr, size_t len) {
- if (munmap(reinterpret_cast<void*>(addr), len) != 0) {
- PLOG(ERROR) << "munmap";
+#if defined(OS_LINUX)
+void* CallMmap(void* addr,
+ size_t len,
+ int prot,
+ int flags,
+ int fd,
+ off_t offset) {
+ return sys_mmap(addr, len, prot, flags, fd, offset);
+}
+
+int CallMunmap(void* addr, size_t len) {
+ return sys_munmap(addr, len);
+}
+
+int CallMprotect(void* addr, size_t len, int prot) {
+ return sys_mprotect(addr, len, prot);
+}
+#else
+void* CallMmap(void* addr,
+ size_t len,
+ int prot,
+ int flags,
+ int fd,
+ off_t offset) {
+ return mmap(addr, len, prot, flags, fd, offset);
+}
+
+int CallMunmap(void* addr, size_t len) {
+ return munmap(addr, len);
+}
+
+int CallMprotect(void* addr, size_t len, int prot) {
+ return mprotect(addr, len, prot);
+}
+#endif
+
+bool LoggingMunmap(uintptr_t addr, size_t len, bool can_log) {
+ if (CallMunmap(reinterpret_cast<void*>(addr), len) != 0) {
+ PLOG_IF(ERROR, can_log) << "munmap";
return false;
}
@@ -35,7 +77,7 @@
}
size_t RoundPage(size_t size) {
- const size_t kPageMask = base::checked_cast<size_t>(getpagesize()) - 1;
+ const size_t kPageMask = base::checked_cast<size_t>(base::GetPageSize()) - 1;
return (size + kPageMask) & ~kPageMask;
}
@@ -43,11 +85,12 @@
namespace crashpad {
-ScopedMmap::ScopedMmap() {}
+ScopedMmap::ScopedMmap(bool can_log) : can_log_(can_log) {}
ScopedMmap::~ScopedMmap() {
if (is_valid()) {
- Munmap(reinterpret_cast<uintptr_t>(addr_), RoundPage(len_));
+ LoggingMunmap(
+ reinterpret_cast<uintptr_t>(addr_), RoundPage(len_), can_log_);
}
}
@@ -63,7 +106,7 @@
DCHECK_EQ(len, 0u);
} else {
DCHECK_NE(len, 0u);
- DCHECK_EQ(new_addr % getpagesize(), 0u);
+ DCHECK_EQ(new_addr % base::GetPageSize(), 0u);
DCHECK((base::CheckedNumeric<uintptr_t>(new_addr) + (new_len_round - 1))
.IsValid());
}
@@ -74,11 +117,13 @@
const uintptr_t old_addr = reinterpret_cast<uintptr_t>(addr_);
const size_t old_len_round = RoundPage(len_);
if (old_addr < new_addr) {
- result &= Munmap(old_addr, std::min(old_len_round, new_addr - old_addr));
+ result &= LoggingMunmap(
+ old_addr, std::min(old_len_round, new_addr - old_addr), can_log_);
}
if (old_addr + old_len_round > new_addr + new_len_round) {
uintptr_t unmap_start = std::max(old_addr, new_addr + new_len_round);
- result &= Munmap(unmap_start, old_addr + old_len_round - unmap_start);
+ result &= LoggingMunmap(
+ unmap_start, old_addr + old_len_round - unmap_start, can_log_);
}
}
@@ -100,9 +145,9 @@
// consider the return value from Reset().
Reset();
- void* new_addr = mmap(addr, len, prot, flags, fd, offset);
+ void* new_addr = CallMmap(addr, len, prot, flags, fd, offset);
if (new_addr == MAP_FAILED) {
- PLOG(ERROR) << "mmap";
+ PLOG_IF(ERROR, can_log_) << "mmap";
return false;
}
@@ -113,8 +158,8 @@
}
bool ScopedMmap::Mprotect(int prot) {
- if (mprotect(addr_, RoundPage(len_), prot) < 0) {
- PLOG(ERROR) << "mprotect";
+ if (CallMprotect(addr_, RoundPage(len_), prot) < 0) {
+ PLOG_IF(ERROR, can_log_) << "mprotect";
return false;
}
diff --git a/util/posix/scoped_mmap.h b/util/posix/scoped_mmap.h
index b497d94..12f5cee 100644
--- a/util/posix/scoped_mmap.h
+++ b/util/posix/scoped_mmap.h
@@ -30,7 +30,10 @@
//! will be released by calling `munmap()`.
class ScopedMmap {
public:
- ScopedMmap();
+ //! \brief Constructs this object.
+ //!
+ //! \param can_log `true` if methods of this class may log messages.
+ explicit ScopedMmap(bool can_log = true);
~ScopedMmap();
//! \brief Releases the memory-mapped region by calling `munmap()`.
@@ -105,6 +108,7 @@
private:
void* addr_ = MAP_FAILED;
size_t len_ = 0;
+ bool can_log_;
DISALLOW_COPY_AND_ASSIGN(ScopedMmap);
};
