Skip to content

Commit

Permalink
Addressing a potential integer underflow in minidump.cc and stackwalk…
Browse files Browse the repository at this point in the history
…er_arm64.cc

Also, defining __STDC_FORMAT_MACROS before including <inttypes.h>

Change-Id: Ia25c4353412ca70512efef5e98670687ab575750
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5310977
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
  • Loading branch information
Ivan Penkov committed Feb 21, 2024
1 parent b48a2d4 commit f032e4c
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 3 deletions.
8 changes: 7 additions & 1 deletion src/processor/minidump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,19 @@
//
// Author: Mark Mentovai

// For <inttypes.h> PRI* macros, before anything else might #include it.
#ifndef __STDC_FORMAT_MACROS
#define __STDC_FORMAT_MACROS
#endif /* __STDC_FORMAT_MACROS */

#ifdef HAVE_CONFIG_H
#include <config.h> // Must come first
#endif

#include "google_breakpad/processor/minidump.h"

#include <assert.h>
#include <cstdint>
#include <fcntl.h>
#include <inttypes.h>
#include <stddef.h>
Expand Down Expand Up @@ -820,7 +826,7 @@ bool MinidumpContext::Read(uint32_t expected_size) {
// Context may include xsave registers and so be larger than
// sizeof(MDRawContextX86). For now we skip this extended data.
if (context_flags & MD_CONTEXT_X86_XSTATE) {
size_t bytes_left = expected_size - sizeof(MDRawContextX86);
int64_t bytes_left = expected_size - sizeof(MDRawContextX86);
if (bytes_left > kMaxXSaveAreaSize) {
BPLOG(ERROR) << "MinidumpContext oversized xstate area";
return false;
Expand Down
3 changes: 2 additions & 1 deletion src/processor/proc_maps_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// For <inttypes.h> PRI* macros, before anything else might #include it.
#ifndef __STDC_FORMAT_MACROS
#define __STDC_FORMAT_MACROS
#endif
#endif /* __STDC_FORMAT_MACROS */

#ifdef HAVE_CONFIG_H
#include <config.h> // Must come first
Expand Down
4 changes: 4 additions & 0 deletions src/processor/range_map_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
//
// Author: Mark Mentovai

// For <inttypes.h> PRI* macros, before anything else might #include it.
#ifndef __STDC_FORMAT_MACROS
#define __STDC_FORMAT_MACROS
#endif /* __STDC_FORMAT_MACROS */

#ifdef HAVE_CONFIG_H
#include <config.h> // Must come first
Expand Down
7 changes: 6 additions & 1 deletion src/processor/stackwalker_arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <config.h> // Must come first
#endif

#include <cstdint>
#include <vector>

#include "common/scoped_ptr.h"
Expand Down Expand Up @@ -269,11 +270,15 @@ void StackwalkerARM64::CorrectRegLRByFramePointer(

// 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;
int64_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--;
}
// last_frame_callee_id should not become negative because at the top of the
// stack trace we always have a context frame (FRAME_TRUST_CONTEXT) so the
// above loop should end before last_frame_callee_id gets negative. But we are
// being extra defensive here and bail if it ever becomes negative.
if (last_frame_callee_id < 0) return;
StackFrameARM64* last_frame_callee =
static_cast<StackFrameARM64*>(frames[last_frame_callee_id]);
Expand Down

0 comments on commit f032e4c

Please sign in to comment.