From dbb6acbedb6632bcb59d1f80fdd1e68dd2147df6 Mon Sep 17 00:00:00 2001 From: Gleb Linnik Date: Wed, 2 Sep 2026 23:18:01 +0300 Subject: [PATCH] fix(native): read frame records at pointer width in the FP walk `read_stack_value` bounds-checked and copied `sizeof(uint64_t)` bytes regardless of the target's pointer size, and the walk stepped to the return address with the same constant. On a 32-bit target every read therefore took two stack slots per pointer: the high half of each `saved_fp` and `return_addr` was the neighbouring word, the return address was read from the wrong slot, and the last legitimate frame record failed the bounds check. Read a `uintptr_t` and widen it, and step by pointer size; 64-bit targets are unchanged. While here, range-check by subtraction rather than `addr + size`: the address comes from a frame pointer in a crashed process, and a corrupted value near the top of the address space wraps the sum past the naive comparison. --- CHANGELOG.md | 1 + src/backends/native/sentry_crash_daemon.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa9dff983..166e48b69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ **Fixes**: +- native: Read frame records at pointer width in the crash daemon's frame-pointer walk, so 32-bit targets no longer read two stack slots per pointer. ([#2052](https://github.com/getsentry/sentry-native/pull/2052)) - Prevent backend state races when `sentry_reinstall_backend` runs concurrently with scope observer callbacks. ([#2041](https://github.com/getsentry/sentry-native/pull/2041)) ## 0.16.5 diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 369e3cd1d..980de372b 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -590,12 +590,19 @@ static bool read_stack_value(const uint8_t *stack_buf, uint64_t stack_start, uint64_t stack_size, uint64_t addr, uint64_t *out_value) { - if (addr < stack_start - || addr + sizeof(uint64_t) > stack_start + stack_size) { + // Range-check by subtraction: `addr` comes from a frame pointer in a + // crashed process, and a corrupted value near the top of the address + // space would wrap an `addr + size` sum past the naive comparison. + if (addr < stack_start) { return false; } uint64_t offset = addr - stack_start; - memcpy(out_value, stack_buf + offset, sizeof(uint64_t)); + if (offset > stack_size || stack_size - offset < sizeof(uintptr_t)) { + return false; + } + uintptr_t value = 0; + memcpy(&value, stack_buf + (size_t)offset, sizeof(value)); + *out_value = (uint64_t)value; return true; } @@ -1112,7 +1119,7 @@ build_stacktrace_for_thread( uint64_t return_addr = 0; // Read saved frame pointer and return address - // Frame layout: [FP+0] = saved FP, [FP+8] = return addr + // Frame layout: [FP] = saved FP, [FP + pointer size] = return addr if (!read_stack_value(stack_buf, stack_start, stack_size, current_fp, &saved_fp)) { SENTRY_TRACEF( @@ -1123,9 +1130,9 @@ build_stacktrace_for_thread( break; } if (!read_stack_value(stack_buf, stack_start, stack_size, - current_fp + sizeof(uint64_t), &return_addr)) { + current_fp + sizeof(uintptr_t), &return_addr)) { SENTRY_TRACEF("Cannot read return addr at 0x%llx", - (unsigned long long)(current_fp + sizeof(uint64_t))); + (unsigned long long)(current_fp + sizeof(uintptr_t))); break; } saved_fp = SENTRY__STRIP_PAC(saved_fp);