ProcessMemory: Add ReadUpTo to enable short reads

ProcessMemory::ReadCStringInternal needs to be able to perform short
reads.

Change-Id: I2b2e1c2e6603d01235d8d2dbd15494375cd7f3f6
Reviewed-on: https://chromium-review.googlesource.com/874776
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>

util/process/process_memory.cc

@@ -18,6 +18,25 @@
 
 namespace crashpad {
 
+bool ProcessMemory::Read(VMAddress address, size_t size, void* buffer) const {
+  char* buffer_c = static_cast<char*>(buffer);
+  while (size > 0) {
+    ssize_t bytes_read = ReadUpTo(address, size, buffer_c);
+    if (bytes_read < 0) {
+      return false;
+    }
+    if (bytes_read == 0) {
+      LOG(ERROR) << "short read";
+      return false;
+    }
+    DCHECK_LE(static_cast<size_t>(bytes_read), size);
+    size -= bytes_read;
+    address += bytes_read;
+    buffer_c += bytes_read;
+  }
+  return true;
+}
+
 bool ProcessMemory::ReadCStringInternal(VMAddress address,
                                         bool has_size,
                                         size_t size,
@@ -33,19 +52,23 @@ bool ProcessMemory::ReadCStringInternal(VMAddress address,
       read_size = sizeof(buffer);
     }
 
-    if (!Read(address, read_size, buffer)) {
+    ssize_t bytes_read = ReadUpTo(address, read_size, buffer);
+    if (bytes_read < 0) {
+      return false;
+    }
+    if (bytes_read == 0) {
       break;
     }
 
-    char* nul = static_cast<char*>(memchr(buffer, '\0', read_size));
+    char* nul = static_cast<char*>(memchr(buffer, '\0', bytes_read));
     if (nul != nullptr) {
       string->append(buffer, nul - buffer);
       return true;
     }
-    string->append(buffer, read_size);
+    string->append(buffer, bytes_read);
 
-    address += read_size;
+    address += bytes_read;
-    size -= read_size;
+    size -= bytes_read;
   } while (!has_size || size > 0);
 
   LOG(ERROR) << "unterminated string";

util/process/process_memory.h

@@ -40,7 +40,7 @@ class ProcessMemory {
   //!
   //! \return `true` on success, with \a buffer filled appropriately. `false` on
   //!     failure, with a message logged.
-  virtual bool Read(VMAddress address, size_t size, void* buffer) const = 0;
+  bool Read(VMAddress address, size_t size, void* buffer) const;
 
   //! \brief Reads a `NUL`-terminated C string from the target process into a
   //!     string in the current process.
@@ -83,6 +83,22 @@ class ProcessMemory {
   ~ProcessMemory() = default;
 
  private:
+  //! \brief Copies memory from the target process into a caller-provided buffer
+  //!     in the current process, up to a maximum number of bytes.
+  //!
+  //! \param[in] address The address, in the target process' address space, of
+  //!     the memory region to copy.
+  //! \param[in] size The maximum size, in bytes, of the memory region to copy.
+  //!     \a buffer must be at least this size.
+  //! \param[out] buffer The buffer into which the contents of the other
+  //!     process' memory will be copied.
+  //!
+  //! \return the number of bytes copied, 0 if there is no more data to read, or
+  //!     -1 on failure with a message logged.
+  virtual ssize_t ReadUpTo(VMAddress address,
+                           size_t size,
+                           void* buffer) const = 0;
+
   //! \brief Reads a `NUL`-terminated C string from the target process into a
   //!     string in the current process.
   //!

util/process/process_memory_fuchsia.cc

@@ -16,6 +16,8 @@
 
 #include <zircon/syscalls.h>
 
+#include <limits>
+
 #include "base/logging.h"
 #include "base/fuchsia/fuchsia_logging.h"
 
@@ -33,25 +35,22 @@ bool ProcessMemoryFuchsia::Initialize(zx_handle_t process) {
   return true;
 }
 
-bool ProcessMemoryFuchsia::Read(VMAddress address,
+ssize_t ProcessMemoryFuchsia::ReadUpTo(VMAddress address,
-                                size_t size,
+                                       size_t size,
-                                void* buffer) const {
+                                       void* buffer) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  DCHECK_LE(size, size_t{std::numeric_limits<ssize_t>::max()});
+
   size_t actual;
   zx_status_t status =
       zx_process_read_memory(process_, address, buffer, size, &actual);
 
   if (status != ZX_OK) {
     ZX_LOG(ERROR, status) << "zx_process_read_memory";
-    return false;
+    return -1;
   }
 
-  if (actual != size) {
+  return actual;
-    LOG(ERROR) << "zx_process_read_memory: short read";
-    return false;
-  }
-
-  return true;
 }
 
 }  // namespace crashpad

util/process/process_memory_fuchsia.h

@@ -42,9 +42,9 @@ class ProcessMemoryFuchsia final : public ProcessMemory {
   //! \return `true` on success, `false` on failure with a message logged.
   bool Initialize(zx_handle_t process);
 
-  bool Read(VMAddress address, size_t size, void* buffer) const override;
-
  private:
+  ssize_t ReadUpTo(VMAddress address, size_t size, void* buffer) const override;
+
   zx_handle_t process_;
   InitializationStateDcheck initialized_;
 

util/process/process_memory_linux.cc

@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <limits>
 
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
@@ -45,30 +46,19 @@ bool ProcessMemoryLinux::Initialize(pid_t pid) {
   return true;
 }
 
-bool ProcessMemoryLinux::Read(VMAddress address,
+ssize_t ProcessMemoryLinux::ReadUpTo(VMAddress address,
-                              size_t size,
+                                     size_t size,
-                              void* buffer) const {
+                                     void* buffer) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   DCHECK(mem_fd_.is_valid());
+  DCHECK_LE(size, size_t{std::numeric_limits<ssize_t>::max()});
 
-  char* buffer_c = static_cast<char*>(buffer);
+  ssize_t bytes_read =
-  while (size > 0) {
+      HANDLE_EINTR(pread64(mem_fd_.get(), buffer, size, address));
-    ssize_t bytes_read =
+  if (bytes_read < 0) {
-        HANDLE_EINTR(pread64(mem_fd_.get(), buffer_c, size, address));
+    PLOG(ERROR) << "pread64";
-    if (bytes_read < 0) {
-      PLOG(ERROR) << "pread64";
-      return false;
-    }
-    if (bytes_read == 0) {
-      LOG(ERROR) << "unexpected eof";
-      return false;
-    }
-    DCHECK_LE(static_cast<size_t>(bytes_read), size);
-    size -= bytes_read;
-    address += bytes_read;
-    buffer_c += bytes_read;
   }
-  return true;
+  return bytes_read;
 }
 
 }  // namespace crashpad

util/process/process_memory_linux.h

@@ -44,9 +44,9 @@ class ProcessMemoryLinux final : public ProcessMemory {
   //! \return `true` on success, `false` on failure with a message logged.
   bool Initialize(pid_t pid);
 
-  bool Read(VMAddress address, size_t size, void* buffer) const override;
-
  private:
+  ssize_t ReadUpTo(VMAddress address, size_t size, void* buffer) const override;
+
   base::ScopedFD mem_fd_;
   pid_t pid_;
   InitializationStateDcheck initialized_;