commit f49c2f1a2023da0cb055874fba050563dfea57db
author Ivan Penkov <ivanpe@chromium.org> Wed Sep 06 18:21:48 2023
committer Joshua Peraza <jperaza@chromium.org> Wed Sep 06 20:16:57 2023
tree 733f09b7df3bfff6679e0ce244e11fe7ad9e8f18
parent e35d1d09ae73eb18e28d11881ec724c279964991

The ARM64 stack walker was doing an illegal down cast from base-class (StackFrame) to derived-class (StackFrameARM64).

Inline frames are always of the base-class type (StackFrame). Treating them as derived-class and accessing members is causing buffer overflows.

Change-Id: Ib41b74256e6162e7d2b14ca3905dfaf5591b9c86
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4847317
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

src/processor/stackwalker_arm64.cc

diff --git a/src/processor/stackwalker_arm64.cc b/src/processor/stackwalker_arm64.cc
index 9c09835..0fa02e0 100644
--- a/src/processor/stackwalker_arm64.cc
+++ b/src/processor/stackwalker_arm64.cc
@@ -267,15 +267,25 @@
           last_frame->context.iregs[MD_CONTEXT_ARM64_REG_SP])
     return;
 
-  StackFrameARM64* last_last_frame =
-      static_cast<StackFrameARM64*>(*(frames.end() - 2));
-  uint64_t last_last_fp =
-      last_last_frame->context.iregs[MD_CONTEXT_ARM64_REG_FP];
+  // Searching for a real callee frame. Skipping inline frames since they
+  // don't contain context (and cannot be downcasted to StackFrameARM64).
+  size_t last_frame_callee_id = frames.size() - 2;
+  while (last_frame_callee_id >= 0 && frames[last_frame_callee_id]->trust ==
+                                          StackFrame::FRAME_TRUST_INLINE) {
+    last_frame_callee_id--;
+  }
+  if (last_frame_callee_id < 0) return;
+  StackFrameARM64* last_frame_callee =
+      static_cast<StackFrameARM64*>(frames[last_frame_callee_id]);
+
+  uint64_t last_frame_callee_fp =
+      last_frame_callee->context.iregs[MD_CONTEXT_ARM64_REG_FP];
 
   uint64_t last_fp = 0;
-  if (last_last_fp && !memory_->GetMemoryAtAddress(last_last_fp, &last_fp)) {
-    BPLOG(ERROR) << "Unable to read last_fp from last_last_fp: 0x"
-                 << std::hex << last_last_fp;
+  if (last_frame_callee_fp &&
+      !memory_->GetMemoryAtAddress(last_frame_callee_fp, &last_fp)) {
+    BPLOG(ERROR) << "Unable to read last_fp from last_last_fp: 0x" << std::hex
+                 << last_frame_callee_fp;
     return;
   }
   // Give up if STACK CFI doesn't agree with frame pointer.
@@ -283,9 +293,10 @@
     return;
 
   uint64_t last_lr = 0;
-  if (last_last_fp && !memory_->GetMemoryAtAddress(last_last_fp + 8, &last_lr)) {
+  if (last_frame_callee_fp &&
+      !memory_->GetMemoryAtAddress(last_frame_callee_fp + 8, &last_lr)) {
     BPLOG(ERROR) << "Unable to read last_lr from (last_last_fp + 8): 0x"
-                 << std::hex << (last_last_fp + 8);
+                 << std::hex << (last_frame_callee_fp + 8);
     return;
   }
   last_lr = PtrauthStrip(last_lr);