-
-
Notifications
You must be signed in to change notification settings - Fork 204
fix(native): Fix smaller findings for Linux minidump writer #1694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
mujacica
wants to merge
7
commits into
master
Choose a base branch
from
fix/native-minidump-linux
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
976905b
fix(native): Fix smaller findings for Linux minidump writer
mujacica 82219ea
Add missing streams
mujacica 32a4e24
Fix CHANGELOG.md
mujacica 5fd69fb
Fix format
mujacica f2a845e
Fix build errors
mujacica c5d4fda
Fix comments + indirect memory
mujacica 6fca3dd
Fixes
mujacica File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
192 changes: 192 additions & 0 deletions
192
src/backends/native/minidump/sentry_minidump_indirect.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| #include "sentry_boot.h" | ||
|
|
||
| #include "sentry_minidump_indirect.h" | ||
|
|
||
| #include "sentry_alloc.h" | ||
| #include "sentry_logger.h" | ||
| #include "sentry_minidump_common.h" | ||
|
|
||
| #include <stdint.h> | ||
| #include <string.h> | ||
|
|
||
| void | ||
| sentry__indirect_init(sentry_indirect_accumulator_t *acc) | ||
| { | ||
| acc->region_count = 0; | ||
| acc->total_bytes = 0; | ||
| } | ||
|
|
||
| /** | ||
| * Binary-search the sorted region list for the first entry whose `end` is | ||
| * strictly greater than `target`. The candidate at index `lo` (if any) | ||
| * is the only one that can possibly overlap [target, target+len). | ||
| * | ||
| * Returns region_count when no such entry exists (target is past the last). | ||
| */ | ||
| static size_t | ||
| find_first_after(const sentry_indirect_accumulator_t *acc, uint64_t target) | ||
| { | ||
| size_t lo = 0; | ||
| size_t hi = acc->region_count; | ||
| while (lo < hi) { | ||
| size_t mid = lo + (hi - lo) / 2; | ||
| if (acc->regions[mid].end <= target) { | ||
| lo = mid + 1; | ||
| } else { | ||
| hi = mid; | ||
| } | ||
| } | ||
| return lo; | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if [start, end) overlaps any region already in the | ||
| * accumulator. The list is sorted, so we only need to check the first | ||
| * region whose end > start — anything earlier ended before us, anything | ||
| * later starts after we'd want it to. | ||
| */ | ||
| static bool | ||
| overlaps_existing( | ||
| const sentry_indirect_accumulator_t *acc, uint64_t start, uint64_t end) | ||
| { | ||
| size_t i = find_first_after(acc, start); | ||
| if (i >= acc->region_count) { | ||
| return false; | ||
| } | ||
| return acc->regions[i].start < end; | ||
| } | ||
|
|
||
| /** | ||
| * Insert a new region at the sorted position. Caller must have verified | ||
| * via overlaps_existing() that no overlap exists, and via the cap checks | ||
| * that there's room. | ||
| */ | ||
| static void | ||
| insert_sorted(sentry_indirect_accumulator_t *acc, | ||
| const sentry_indirect_region_t *new_region) | ||
| { | ||
| size_t i = find_first_after(acc, new_region->start); | ||
| if (i < acc->region_count) { | ||
| memmove(&acc->regions[i + 1], &acc->regions[i], | ||
| (acc->region_count - i) * sizeof(*acc->regions)); | ||
| } | ||
| acc->regions[i] = *new_region; | ||
| acc->region_count++; | ||
| acc->total_bytes += new_region->size; | ||
| } | ||
|
|
||
| void | ||
| sentry__indirect_consider(sentry_indirect_accumulator_t *acc, | ||
| minidump_writer_base_t *writer, uint64_t addr, | ||
| const sentry_indirect_ops_t *ops) | ||
| { | ||
| // Cap checks first — they're the cheapest filters and short-circuit the | ||
| // mapping lookup once we've spent the budget. | ||
| if (acc->region_count >= SENTRY_INDIRECT_MAX_REGIONS) { | ||
| return; | ||
| } | ||
| if (acc->total_bytes >= SENTRY_INDIRECT_MAX_TOTAL_BYTES) { | ||
| return; | ||
| } | ||
|
|
||
| // Cheap rejects before paying for is_writable_heap. | ||
| // - 0/low values: NULLs and small ints | ||
| // - very high bits set: kernel addresses (canonical AArch64/x86_64 user | ||
| // space tops out below this) | ||
| if (addr < SENTRY_INDIRECT_PAGE_SIZE) { | ||
| return; | ||
| } | ||
| if ((addr >> 56) != 0) { | ||
| return; | ||
| } | ||
|
|
||
| if (!ops->is_writable_heap(ops->ctx, addr)) { | ||
| return; | ||
| } | ||
|
|
||
| // Page-align down so the captured region covers the page containing the | ||
| // pointee (and we can dedup multiple pointers landing in the same page). | ||
| // Capture is roughly centered on the pointer but always starts on a page | ||
| // boundary so adjacent allocations get covered too. | ||
| uint64_t centered = addr - (SENTRY_INDIRECT_PER_POINTER_BYTES / 2); | ||
| uint64_t start = centered & ~((uint64_t)SENTRY_INDIRECT_PAGE_SIZE - 1); | ||
| uint64_t end = start + SENTRY_INDIRECT_PER_POINTER_BYTES; | ||
| // Round end up to page boundary too — small bump but keeps reads aligned. | ||
| end = (end + SENTRY_INDIRECT_PAGE_SIZE - 1) | ||
| & ~((uint64_t)SENTRY_INDIRECT_PAGE_SIZE - 1); | ||
|
|
||
| // Trim against the global byte budget so a candidate near the cap doesn't | ||
| // blow it. | ||
| size_t want = (size_t)(end - start); | ||
| size_t remaining = SENTRY_INDIRECT_MAX_TOTAL_BYTES - acc->total_bytes; | ||
| if (want > remaining) { | ||
| end = start + remaining; | ||
| if (end <= start) { | ||
| return; | ||
| } | ||
| want = (size_t)(end - start); | ||
| } | ||
|
|
||
| if (overlaps_existing(acc, start, end)) { | ||
| return; | ||
| } | ||
|
|
||
| // Read from the target. Soft-fail on read errors — a pointer into a | ||
| // mapped-but-paged-out region or a recently-unmapped one is common during | ||
| // crash handling and shouldn't abort the whole walk. | ||
| void *buf = sentry_malloc(want); | ||
| if (!buf) { | ||
| return; | ||
| } | ||
| ssize_t got = ops->read_memory(ops->ctx, start, buf, want); | ||
| if (got <= 0) { | ||
| sentry_free(buf); | ||
| return; | ||
| } | ||
|
|
||
| minidump_rva_t rva = sentry__minidump_write_data(writer, buf, (size_t)got); | ||
| sentry_free(buf); | ||
| if (!rva) { | ||
| return; | ||
| } | ||
|
|
||
| sentry_indirect_region_t r; | ||
| r.start = start; | ||
| r.end = start + (uint64_t)got; | ||
| r.rva = rva; | ||
| r.size = (uint32_t)got; | ||
| insert_sorted(acc, &r); | ||
| } | ||
|
|
||
| void | ||
| sentry__indirect_walk_words(sentry_indirect_accumulator_t *acc, | ||
| minidump_writer_base_t *writer, const void *buf, size_t len_bytes, | ||
| const sentry_indirect_ops_t *ops) | ||
| { | ||
| const size_t word_size = sizeof(void *); | ||
| const size_t word_count = len_bytes / word_size; | ||
| if (word_size == 8) { | ||
| const uint64_t *words = (const uint64_t *)buf; | ||
| for (size_t i = 0; i < word_count; i++) { | ||
| sentry__indirect_consider(acc, writer, words[i], ops); | ||
| // Bail early once the byte cap is fully spent — no point grinding | ||
| // through the rest of the stack. The region cap also acts as a | ||
| // floor here. | ||
| if (acc->total_bytes >= SENTRY_INDIRECT_MAX_TOTAL_BYTES | ||
| || acc->region_count >= SENTRY_INDIRECT_MAX_REGIONS) { | ||
| return; | ||
| } | ||
| } | ||
| } else { | ||
| // 32-bit hosts (Linux i386 / arm). Pointers are 32-bit but our | ||
| // accumulator stores them as 64-bit just fine. | ||
| const uint32_t *words = (const uint32_t *)buf; | ||
| for (size_t i = 0; i < word_count; i++) { | ||
| sentry__indirect_consider(acc, writer, (uint64_t)words[i], ops); | ||
| if (acc->total_bytes >= SENTRY_INDIRECT_MAX_TOTAL_BYTES | ||
| || acc->region_count >= SENTRY_INDIRECT_MAX_REGIONS) { | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| } |
109 changes: 109 additions & 0 deletions
109
src/backends/native/minidump/sentry_minidump_indirect.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| #ifndef SENTRY_MINIDUMP_INDIRECT_H_INCLUDED | ||
| #define SENTRY_MINIDUMP_INDIRECT_H_INCLUDED | ||
|
|
||
| #include "sentry_minidump_common.h" | ||
| #include "sentry_minidump_format.h" | ||
| #include <stdbool.h> | ||
| #include <stddef.h> | ||
| #include <stdint.h> | ||
| #include <sys/types.h> | ||
|
|
||
| /** | ||
| * Indirectly-referenced memory capture for SMART minidump mode. | ||
| * | ||
| * Mirrors the Windows MiniDumpWithIndirectlyReferencedMemory contract: | ||
| * for every captured thread, scan its registers and stack words, and for | ||
| * each value that resolves into a writable heap region capture a small | ||
| * page-aligned chunk of memory around it. Lets debuggers (LLDB, VS Code) | ||
| * deref pointers held in struct locals or registers at crash time. | ||
| * | ||
| * This file is a small algorithm + a 2-function vtable. Each platform | ||
| * (linux, macos) provides the vtable and feeds in the per-thread | ||
| * registers and stack bytes; the dedup, paging, and dump-emission logic | ||
| * lives here once. | ||
| */ | ||
|
|
||
| // Per-pointer capture size — matches the Windows API default of 1 KiB. | ||
| #define SENTRY_INDIRECT_PER_POINTER_BYTES 1024 | ||
| // Hard caps so dump size doesn't blow up under pathological pointer density. | ||
| #define SENTRY_INDIRECT_MAX_REGIONS 1024 | ||
| #define SENTRY_INDIRECT_MAX_TOTAL_BYTES (4 * 1024 * 1024) | ||
| // Page size used for region alignment. 4 KiB on every arch we target. | ||
| #define SENTRY_INDIRECT_PAGE_SIZE 4096 | ||
|
|
||
| typedef struct { | ||
| uint64_t start; // page-aligned target VA | ||
| uint64_t end; // exclusive | ||
| minidump_rva_t rva; | ||
| uint32_t size; // bytes actually present in the dump | ||
| } sentry_indirect_region_t; | ||
|
|
||
| /** | ||
| * Platform shim. Each backend supplies its own pointer-validation and | ||
| * remote-memory-read implementations; the algorithm calls these via this | ||
| * tiny vtable instead of duplicating the loop in every platform file. | ||
| */ | ||
| typedef struct sentry_indirect_ops_s { | ||
| /** | ||
| * Returns true iff `addr` falls inside a readable, writable, non-executable | ||
| * mapping that is NOT a thread stack, vDSO, or other kernel-private region | ||
| * — i.e. the kind of mapping a heap pointer would target. | ||
| * | ||
| * Called O(words-on-stack) times per dump, so should be O(log n) over the | ||
| * cached mappings table (binary search) rather than a linear scan. | ||
| */ | ||
| bool (*is_writable_heap)(void *ctx, uint64_t addr); | ||
|
|
||
| /** | ||
| * Read up to `len` bytes from the *target* (crashed) process at virtual | ||
| * address `addr` into `buf`. Returns bytes read on success, or -1 on | ||
| * failure. | ||
| */ | ||
| ssize_t (*read_memory)(void *ctx, uint64_t addr, void *buf, size_t len); | ||
|
|
||
| /** Opaque pointer passed to the callbacks above. */ | ||
| void *ctx; | ||
| } sentry_indirect_ops_t; | ||
|
|
||
| /** | ||
| * Bounded accumulator. Holds the regions captured so far plus the running | ||
| * byte total used to enforce SENTRY_INDIRECT_MAX_TOTAL_BYTES. | ||
| * | ||
| * Regions are kept sorted by `start` so dedup is O(log n) per candidate. | ||
| */ | ||
| typedef struct { | ||
| sentry_indirect_region_t regions[SENTRY_INDIRECT_MAX_REGIONS]; | ||
| size_t region_count; | ||
| size_t total_bytes; | ||
| } sentry_indirect_accumulator_t; | ||
|
|
||
| void sentry__indirect_init(sentry_indirect_accumulator_t *acc); | ||
|
|
||
| /** | ||
| * Consider one address as a candidate heap pointer. If `addr` resolves into | ||
| * a writable-heap mapping (per ops->is_writable_heap), is not already covered | ||
| * by a previously-captured region, and the global byte budget hasn't been | ||
| * exhausted, this captures SENTRY_INDIRECT_PER_POINTER_BYTES around `addr`, | ||
| * page-aligned, and writes them to the dump file via `writer`. | ||
| * | ||
| * Safe to call with junk values — non-pointer addresses are filtered cheaply | ||
| * via the writable-heap check. | ||
| */ | ||
| void sentry__indirect_consider(sentry_indirect_accumulator_t *acc, | ||
| minidump_writer_base_t *writer, uint64_t addr, | ||
| const sentry_indirect_ops_t *ops); | ||
|
|
||
| /** | ||
| * Walk a buffer of pointer-sized words and call sentry__indirect_consider on | ||
| * each. `buf` is treated as an array of native-endian uint64_t (or uint32_t | ||
| * on 32-bit; the function handles both based on sizeof(void*)). `len_bytes` | ||
| * may be unaligned — the trailing bytes shorter than a word are ignored. | ||
| * | ||
| * This is the hot loop for stack scanning: a 1 MiB stack on AArch64 yields | ||
| * 128 K candidates, each costing one O(log n) mapping lookup. | ||
| */ | ||
| void sentry__indirect_walk_words(sentry_indirect_accumulator_t *acc, | ||
| minidump_writer_base_t *writer, const void *buf, size_t len_bytes, | ||
| const sentry_indirect_ops_t *ops); | ||
|
|
||
| #endif // SENTRY_MINIDUMP_INDIRECT_H_INCLUDED |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.