fuchsia: Don't capture incorrect/unreasonably large stacks

In a stack overflow test from the Fuchsia tree, an intentional crash was
being induced that at the point it was reported to Crashpad resulted in
a stack pointer outside of the stack. This caused two problems:

- Crashpad attempted to capture that whole "thing" which could have been
  anything, and in the failing test was a 1G guard region
- The whole wrong thing could be very large, resulting in OOM when
  trying to write the minidump, which was the symptom of the bug.

Don't attempt to continue of SP isn't at least in a mapping, and don't
capture too-large regions for the stack as nothing useful can come of
that anyway.

New test added: ProcessSnapshotFuchsiaTest.InvalidStackPointer.

Bug: fuchsia:41212
Change-Id: Ifb48fd8b4b5b2f0cf10ab97e01dbd8b842368775
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1912942
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Francois Rousseau <frousseau@google.com>

snapshot/fuchsia/process_reader_fuchsia.cc

@@ -53,7 +53,9 @@ void GetStackRegions(
   }
 
   if (range_with_sp.type != ZX_INFO_MAPS_TYPE_MAPPING) {
-    LOG(ERROR) << "stack range has unexpected type, continuing anyway";
+    LOG(ERROR) << "stack range has unexpected type " << range_with_sp.type
+               << ", aborting";
+    return;
   }
 
   if (range_with_sp.u.mapping.mmu_flags & ZX_VM_PERM_EXECUTE) {
@@ -75,8 +77,16 @@ void GetStackRegions(
                range_with_sp.base);
   const size_t region_size =
       range_with_sp.size - (start_address - range_with_sp.base);
+
+  // Because most Fuchsia processes use safestack, it is very unlikely that a
+  // stack this large would be valid. Even if it were, avoid creating
+  // unreasonably large dumps by artificially limiting the captured amount.
+  constexpr uint64_t kMaxStackCapture = 1048576u;
+  LOG_IF(ERROR, region_size > kMaxStackCapture)
+      << "clamping unexpectedly large stack capture of " << region_size;
+  const size_t clamped_region_size = std::min(region_size, kMaxStackCapture);
   stack_regions->push_back(
-      CheckedRange<zx_vaddr_t, size_t>(start_address, region_size));
+      CheckedRange<zx_vaddr_t, size_t>(start_address, clamped_region_size));
 
   // TODO(scottmg): https://crashpad.chromium.org/bug/196, once the retrievable
   // registers include FS and similar for ARM, retrieve the region for the

snapshot/fuchsia/process_snapshot_fuchsia_test.cc

@@ -24,6 +24,7 @@
 #include "gtest/gtest.h"
 #include "snapshot/fuchsia/process_snapshot_fuchsia.h"
 #include "test/multiprocess_exec.h"
+#include "util/fuchsia/koid_utilities.h"
 #include "util/fuchsia/scoped_task_suspend.h"
 
 namespace crashpad {
@@ -139,6 +140,96 @@ TEST(ProcessSnapshotFuchsiaTest, AddressSpaceMapping) {
   test.Run();
 }
 
+CRASHPAD_CHILD_TEST_MAIN(StackPointerIntoInvalidLocation) {
+  // Map a large block, output the base address of it, and block. The parent
+  // will artificially set the SP into this large block to confirm that a huge
+  // stack is not accidentally captured.
+  zx_handle_t large_vmo;
+  constexpr uint64_t kSize = 1 << 30u;
+  zx_status_t status = zx_vmo_create(kSize, 0, &large_vmo);
+  ZX_CHECK(status == ZX_OK, status) << "zx_vmo_create";
+  zx_vaddr_t mapped_addr;
+  status = zx_vmar_map(zx_vmar_root_self(),
+                       ZX_VM_PERM_READ | ZX_VM_PERM_WRITE,
+                       0,
+                       large_vmo,
+                       0,
+                       kSize,
+                       &mapped_addr);
+  ZX_CHECK(status == ZX_OK, status) << "zx_vmar_map";
+  CheckedWriteFile(StdioFileHandle(StdioStream::kStandardOutput),
+                   &mapped_addr,
+                   sizeof(mapped_addr));
+  zx_nanosleep(ZX_TIME_INFINITE);
+  return 0;
+}
+
+class InvalidStackPointerTest : public MultiprocessExec {
+ public:
+  InvalidStackPointerTest() : MultiprocessExec() {
+    SetChildTestMainFunction("StackPointerIntoInvalidLocation");
+    SetExpectedChildTermination(kTerminationNormal,
+                                ZX_TASK_RETCODE_SYSCALL_KILL);
+  }
+  ~InvalidStackPointerTest() {}
+
+ private:
+  void MultiprocessParent() override {
+    uint64_t address_of_large_mapping;
+    ASSERT_TRUE(ReadFileExactly(ReadPipeHandle(),
+                                &address_of_large_mapping,
+                                sizeof(address_of_large_mapping)));
+
+    ScopedTaskSuspend suspend(*ChildProcess());
+
+    std::vector<zx::thread> threads = GetThreadHandles(*ChildProcess());
+    ASSERT_EQ(threads.size(), 1u);
+
+    zx_thread_state_general_regs_t regs;
+    ASSERT_EQ(threads[0].read_state(
+                  ZX_THREAD_STATE_GENERAL_REGS, &regs, sizeof(regs)),
+              ZX_OK);
+
+    constexpr uint64_t kOffsetIntoMapping = 1024;
+#if defined(ARCH_CPU_X86_64)
+    regs.rsp = address_of_large_mapping + kOffsetIntoMapping;
+#elif defined(ARCH_CPU_ARM64)
+    regs.sp = address_of_large_mapping + kOffsetIntoMapping;
+#else
+#error
+#endif
+
+    ASSERT_EQ(threads[0].write_state(
+                  ZX_THREAD_STATE_GENERAL_REGS, &regs, sizeof(regs)),
+              ZX_OK);
+
+    ProcessSnapshotFuchsia process_snapshot;
+    ASSERT_TRUE(process_snapshot.Initialize(*ChildProcess()));
+
+    ASSERT_EQ(process_snapshot.Threads().size(), 1u);
+    const MemorySnapshot* stack = process_snapshot.Threads()[0]->Stack();
+    ASSERT_TRUE(stack);
+    // Ensure the stack capture isn't unreasonably large.
+    EXPECT_LT(stack->Size(), 10 * 1048576u);
+
+    // As we've corrupted the child, don't let it run again.
+    ASSERT_EQ(ChildProcess()->kill(), ZX_OK);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(InvalidStackPointerTest);
+};
+
+// This is a test for a specific failure detailed in
+// https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=41212. A test of stack
+// behavior that was intentionally overflowing the stack, and so when Crashpad
+// received the exception the SP did not point into the actual stack. This
+// caused Crashpad to erronously capture the "stack" from the next mapping in
+// the address space (which could be very large, cause OOM, etc.).
+TEST(ProcessSnapshotFuchsiaTest, InvalidStackPointer) {
+  InvalidStackPointerTest test;
+  test.Run();
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad