diff --git a/CHANGELOG.md b/CHANGELOG.md index caa497b98e..19066e8107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes**: - macOS: cache VM regions for FP validation in the new unwinder. ([#1634](https://github.com/getsentry/sentry-native/pull/1634)) +- Linux: remove dependency on `stdio` in the unwinder pointer validation code to reduce exposure to async-signal-unsafe functions. ([#1637](https://github.com/getsentry/sentry-native/pull/1637)) ## 0.13.6 diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 9c0c95d7b0..997c65e6e5 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -3,36 +3,142 @@ #include "sentry_unwinder.h" #define UNW_LOCAL_ONLY #include -#include + +#include +#include +#include + +/** + * Parse a lower-case hex number starting at `s`. + * Advances `*pos` past the parsed digits. Returns the parsed value. + */ +static uintptr_t +parse_hex(const char *s, size_t len, size_t *pos) +{ + uintptr_t val = 0; + while (*pos < len) { + char c = s[*pos]; + if (c >= '0' && c <= '9') { + val = (val << 4) | (uintptr_t)(c - '0'); + } else if (c >= 'a' && c <= 'f') { + val = (val << 4) | (uintptr_t)(c - 'a' + 10); + } else { + break; + } + (*pos)++; + } + return val; +} /** - * Looks up the memory range for a given pointer in /proc/self/maps. - * `range` is an output parameter. Returns `true` if a range was found. - * Note: it is safe to use this function as long as we are running in a healthy - * thread or in the handler thread. The function is unsafe for a signal handler. + * Try to parse "lo-hi" from the start of `line` (length `len`). + * Returns `true` and fills `range` if `ptr` falls inside [lo, hi). */ static bool -find_mem_range(uintptr_t ptr, mem_range_t *range) +match_range(const char *line, size_t len, uintptr_t ptr, mem_range_t *range) { - bool found = false; - FILE *fp = fopen("/proc/self/maps", "r"); - if (!fp) { - return found; + size_t p = 0; + uintptr_t lo = parse_hex(line, len, &p); + if (p >= len || line[p] != '-') { + return false; } - char line[512]; - while (fgets(line, sizeof line, fp)) { - unsigned long lo, hi; - if (sscanf(line, "%lx-%lx", &lo, &hi) == 2) { - // our bounds are [lo, hi) - if (ptr >= lo && ptr < hi) { - range->lo = (uintptr_t)lo; - range->hi = (uintptr_t)hi; - found = true; + p++; + uintptr_t hi = parse_hex(line, len, &p); + if (lo < hi && ptr >= lo && ptr < hi) { + range->lo = lo; + range->hi = hi; + return true; + } + return false; +} + +/** + * Searches for the memory range containing `ptr` by reading lines in the + * /proc/self/maps format from `fd`. + * + * This implementation is async-signal-safe: it only uses POSIX file-IO and + * stdio-free hex parsing. + */ +#ifndef SENTRY_UNITTEST +static +#endif + bool + find_mem_range_from_fd(int fd, uintptr_t ptr, mem_range_t *range) +{ + char buf[4096]; + size_t carry = 0; + bool skip_to_nl = false; // true while discarding an oversized line's tail + + for (;;) { + ssize_t n; + do { + n = read(fd, buf + carry, sizeof(buf) - carry); + } while (n < 0 && errno == EINTR); + if (n <= 0) { + return false; + } + + size_t total = carry + (size_t)n; + size_t pos = 0; + + // If we are in the middle of skipping an oversized line, scan + // forward to the newline before resuming normal parsing. + if (skip_to_nl) { + while (pos < total && buf[pos] != '\n') { + pos++; + } + if (pos >= total) { + carry = 0; + continue; + } + pos++; // skip past newline + skip_to_nl = false; + } + + while (pos < total) { + size_t nl = pos; + while (nl < total && buf[nl] != '\n') { + nl++; + } + if (nl >= total) { break; } + if (match_range(buf + pos, nl - pos, ptr, range)) { + return true; + } + pos = nl + 1; } + + carry = total - pos; + if (carry >= sizeof(buf)) { + // A single line fills the entire buffer. Parse the "lo-hi" + // prefix that is already in buf, then skip the rest of the + // line on subsequent reads. + if (match_range(buf, sizeof(buf), ptr, range)) { + return true; + } + carry = 0; + skip_to_nl = true; + } else if (carry > 0 && pos > 0) { + for (size_t i = 0; i < carry; i++) { + buf[i] = buf[pos + i]; + } + } + } +} + +/** + * Looks up the memory range for `ptr` in /proc/self/maps. + */ +static bool +find_mem_range(uintptr_t ptr, mem_range_t *range) +{ + int fd = open("/proc/self/maps", O_RDONLY); + if (fd < 0) { + return false; } - fclose(fp); + bool found = find_mem_range_from_fd(fd, ptr, range); + close(fd); return found; } diff --git a/tests/unit/sentry_testsupport.h b/tests/unit/sentry_testsupport.h index b6afa1959e..54c8bb2315 100644 --- a/tests/unit/sentry_testsupport.h +++ b/tests/unit/sentry_testsupport.h @@ -55,6 +55,13 @@ TEST_CHECK_(_a == _b, "%llu == %llu", _a, _b); \ } while (0) +#define TEST_CHECK_PTR_EQUAL(A, B) \ + do { \ + const void *_a = (const void *)(A); \ + const void *_b = (const void *)(B); \ + TEST_CHECK_(_a == _b, "%p == %p", _a, _b); \ + } while (0) + #define TEST_ASSERT_INT_EQUAL(A, B) \ do { \ long long _a = (long long)(A); \ diff --git a/tests/unit/test_unwinder.c b/tests/unit/test_unwinder.c index 2dfb201c40..0dd674c029 100644 --- a/tests/unit/test_unwinder.c +++ b/tests/unit/test_unwinder.c @@ -2,6 +2,13 @@ #include "sentry_symbolizer.h" #include "sentry_testsupport.h" +#ifdef SENTRY_WITH_UNWINDER_LIBUNWIND +# include "unwinder/sentry_unwinder.h" +# include +# include +extern bool find_mem_range_from_fd(int fd, uintptr_t ptr, mem_range_t *range); +#endif + // On arm64e, function pointers have PAC (Pointer Authentication Code) bits // that must be stripped for comparison with addresses from dladdr(). #if defined(__arm64e__) @@ -84,8 +91,83 @@ SENTRY_TEST(unwinder) if (frame_count2 > 0) { TEST_CHECK_INT_EQUAL(frame_count2, frame_count1 - offset); for (size_t i = 0; i < frame_count2; i++) { - TEST_CHECK_INT_EQUAL(backtrace2[i], backtrace1[offset + i]); + TEST_CHECK_PTR_EQUAL(backtrace2[i], backtrace1[offset + i]); } } } } + +SENTRY_TEST(find_mem_range) +{ +#if !defined(SENTRY_WITH_UNWINDER_LIBUNWIND) + SKIP_TEST(); +#else + // A stack variable must be in a mapped region via /proc/self/maps. + int stack_var = 42; + (void)stack_var; + int fd = open("/proc/self/maps", O_RDONLY); + TEST_ASSERT(fd >= 0); + mem_range_t range = { 0, 0 }; + bool found = find_mem_range_from_fd(fd, (uintptr_t)&stack_var, &range); + close(fd); + TEST_CHECK(found); + TEST_CHECK(range.lo > 0); + TEST_CHECK(range.lo < range.hi); + TEST_CHECK((uintptr_t)&stack_var >= range.lo); + TEST_CHECK((uintptr_t)&stack_var < range.hi); + + // An obviously invalid pointer should not be found. + fd = open("/proc/self/maps", O_RDONLY); + TEST_ASSERT(fd >= 0); + mem_range_t bad_range = { 0, 0 }; + found = find_mem_range_from_fd(fd, (uintptr_t)0x1, &bad_range); + close(fd); + TEST_CHECK(!found); + TEST_CHECK(bad_range.lo == 0); + TEST_CHECK(bad_range.hi == 0); + + // Test that the target range ON an oversized line (> 4096 bytes) is + // still found. The lo-hi prefix must be parsed before discarding the + // remainder of the line. + char tmp_path[] = "/tmp/sentry_test_maps_XXXXXX"; + fd = mkstemp(tmp_path); + TEST_ASSERT(fd >= 0); + const char *long_prefix = "dead0000-deadf000 r-xp 00000000 08:01 1234 /"; + TEST_ASSERT(write(fd, long_prefix, strlen(long_prefix)) + == (ssize_t)strlen(long_prefix)); + char filler[4096]; + memset(filler, 'a', sizeof(filler)); + TEST_ASSERT(write(fd, filler, sizeof(filler)) == (ssize_t)sizeof(filler)); + TEST_ASSERT(write(fd, "\n", 1) == 1); + lseek(fd, 0, SEEK_SET); + + mem_range_t long_range = { 0, 0 }; + found = find_mem_range_from_fd(fd, (uintptr_t)0xdead1000, &long_range); + TEST_CHECK(found); + TEST_CHECK_PTR_EQUAL((void *)long_range.lo, (void *)(uintptr_t)0xdead0000); + TEST_CHECK_PTR_EQUAL((void *)long_range.hi, (void *)(uintptr_t)0xdeadf000); + + // Test that a non-matching oversized line is correctly skipped and + // a subsequent normal line is still found. + TEST_ASSERT(ftruncate(fd, 0) == 0); + lseek(fd, 0, SEEK_SET); + const char *skip_prefix = "1000-2000 r-xp 00000000 08:01 1234 /"; + TEST_ASSERT(write(fd, skip_prefix, strlen(skip_prefix)) + == (ssize_t)strlen(skip_prefix)); + TEST_ASSERT(write(fd, filler, sizeof(filler)) == (ssize_t)sizeof(filler)); + TEST_ASSERT(write(fd, "\n", 1) == 1); + const char *target = "dead0000-deadf000 r-xp 00000000 08:01 5678 /normal\n"; + TEST_ASSERT(write(fd, target, strlen(target)) == (ssize_t)strlen(target)); + lseek(fd, 0, SEEK_SET); + + mem_range_t crafted_range = { 0, 0 }; + found = find_mem_range_from_fd(fd, (uintptr_t)0xdead1000, &crafted_range); + close(fd); + unlink(tmp_path); + TEST_CHECK(found); + TEST_CHECK_PTR_EQUAL( + (void *)crafted_range.lo, (void *)(uintptr_t)0xdead0000); + TEST_CHECK_PTR_EQUAL( + (void *)crafted_range.hi, (void *)(uintptr_t)0xdeadf000); +#endif +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index a1df4cafa7..a4d72ca20c 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -118,6 +118,7 @@ XX(feedback_with_file_attachment) XX(feedback_with_multiple_attachments) XX(feedback_with_null_hint) XX(feedback_without_hint) +XX(find_mem_range) XX(formatted_log_messages) XX(fuzz_json) XX(getenv_double)