From 503046230961e8fc7fa7ced6c382a9884b7a512d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 28 Oct 2026 09:52:58 +0100 Subject: [PATCH 1/6] fix(android): avoid merging APK libraries with distant mappings Another mapping of an APK library's ELF header can lie anywhere in the address space. `is_duplicated_mapping()` accepted any mapping with the same inode and file offset, so without another module in between, the library merged into the earlier mapping, with a wrong `image_addr` and an `image_size` covering both. Accept such a mapping only within the module's `PT_LOAD` span. Share the program header walk with `get_code_id_from_program_header()` via `ELF_PROGRAM_HEADER_ITER`. Fixes: #2261 --- src/modulefinder/sentry_modulefinder_linux.c | 92 +++++++++++--------- tests/unit/test_modulefinder.c | 68 +++++++++++++++ tests/unit/tests.inc | 1 + 3 files changed, 118 insertions(+), 43 deletions(-) diff --git a/src/modulefinder/sentry_modulefinder_linux.c b/src/modulefinder/sentry_modulefinder_linux.c index 7d6b0d66ac..df1cacff5f 100644 --- a/src/modulefinder/sentry_modulefinder_linux.c +++ b/src/modulefinder/sentry_modulefinder_linux.c @@ -208,6 +208,46 @@ sentry__module_mapping_push( } } +#define ELFN_PROGRAM_HEADER_ITER(N, INNER) \ + Elf##N##_Ehdr elf; \ + ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(elf))); \ + \ + for (uint32_t i = 0; i < elf.e_phnum; i++) { \ + Elf##N##_Phdr header; \ + ENSURE(sentry__module_read_safely(&header, module, \ + elf.e_phoff + (uint64_t)elf.e_phentsize * i, sizeof(header))); \ + \ + INNER \ + } + +#define ELF_PROGRAM_HEADER_ITER(INNER) \ + unsigned char e_ident[EI_NIDENT]; \ + ENSURE(sentry__module_read_safely(e_ident, module, 0, EI_NIDENT)); \ + if (e_ident[EI_CLASS] == ELFCLASS64) { \ + ELFN_PROGRAM_HEADER_ITER(64, INNER) \ + } else { \ + ELFN_PROGRAM_HEADER_ITER(32, INNER) \ + } + +/** + * Returns the size of the address range that the PT_LOAD segments of the + * `module` span, or 0 if it cannot be determined. + */ +static uint64_t +get_load_size(const sentry_module_t *module) +{ + uint64_t min_vaddr = UINT64_MAX; + uint64_t max_vaddr = 0; + + ELF_PROGRAM_HEADER_ITER(if (header.p_type == PT_LOAD) { + min_vaddr = MIN(min_vaddr, header.p_vaddr); + max_vaddr = MAX(max_vaddr, (uint64_t)header.p_vaddr + header.p_memsz); + }) + return max_vaddr > min_vaddr ? max_vaddr - min_vaddr : 0; +fail: + return 0; +} + static bool is_duplicated_mapping( const sentry_module_t *module, const sentry_parsed_module_t *parsed) @@ -217,7 +257,9 @@ is_duplicated_mapping( } const sentry_mapped_region_t *mapping = &module->mappings[0]; return (mapping->offset == parsed->offset - && module->mappings_inode == parsed->inode); + && module->mappings_inode == parsed->inode + // the same file can also be mapped elsewhere, outside of this load + && parsed->start - mapping->addr < get_load_size(module)); } int @@ -266,22 +308,9 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) *size_out = 0; // iterate over all the program headers, for 32/64 bit separately - unsigned char e_ident[EI_NIDENT]; - ENSURE(sentry__module_read_safely(e_ident, module, 0, EI_NIDENT)); - if (e_ident[EI_CLASS] == ELFCLASS64) { - Elf64_Ehdr elf; - ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf64_Ehdr))); - - for (uint32_t i = 0; i < elf.e_phnum; i++) { - Elf64_Phdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_phoff + (uint64_t)elf.e_phentsize * i, - sizeof(Elf64_Phdr))); - - // we are only interested in notes - if (header.p_type != PT_NOTE) { - continue; - } + ELF_PROGRAM_HEADER_ITER( + // we are only interested in notes + if (header.p_type == PT_NOTE) { void *segment_addr = sentry__module_get_addr( module, header.p_offset, header.p_filesz); ENSURE(segment_addr); @@ -291,32 +320,7 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) if (code_id) { return code_id; } - } - } else { - Elf32_Ehdr elf; - ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); - - for (uint32_t i = 0; i < elf.e_phnum; i++) { - Elf32_Phdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_phoff + (uint64_t)elf.e_phentsize * i, - sizeof(Elf32_Phdr))); - - // we are only interested in notes - if (header.p_type != PT_NOTE) { - continue; - } - void *segment_addr = sentry__module_get_addr( - module, header.p_offset, header.p_filesz); - ENSURE(segment_addr); - const uint8_t *code_id - = sentry__elf_find_note(segment_addr, header.p_filesz, - header.p_align, NT_GNU_BUILD_ID, "GNU", 4, size_out); - if (code_id) { - return code_id; - } - } - } + }) fail: return NULL; } @@ -422,6 +426,8 @@ get_code_id_from_text_section(const sentry_module_t *module) return sentry_uuid_nil(); } +#undef ELFN_PROGRAM_HEADER_ITER +#undef ELF_PROGRAM_HEADER_ITER #undef ELF_SECTION_ITER bool diff --git a/tests/unit/test_modulefinder.c b/tests/unit/test_modulefinder.c index c48538f80d..7d8bf52661 100644 --- a/tests/unit/test_modulefinder.c +++ b/tests/unit/test_modulefinder.c @@ -4,8 +4,13 @@ #ifdef SENTRY_PLATFORM_LINUX # include "modulefinder/sentry_modulefinder_linux.h" +# include "sentry_string.h" +# include # include +# include # include +# include +# include # include #endif @@ -290,3 +295,66 @@ SENTRY_TEST(build_id_parser) "7fa824da-38f1-b87c-04df-718fda64990c"); #endif } + +SENTRY_TEST(module_finder_remapped_image) +{ +#if !defined(SENTRY_PLATFORM_LINUX) || !defined(MAP_FIXED_NOREPLACE) + SKIP_TEST(); +#else + Dl_info info; + TEST_ASSERT(dladdr((void *)&sentry_get_modules_list, &info) != 0); + uint64_t base = (uint64_t)(uintptr_t)info.dli_fbase; + + char path[PATH_MAX]; + ssize_t len = readlink("/proc/self/exe", path, sizeof(path) - 1); + TEST_ASSERT(len > 0); + path[len] = '\0'; + + int fd = open(path, O_RDONLY); + TEST_ASSERT(fd >= 0); + struct stat st; + TEST_ASSERT(fstat(fd, &st) == 0); + + // map the ELF header of the executable again, further below than its + // load spans, and reserve the gap to keep other modules out of it + uint64_t page = (uint64_t)getpagesize(); + uint64_t gap = ((uint64_t)st.st_size * 2 + page - 1) & ~(page - 1); + void *addr = (void *)(uintptr_t)(base - gap); + void *reserved = MAP_FAILED; + if (gap < base) { + reserved = mmap(addr, gap, PROT_NONE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE | MAP_FIXED_NOREPLACE, + -1, 0); + } + if (reserved != addr) { + if (reserved != MAP_FAILED) { + munmap(reserved, gap); + } + close(fd); + SKIP_TEST(); + } + void *header = mmap(addr, page, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, 0); + close(fd); + + sentry_clear_modulecache(); + sentry_value_t modules = sentry_get_modules_list(); + munmap(addr, gap); + TEST_CHECK(header == addr); + + char expected[32]; + sentry__addr_to_string(expected, sizeof(expected), base); + + bool found = false; + for (size_t i = 0; i < sentry_value_get_length(modules); i++) { + sentry_value_t mod = sentry_value_get_by_index(modules, i); + const char *image_addr = sentry_value_as_string( + sentry_value_get_by_key(mod, "image_addr")); + found |= strcmp(image_addr, expected) == 0; + } + sentry_value_decref(modules); + sentry_clear_modulecache(); + + TEST_CHECK(found); + TEST_MSG("Expected a module at %s", expected); +#endif +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 7afcfadc26..972592b773 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -295,6 +295,7 @@ XX(minidump_thread_structure) XX(mmap_file_closes_fd_zero_on_failure) XX(module_addr) XX(module_finder) +XX(module_finder_remapped_image) XX(mpack_newlines) XX(mpack_removed_tags) XX(multiple_inits) From a23962d08849025a2acaded3ec9573efebe30545 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 28 Oct 2026 10:19:28 +0100 Subject: [PATCH 2/6] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index df6eb82a9e..776e7bbb2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - An empty proxy (`sentry_options_set_proxy(options, "")`) now means a direct connection with every transport and backend. Before, the WinHTTP transport, the Windows and macOS crashpad handlers and the native backend's daemon fell back to the proxy environment variables or the system proxy settings. ([#2218](https://github.com/getsentry/sentry-native/pull/2218), [crashpad#175](https://github.com/getsentry/crashpad/pull/175)) - Inproc/Android: keep the original crash when the unwinder fails to allocate memory in the crash handler. ([#2239](https://github.com/getsentry/sentry-native/pull/2239)) - Inproc: fix a deadlock when a crash occurs during `sentry_close()`, such as when re-initializing the SDK. ([#2252](https://github.com/getsentry/sentry-native/pull/2252)) +- Android: fix image addresses of libraries loaded from the APK. ([#2262](https://github.com/getsentry/sentry-native/pull/2262)) **Thank you**: From 32923418cfc7e89fc727b28cf96fdc807b7cc3d1 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 28 Oct 2026 11:47:23 +0100 Subject: [PATCH 3/6] test: fix remapped image test under clang TSAN The test mapped the executable's ELF header again twice its file size below it, assuming its load range is smaller. Clang links the TSAN runtime statically, and its `.bss` makes the load range about four times the file size, so the mapping landed inside it, where it still merges by design. Size the gap from the executable's `PT_LOAD` segments instead. --- tests/unit/test_modulefinder.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_modulefinder.c b/tests/unit/test_modulefinder.c index 7d8bf52661..e204183a5b 100644 --- a/tests/unit/test_modulefinder.c +++ b/tests/unit/test_modulefinder.c @@ -8,9 +8,10 @@ # include # include # include +# include # include +# include # include -# include # include #endif @@ -312,13 +313,19 @@ SENTRY_TEST(module_finder_remapped_image) int fd = open(path, O_RDONLY); TEST_ASSERT(fd >= 0); - struct stat st; - TEST_ASSERT(fstat(fd, &st) == 0); // map the ELF header of the executable again, further below than its // load spans, and reserve the gap to keep other modules out of it + const ElfW(Phdr) *phdrs = (const ElfW(Phdr) *)getauxval(AT_PHDR); + uint64_t span = 0; + for (size_t i = 0; i < getauxval(AT_PHNUM); i++) { + if (phdrs[i].p_type == PT_LOAD + && phdrs[i].p_vaddr + phdrs[i].p_memsz > span) { + span = phdrs[i].p_vaddr + phdrs[i].p_memsz; + } + } uint64_t page = (uint64_t)getpagesize(); - uint64_t gap = ((uint64_t)st.st_size * 2 + page - 1) & ~(page - 1); + uint64_t gap = (span + page - 1) & ~(page - 1); void *addr = (void *)(uintptr_t)(base - gap); void *reserved = MAP_FAILED; if (gap < base) { From e9877e6775048fbbf923e84f15bf432629a0c02e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 28 Oct 2026 10:45:01 +0100 Subject: [PATCH 4/6] fix(android): unwind past PAC-signed return addresses On Armv8.3+ devices, native stack traces ended at the first PAC-signed return address, because libunwindstack-ndk only stripped PAC with a mask that nothing sets. Bump it to strip with `xpaclri` like bionic does. Fixes: #2258 --- external/libunwindstack-ndk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/libunwindstack-ndk b/external/libunwindstack-ndk index 284202fb1e..5dfaca743f 160000 --- a/external/libunwindstack-ndk +++ b/external/libunwindstack-ndk @@ -1 +1 @@ -Subproject commit 284202fb1e42dbeba6598e26ced2e1ec404eecd1 +Subproject commit 5dfaca743fd9a7bab5ceb51398dfb5a6b45a3c0d From 21b50df7f081651939a81ad25434f36ea3ad5a5e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 28 Oct 2026 11:04:43 +0100 Subject: [PATCH 5/6] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 776e7bbb2b..b4656634e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Inproc/Android: keep the original crash when the unwinder fails to allocate memory in the crash handler. ([#2239](https://github.com/getsentry/sentry-native/pull/2239)) - Inproc: fix a deadlock when a crash occurs during `sentry_close()`, such as when re-initializing the SDK. ([#2252](https://github.com/getsentry/sentry-native/pull/2252)) - Android: fix image addresses of libraries loaded from the APK. ([#2262](https://github.com/getsentry/sentry-native/pull/2262)) +- Android: unwind past PAC-signed return addresses on ARM64. ([#2263](https://github.com/getsentry/sentry-native/pull/2263), [libunwindstack-ndk#10](https://github.com/getsentry/libunwindstack-ndk/pull/10)) **Thank you**: From f80cc9375018a48ae59ab3adacb17fc7f9da3229 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 30 Oct 2026 10:13:55 +0100 Subject: [PATCH 6/6] bump libunwindstack-ndk --- external/libunwindstack-ndk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/libunwindstack-ndk b/external/libunwindstack-ndk index 5dfaca743f..d81a291bf6 160000 --- a/external/libunwindstack-ndk +++ b/external/libunwindstack-ndk @@ -1 +1 @@ -Subproject commit 5dfaca743fd9a7bab5ceb51398dfb5a6b45a3c0d +Subproject commit d81a291bf6487f5c00d256d0084242d1cc1df2fd