From bcab7ad54c2ed7df41f22de117657c1ade308434 Mon Sep 17 00:00:00 2001
From: Joshua Peraza <jperaza@chromium.org>
Date: Thu, 5 Dec 2019 12:36:26 -0800
Subject: [PATCH] linux: handle large mapped files

Chrome on Android normally builds the handler without large file
support because support for large files varies by API level and NDK
version.

https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?rcl=6b5017edcd8544acbdb157086a1645ce36c03057&l=360

https://android.googlesource.com/platform/bionic/+/master/docs/32-bit-abi.md#32_bit-and

The handler still needs to able to handle large files mapped by other
code modules.

Bug: crashpad:312
Change-Id: I1022b706797f41445650f82c425a92e6e2308618
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1954426
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
---
 util/linux/memory_map.cc      | 28 ++--------------------------
 util/linux/memory_map.h       |  2 +-
 util/linux/memory_map_test.cc | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/util/linux/memory_map.cc b/util/linux/memory_map.cc
index 17fd7a3d..0890cd61 100644
--- a/util/linux/memory_map.cc
+++ b/util/linux/memory_map.cc
@@ -31,33 +31,9 @@ namespace crashpad {
 
 namespace {
 
-// This function is used in this file specfically for signed or unsigned longs.
-// longs are typically either int or int64 sized, but pointers to longs are not
-// automatically coerced to pointers to ints when they are the same size.
-// Simply adding a StringToNumber for longs doesn't work since sometimes long
-// and int64_t are actually the same type, resulting in a redefinition error.
-template <typename Type>
-bool LocalStringToNumber(const std::string& string, Type* number) {
-  static_assert(sizeof(Type) == sizeof(int) || sizeof(Type) == sizeof(int64_t),
-                "Unexpected Type size");
-
-  char data[sizeof(Type)];
-  if (sizeof(Type) == sizeof(int)) {
-    if (!StringToNumber(string, reinterpret_cast<unsigned int*>(data))) {
-      return false;
-    }
-  } else {
-    if (!StringToNumber(string, reinterpret_cast<uint64_t*>(data))) {
-      return false;
-    }
-  }
-  *number = bit_cast<Type>(data);
-  return true;
-}
-
 template <typename Type>
 bool HexStringToNumber(const std::string& string, Type* number) {
-  return LocalStringToNumber("0x" + string, number);
+  return StringToNumber("0x" + string, number);
 }
 
 // The result from parsing a line from the maps file.
@@ -182,7 +158,7 @@ ParseResult ParseMapsLine(DelimitedFileReader* maps_file_reader,
 
   if (maps_file_reader->GetDelim(' ', &field) !=
           DelimitedFileReader::Result::kSuccess ||
-      (field.pop_back(), !LocalStringToNumber(field, &mapping.inode))) {
+      (field.pop_back(), !StringToNumber(field, &mapping.inode))) {
     LOG(ERROR) << "format error";
     return ParseResult::kError;
   }
diff --git a/util/linux/memory_map.h b/util/linux/memory_map.h
index a062b560..d43b7af4 100644
--- a/util/linux/memory_map.h
+++ b/util/linux/memory_map.h
@@ -43,7 +43,7 @@ class MemoryMap {
 
     std::string name;
     CheckedLinuxAddressRange range;
-    off_t offset;
+    off64_t offset;
     dev_t device;
     ino_t inode;
     bool readable;
diff --git a/util/linux/memory_map_test.cc b/util/linux/memory_map_test.cc
index 2df44b3a..20e5f227 100644
--- a/util/linux/memory_map_test.cc
+++ b/util/linux/memory_map_test.cc
@@ -29,7 +29,9 @@
 #include "test/linux/fake_ptrace_connection.h"
 #include "test/multiprocess.h"
 #include "test/scoped_temp_dir.h"
+#include "third_party/lss/lss.h"
 #include "util/file/file_io.h"
+#include "util/file/scoped_remove_file.h"
 #include "util/linux/direct_ptrace_connection.h"
 #include "util/misc/clock.h"
 #include "util/misc/from_pointer_cast.h"
@@ -39,6 +41,38 @@ namespace crashpad {
 namespace test {
 namespace {
 
+TEST(MemoryMap, SelfLargeFiles) {
+  // This test is meant to test the handler's ability to understand files
+  // mapped from large offsets, even if the handler wasn't built with
+  // _FILE_OFFSET_BITS=64. ScopedTempDir needs to stat files to determine
+  // whether to recurse into directories, which may will fail without large file
+  // support. ScopedRemoveFile doesn't have that restriction.
+  ScopedTempDir dir;
+  ScopedRemoveFile large_file_path(dir.path().Append("crashpad_test_file"));
+  ScopedFileHandle handle(
+      LoggingOpenFileForReadAndWrite(large_file_path.get(),
+                                     FileWriteMode::kCreateOrFail,
+                                     FilePermissions::kWorldReadable));
+  ASSERT_TRUE(handle.is_valid());
+
+  // sys_fallocate supports large files as long as the kernel supports them,
+  // regardless of _FILE_OFFSET_BITS.
+  off64_t off = 1llu + UINT32_MAX;
+  ASSERT_EQ(sys_fallocate(handle.get(), 0, off, getpagesize()), 0)
+      << ErrnoMessage("fallocate");
+
+  ScopedMmap mapping;
+  void* addr = sys_mmap(
+      nullptr, getpagesize(), PROT_READ, MAP_SHARED, handle.get(), off);
+  ASSERT_TRUE(addr);
+  ASSERT_TRUE(mapping.ResetAddrLen(addr, getpagesize()));
+
+  FakePtraceConnection connection;
+  ASSERT_TRUE(connection.Initialize(getpid()));
+  MemoryMap map;
+  ASSERT_TRUE(map.Initialize(&connection));
+}
+
 TEST(MemoryMap, SelfBasic) {
   ScopedMmap mmapping;
   ASSERT_TRUE(mmapping.ResetMmap(nullptr,