From ff074baa7f4cab58d4e30b71e7c131887f8aac64 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 6 Dec 2022 12:50:50 +0100 Subject: [PATCH 1/5] Introduce new fallback to retrieve the code-id from the .note section --- src/modulefinder/sentry_modulefinder_linux.c | 104 +++++++++++++++++-- 1 file changed, 97 insertions(+), 7 deletions(-) diff --git a/src/modulefinder/sentry_modulefinder_linux.c b/src/modulefinder/sentry_modulefinder_linux.c index b37722de4..5fdaaa7b3 100644 --- a/src/modulefinder/sentry_modulefinder_linux.c +++ b/src/modulefinder/sentry_modulefinder_linux.c @@ -292,7 +292,7 @@ get_code_id_from_notes( } static const uint8_t * -get_code_id_from_elf(const sentry_module_t *module, size_t *size_out) +get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) { *size_out = 0; @@ -350,13 +350,91 @@ get_code_id_from_elf(const sentry_module_t *module, size_t *size_out) return NULL; } +static const uint8_t * +get_code_id_from_note_section(const sentry_module_t *module, size_t *size_out) +{ + *size_out = 0; + + size_t text_size = 0; + + // iterate over all the section 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))); + + Elf64_Shdr strheader; + ENSURE(sentry__module_read_safely(&strheader, module, + elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, + sizeof(Elf64_Shdr))); + + for (int i = 0; i < elf.e_shnum; i++) { + Elf64_Shdr header; + ENSURE(sentry__module_read_safely(&header, module, + elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); + + char name[6]; + ENSURE(sentry__module_read_safely(name, module, + strheader.sh_offset + header.sh_name, sizeof(name))); + name[5] = '\0'; + if (header.sh_type == SHT_NOTE && strcmp(name, ".note") == 0) { + void *segment_addr = sentry__module_get_addr( + module, header.sh_offset, header.sh_size); + ENSURE(segment_addr); + const uint8_t *code_id + = get_code_id_from_notes(header.sh_addralign, segment_addr, + (void *)((uintptr_t)segment_addr + header.sh_size), + size_out); + if (code_id) { + return code_id; + } + } + } + } else { + Elf32_Ehdr elf; + ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); + + Elf32_Shdr strheader; + ENSURE(sentry__module_read_safely(&strheader, module, + elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, + sizeof(Elf32_Shdr))); + + for (int i = 0; i < elf.e_shnum; i++) { + Elf32_Shdr header; + ENSURE(sentry__module_read_safely(&header, module, + elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); + + char name[6]; + ENSURE(sentry__module_read_safely(name, module, + strheader.sh_offset + header.sh_name, sizeof(name))); + name[5] = '\0'; + if (header.sh_type == SHT_NOTE && strcmp(name, ".note") == 0) { + void *segment_addr = sentry__module_get_addr( + module, header.sh_offset, header.sh_size); + ENSURE(segment_addr); + const uint8_t *code_id + = get_code_id_from_notes(header.sh_addralign, segment_addr, + (void *)((uintptr_t)segment_addr + header.sh_size), + size_out); + if (code_id) { + return code_id; + } + } + } + } + +fail: + return NULL; +} + static sentry_uuid_t -get_code_id_from_text_fallback(const sentry_module_t *module) +get_code_id_from_text_section(const sentry_module_t *module) { const uint8_t *text = NULL; size_t text_size = 0; - // iterate over all the program headers, for 32/64 bit separately + // iterate over all the section 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) { @@ -431,18 +509,30 @@ bool sentry__procmaps_read_ids_from_elf( sentry_value_t value, const sentry_module_t *module) { - // and try to get the debug id from the elf headers of the loaded - // modules + // try to get the debug id from the elf headers of the loaded modules size_t code_id_size; - const uint8_t *code_id = get_code_id_from_elf(module, &code_id_size); + const uint8_t *code_id + = get_code_id_from_program_header(module, &code_id_size); sentry_uuid_t uuid = sentry_uuid_nil(); + if (code_id) { sentry_value_set_by_key(value, "code_id", sentry__value_new_hexstring(code_id, code_id_size)); memcpy(uuid.bytes, code_id, MIN(code_id_size, 16)); } else { - uuid = get_code_id_from_text_fallback(module); + // no code-id found, try the ".note.gnu.build-id" section + code_id = get_code_id_from_note_section(module, &code_id_size); + if (code_id) { + sentry_value_set_by_key(value, "code_id", + sentry__value_new_hexstring(code_id, code_id_size)); + + memcpy(uuid.bytes, code_id, MIN(code_id_size, 16)); + } else { + // We were not able to locate the code-id, so fall back to hashing + // the first page of the ".text" (program code) section. + uuid = get_code_id_from_text_section(module); + } } // the usage of these is described here: From 5f7e041329cb1dc224d2a8b5985912c66c3ea06a Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 6 Dec 2022 16:39:06 +0100 Subject: [PATCH 2/5] Deduplicate at least the ELF section iteration. --- src/modulefinder/sentry_modulefinder_linux.c | 245 ++++++++----------- 1 file changed, 96 insertions(+), 149 deletions(-) diff --git a/src/modulefinder/sentry_modulefinder_linux.c b/src/modulefinder/sentry_modulefinder_linux.c index 5fdaaa7b3..1d459c12f 100644 --- a/src/modulefinder/sentry_modulefinder_linux.c +++ b/src/modulefinder/sentry_modulefinder_linux.c @@ -350,80 +350,72 @@ get_code_id_from_program_header(const sentry_module_t *module, size_t *size_out) return NULL; } +#define ELF_SECTION_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) { \ + Elf64_Ehdr elf; \ + ENSURE( \ + sentry__module_read_safely(&elf, module, 0, sizeof(Elf64_Ehdr))); \ + \ + Elf64_Shdr strheader; \ + ENSURE(sentry__module_read_safely(&strheader, module, \ + elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \ + sizeof(Elf64_Shdr))); \ + \ + for (int i = 0; i < elf.e_shnum; i++) { \ + Elf64_Shdr header; \ + ENSURE(sentry__module_read_safely(&header, module, \ + elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); \ + \ + char name[6]; \ + ENSURE(sentry__module_read_safely(name, module, \ + strheader.sh_offset + header.sh_name, sizeof(name))); \ + name[5] = '\0'; \ + \ + INNER \ + } \ + } else { \ + Elf32_Ehdr elf; \ + ENSURE( \ + sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); \ + \ + Elf32_Shdr strheader; \ + ENSURE(sentry__module_read_safely(&strheader, module, \ + elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, \ + sizeof(Elf32_Shdr))); \ + \ + for (int i = 0; i < elf.e_shnum; i++) { \ + Elf32_Shdr header; \ + ENSURE(sentry__module_read_safely(&header, module, \ + elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); \ + \ + char name[6]; \ + ENSURE(sentry__module_read_safely(name, module, \ + strheader.sh_offset + header.sh_name, sizeof(name))); \ + name[5] = '\0'; \ + \ + INNER \ + } \ + } + static const uint8_t * get_code_id_from_note_section(const sentry_module_t *module, size_t *size_out) { *size_out = 0; - size_t text_size = 0; - - // iterate over all the section 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))); - - Elf64_Shdr strheader; - ENSURE(sentry__module_read_safely(&strheader, module, - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, - sizeof(Elf64_Shdr))); - - for (int i = 0; i < elf.e_shnum; i++) { - Elf64_Shdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); - - char name[6]; - ENSURE(sentry__module_read_safely(name, module, - strheader.sh_offset + header.sh_name, sizeof(name))); - name[5] = '\0'; - if (header.sh_type == SHT_NOTE && strcmp(name, ".note") == 0) { - void *segment_addr = sentry__module_get_addr( - module, header.sh_offset, header.sh_size); - ENSURE(segment_addr); - const uint8_t *code_id - = get_code_id_from_notes(header.sh_addralign, segment_addr, - (void *)((uintptr_t)segment_addr + header.sh_size), - size_out); - if (code_id) { - return code_id; - } - } - } - } else { - Elf32_Ehdr elf; - ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); - - Elf32_Shdr strheader; - ENSURE(sentry__module_read_safely(&strheader, module, - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, - sizeof(Elf32_Shdr))); - - for (int i = 0; i < elf.e_shnum; i++) { - Elf32_Shdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); - - char name[6]; - ENSURE(sentry__module_read_safely(name, module, - strheader.sh_offset + header.sh_name, sizeof(name))); - name[5] = '\0'; - if (header.sh_type == SHT_NOTE && strcmp(name, ".note") == 0) { - void *segment_addr = sentry__module_get_addr( - module, header.sh_offset, header.sh_size); - ENSURE(segment_addr); - const uint8_t *code_id - = get_code_id_from_notes(header.sh_addralign, segment_addr, - (void *)((uintptr_t)segment_addr + header.sh_size), - size_out); - if (code_id) { - return code_id; - } + ELF_SECTION_ITER( + if (header.sh_type == SHT_NOTE && strcmp(name, ".note") == 0) { + void *segment_addr = sentry__module_get_addr( + module, header.sh_offset, header.sh_size); + ENSURE(segment_addr); + const uint8_t *code_id = get_code_id_from_notes(header.sh_addralign, + segment_addr, + (void *)((uintptr_t)segment_addr + header.sh_size), size_out); + if (code_id) { + return code_id; } - } - } - + }) fail: return NULL; } @@ -434,62 +426,14 @@ get_code_id_from_text_section(const sentry_module_t *module) const uint8_t *text = NULL; size_t text_size = 0; - // iterate over all the section 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))); - - Elf64_Shdr strheader; - ENSURE(sentry__module_read_safely(&strheader, module, - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, - sizeof(Elf64_Shdr))); - - for (int i = 0; i < elf.e_shnum; i++) { - Elf64_Shdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf64_Shdr))); - - char name[6]; - ENSURE(sentry__module_read_safely(name, module, - strheader.sh_offset + header.sh_name, sizeof(name))); - name[5] = '\0'; - if (header.sh_type == SHT_PROGBITS && strcmp(name, ".text") == 0) { - text = sentry__module_get_addr( - module, header.sh_offset, header.sh_size); - ENSURE(text); - text_size = header.sh_size; - break; - } - } - } else { - Elf32_Ehdr elf; - ENSURE(sentry__module_read_safely(&elf, module, 0, sizeof(Elf32_Ehdr))); - - Elf32_Shdr strheader; - ENSURE(sentry__module_read_safely(&strheader, module, - elf.e_shoff + elf.e_shentsize * elf.e_shstrndx, - sizeof(Elf32_Shdr))); - - for (int i = 0; i < elf.e_shnum; i++) { - Elf32_Shdr header; - ENSURE(sentry__module_read_safely(&header, module, - elf.e_shoff + elf.e_shentsize * i, sizeof(Elf32_Shdr))); - - char name[6]; - ENSURE(sentry__module_read_safely(name, module, - strheader.sh_offset + header.sh_name, sizeof(name))); - name[5] = '\0'; - if (header.sh_type == SHT_PROGBITS && strcmp(name, ".text") == 0) { - text = sentry__module_get_addr( - module, header.sh_offset, header.sh_size); - ENSURE(text); - text_size = header.sh_size; - break; - } - } - } + ELF_SECTION_ITER( + if (header.sh_type == SHT_PROGBITS && strcmp(name, ".text") == 0) { + text = sentry__module_get_addr( + module, header.sh_offset, header.sh_size); + ENSURE(text); + text_size = header.sh_size; + break; + }) sentry_uuid_t uuid = sentry_uuid_nil(); @@ -505,6 +449,8 @@ get_code_id_from_text_section(const sentry_module_t *module) return sentry_uuid_nil(); } +#undef ELF_SECTION_ITER + bool sentry__procmaps_read_ids_from_elf( sentry_value_t value, const sentry_module_t *module) @@ -529,16 +475,17 @@ sentry__procmaps_read_ids_from_elf( memcpy(uuid.bytes, code_id, MIN(code_id_size, 16)); } else { - // We were not able to locate the code-id, so fall back to hashing - // the first page of the ".text" (program code) section. + // We were not able to locate the code-id, so fall back to + // hashing the first page of the ".text" (program code) + // section. uuid = get_code_id_from_text_section(module); } } // the usage of these is described here: // https://getsentry.github.io/symbolicator/advanced/symbol-server-compatibility/#identifiers - // in particular, the debug_id is a `little-endian GUID`, so we have to do - // appropriate byte-flipping + // in particular, the debug_id is a `little-endian GUID`, so we have + // to do appropriate byte-flipping char *uuid_bytes = uuid.bytes; uint32_t *a = (uint32_t *)uuid_bytes; *a = htonl(*a); @@ -570,14 +517,15 @@ sentry__procmaps_module_to_value(const sentry_module_t *module) sentry_value_set_by_key( mod_val, "image_size", sentry_value_new_int32(module_size)); - // At least on the android API-16, x86 simulator, the linker apparently - // does not load the complete file into memory. Or at least, the section - // headers which are located at the end of the file are not loaded, and - // we would be poking into invalid memory. To be safe, we mmap the - // complete file from disk, so we have the on-disk layout, and are - // independent of how the runtime linker would load or re-order any - // sections. The exception here is the linux-gate, which is not an - // actual file on disk, so we actually poke at its memory. + // At least on the android API-16, x86 simulator, the linker + // apparently does not load the complete file into memory. Or at + // least, the section headers which are located at the end of the + // file are not loaded, and we would be poking into invalid memory. + // To be safe, we mmap the complete file from disk, so we have the + // on-disk layout, and are independent of how the runtime linker + // would load or re-order any sections. The exception here is the + // linux-gate, which is not an actual file on disk, so we actually + // poke at its memory. if (sentry__slice_eq(module->file, LINUX_GATE)) { sentry__procmaps_read_ids_from_elf(mod_val, module); } else { @@ -707,8 +655,9 @@ load_modules(sentry_value_t modules) uint64_t linux_vdso = get_linux_vdso(); - // we have multiple memory maps per file, and we need to merge their offsets - // based on the filename. Luckily, the maps are ordered by filename, so yay + // we have multiple memory maps per file, and we need to merge their + // offsets based on the filename. Luckily, the maps are ordered by + // filename, so yay sentry_module_t last_module; memset(&last_module, 0, sizeof(sentry_module_t)); while (true) { @@ -739,19 +688,17 @@ load_modules(sentry_value_t modules) } if (is_valid_elf_header((void *)(size_t)module.start)) { - // On android, we sometimes have multiple mappings for the same - // inode at the same offset, such as this, excuse the auto-format - // here: - // 737b5570d000-737b5570e000 r--p 00000000 07:70 34 - // /apex/com.android.runtime/lib64/bionic/libdl.so - // 737b5570e000-737b5570f000 r-xp 00000000 07:70 34 - // /apex/com.android.runtime/lib64/bionic/libdl.so - // 737b5570f000-737b55710000 r--p 00000000 07:70 34 - // /apex/com.android.runtime/lib64/bionic/libdl.so + // clang-format off + // On android, we sometimes have multiple mappings for the + // same inode at the same offset, such as this: + // 737b5570d000-737b5570e000 r--p 00000000 07:70 34 /apex/com.android.runtime/lib64/bionic/libdl.so + // 737b5570e000-737b5570f000 r-xp 00000000 07:70 34 /apex/com.android.runtime/lib64/bionic/libdl.so + // 737b5570f000-737b55710000 r--p 00000000 07:70 34 /apex/com.android.runtime/lib64/bionic/libdl.so + // clang-format on if (!is_duplicated_mapping(&last_module, &module)) { - // try to append the module based on the mappings that we have - // found so far + // try to append the module based on the mappings that + // we have found so far try_append_module(modules, &last_module); // start a new module based on the current mapping From 76f8de4ec0a8cc7957fd7c91c183517d8eee6845 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 6 Dec 2022 16:49:55 +0100 Subject: [PATCH 3/5] Update changelog. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 825fc9229..a5d9c913d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +**Fixes** + +- Linux module-finder now also searches for code-id in ".note" ELF sections ([#775](https://github.com/getsentry/sentry-native/pull/775)) + **Internal**: - CI: updated github actions to upgrade deprecated node runners. ([#767](https://github.com/getsentry/sentry-native/pull/767)) From 6d61c4d53d162c811c4da0b03da7f1ac8d849ac3 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 7 Dec 2022 10:29:17 +0100 Subject: [PATCH 4/5] Disable ERROR_ON_WARNINGS in macOS CI builds --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3afc36bb5..a9657a16d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,18 +61,21 @@ jobs: RUN_ANALYZER: code-checker,valgrind - name: macOS (xcode llvm) os: macOs-latest - ERROR_ON_WARNINGS: 1 + # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium + # ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 - name: macOS (xcode llvm + universal) os: macOs-latest - ERROR_ON_WARNINGS: 1 + # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium + # ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 CMAKE_DEFINES: -DCMAKE_OSX_ARCHITECTURES=arm64;x86_64 - name: macOS (clang + asan + llvm-cov) os: macOs-latest CC: clang CXX: clang++ - ERROR_ON_WARNINGS: 1 + # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium + # ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 RUN_ANALYZER: asan,llvm-cov - name: Windows (old VS, 32-bit) From d691280fcc84f79fbad590311f82b43e2e54bd84 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 7 Dec 2022 11:34:17 +0100 Subject: [PATCH 5/5] Enable ERROR_ON_WARNINGS and stick to macOS-11 for now --- .github/workflows/ci.yml | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9657a16d..ca414278f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,22 +60,19 @@ jobs: os: ubuntu-22.04 RUN_ANALYZER: code-checker,valgrind - name: macOS (xcode llvm) - os: macOs-latest - # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium - # ERROR_ON_WARNINGS: 1 + os: macOs-11 + ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 - name: macOS (xcode llvm + universal) - os: macOs-latest - # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium - # ERROR_ON_WARNINGS: 1 + os: macOs-11 + ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 CMAKE_DEFINES: -DCMAKE_OSX_ARCHITECTURES=arm64;x86_64 - name: macOS (clang + asan + llvm-cov) - os: macOs-latest + os: macOs-11 CC: clang CXX: clang++ - # temporarily disabled: with macOS 12 we get deprecation warnings in crashpad and minichromium - # ERROR_ON_WARNINGS: 1 + ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 RUN_ANALYZER: asan,llvm-cov - name: Windows (old VS, 32-bit)