diff --git a/client/crashpad_info_note.S b/client/crashpad_info_note.S index b60093c9..4c29298d 100644 --- a/client/crashpad_info_note.S +++ b/client/crashpad_info_note.S @@ -26,11 +26,9 @@ #define NOTE_ALIGN 4 // This section must be "a"llocated so that it appears in the final binary at - // runtime. The reference to CRASHPAD_INFO_SYMBOL uses an offset relative to - // this note to avoid making this note writable, which triggers a bug in GNU - // ld, or adding text relocations which require the target system to allow - // making text segments writable. https://crbug.com/crashpad/260. - .section .note.crashpad.info,"a",%note + // runtime, and "w"ritable so that the relocation to CRASHPAD_INFO_SYMBOL can + // be performed. + .section .note.crashpad.info,"aw",%note .balign NOTE_ALIGN # .globl indicates that it's available to link against other .o files. .hidden # indicates that it will not appear in the executable's symbol table. @@ -47,9 +45,15 @@ name_end: .balign NOTE_ALIGN desc: #if defined(__LP64__) - .quad CRASHPAD_INFO_SYMBOL - desc + .quad CRASHPAD_INFO_SYMBOL #else - .long CRASHPAD_INFO_SYMBOL - desc +#if defined(__LITTLE_ENDIAN__) + .long CRASHPAD_INFO_SYMBOL + .long 0 +#else + .long 0 + .long CRASHPAD_INFO_SYMBOL +#endif // __LITTLE_ENDIAN__ #endif // __LP64__ desc_end: .size CRASHPAD_NOTE_REFERENCE, .-CRASHPAD_NOTE_REFERENCE diff --git a/snapshot/crashpad_info_size_test_note.S b/snapshot/crashpad_info_size_test_note.S index 16b5d499..96b996db 100644 --- a/snapshot/crashpad_info_size_test_note.S +++ b/snapshot/crashpad_info_size_test_note.S @@ -26,11 +26,9 @@ #define NOTE_ALIGN 4 // This section must be "a"llocated so that it appears in the final binary at - // runtime. The reference to TEST_CRASHPAD_INFO_SYMBOL uses an offset relative - // to this note to avoid making this note writable, which triggers a bug in - // GNU ld, or adding text relocations which require the target system to allow - // making text segments writable. https://crbug.com/crashpad/260. - .section .note.crashpad.info,"a",%note + // runtime, and "w"ritable so that the relocation to TEST_CRASHPAD_INFO_SYMBOL + // can be performed. + .section .note.crashpad.info,"aw",%note .balign NOTE_ALIGN .type info_size_test_note, %object info_size_test_note: @@ -43,9 +41,15 @@ name_end: .balign NOTE_ALIGN desc: #if defined(__LP64__) - .quad TEST_CRASHPAD_INFO_SYMBOL - desc + .quad TEST_CRASHPAD_INFO_SYMBOL #else - .long TEST_CRASHPAD_INFO_SYMBOL - desc +#if defined(__LITTLE_ENDIAN__) + .long TEST_CRASHPAD_INFO_SYMBOL + .long 0 +#else + .long 0 + .long TEST_CRASHPAD_INFO_SYMBOL +#endif // __LITTLE_ENDIAN__ #endif // __LP64__ desc_end: .size info_size_test_note, .-info_size_test_note diff --git a/snapshot/elf/elf_image_reader.cc b/snapshot/elf/elf_image_reader.cc index ffe3f7b1..8ee51d34 100644 --- a/snapshot/elf/elf_image_reader.cc +++ b/snapshot/elf/elf_image_reader.cc @@ -191,8 +191,7 @@ ElfImageReader::NoteReader::~NoteReader() = default; ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::NextNote( std::string* name, NoteType* type, - std::string* desc, - VMAddress* desc_address) { + std::string* desc) { if (!is_valid_) { LOG(ERROR) << "invalid note reader"; return Result::kError; @@ -216,9 +215,8 @@ ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::NextNote( } retry_ = false; - result = range_->Is64Bit() - ? ReadNote(name, type, desc, desc_address) - : ReadNote(name, type, desc, desc_address); + result = range_->Is64Bit() ? ReadNote(name, type, desc) + : ReadNote(name, type, desc); } while (retry_); if (result == Result::kSuccess) { @@ -253,8 +251,7 @@ template ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::ReadNote( std::string* name, NoteType* type, - std::string* desc, - VMAddress* desc_address) { + std::string* desc) { static_assert(sizeof(*type) >= sizeof(NhdrType::n_namesz), "Note field size mismatch"); DCHECK_LT(current_address_, segment_end_address_); @@ -320,7 +317,6 @@ ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::ReadNote( current_address_, note_info.n_descsz, &local_desc[0])) { return Result::kError; } - *desc_address = current_address_; current_address_ = end_of_note; diff --git a/snapshot/elf/elf_image_reader.h b/snapshot/elf/elf_image_reader.h index f35c7616..317b35ad 100644 --- a/snapshot/elf/elf_image_reader.h +++ b/snapshot/elf/elf_image_reader.h @@ -70,14 +70,9 @@ class ElfImageReader { //! \param[out] name The name of the note owner, if not `nullptr`. //! \param[out] type A type for the note, if not `nullptr`. //! \param[out] desc The note descriptor. - //! \param[out] desc_addr The address in the remote process' address space - //! \a desc was read from. - //! \return a #Result value. \a name, \a type, \a desc, and \a desc_addr are - //! only valid if this method returns Result::kSuccess. - Result NextNote(std::string* name, - NoteType* type, - std::string* desc, - VMAddress* desc_addr); + //! \return a #Result value. \a name, \a type, and \a desc are only valid if + //! this method returns Result::kSuccess. + Result NextNote(std::string* name, NoteType* type, std::string* desc); // private NoteReader(const ElfImageReader* elf_reader_, @@ -93,10 +88,7 @@ class ElfImageReader { // and returns kError if use_filter_ is true and the note's name and type do // not match name_filter_ and type_filter_. template - Result ReadNote(std::string* name, - NoteType* type, - std::string* desc, - VMAddress* desc_addr); + Result ReadNote(std::string* name, NoteType* type, std::string* desc); VMAddress current_address_; VMAddress segment_end_address_; diff --git a/snapshot/elf/elf_image_reader_fuzzer.cc b/snapshot/elf/elf_image_reader_fuzzer.cc index 73bded72..1686650b 100644 --- a/snapshot/elf/elf_image_reader_fuzzer.cc +++ b/snapshot/elf/elf_image_reader_fuzzer.cc @@ -65,10 +65,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { std::string note_name; std::string note_desc; ElfImageReader::NoteReader::NoteType note_type; - VMAddress desc_addr; auto notes = reader.Notes(-1); - while ((result = notes->NextNote( - ¬e_name, ¬e_type, ¬e_desc, &desc_addr)) == + while ((result = notes->NextNote(¬e_name, ¬e_type, ¬e_desc)) == ElfImageReader::NoteReader::Result::kSuccess) { LOG(ERROR) << note_name << note_type << note_desc; } diff --git a/snapshot/elf/elf_image_reader_test.cc b/snapshot/elf/elf_image_reader_test.cc index 2cc0faa5..6d88d817 100644 --- a/snapshot/elf/elf_image_reader_test.cc +++ b/snapshot/elf/elf_image_reader_test.cc @@ -154,24 +154,22 @@ void ReadThisExecutableInTarget(ProcessType process, std::string note_name; std::string note_desc; ElfImageReader::NoteReader::NoteType note_type; - VMAddress desc_addr; std::unique_ptr notes = reader.Notes(-1); - while ((result = notes->NextNote( - ¬e_name, ¬e_type, ¬e_desc, &desc_addr)) == + while ((result = notes->NextNote(¬e_name, ¬e_type, ¬e_desc)) == ElfImageReader::NoteReader::Result::kSuccess) { } EXPECT_EQ(result, ElfImageReader::NoteReader::Result::kNoMoreNotes); notes = reader.Notes(0); - EXPECT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc, &desc_addr), + EXPECT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc), ElfImageReader::NoteReader::Result::kNoMoreNotes); // Find the note defined in elf_image_reader_test_note.S. constexpr uint32_t kCrashpadNoteDesc = 42; notes = reader.NotesWithNameAndType( CRASHPAD_ELF_NOTE_NAME, CRASHPAD_ELF_NOTE_TYPE_SNAPSHOT_TEST, -1); - ASSERT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc, &desc_addr), + ASSERT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc), ElfImageReader::NoteReader::Result::kSuccess); EXPECT_EQ(note_name, CRASHPAD_ELF_NOTE_NAME); EXPECT_EQ(note_type, @@ -180,7 +178,7 @@ void ReadThisExecutableInTarget(ProcessType process, EXPECT_EQ(*reinterpret_cast(¬e_desc[0]), kCrashpadNoteDesc); - EXPECT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc, &desc_addr), + EXPECT_EQ(notes->NextNote(¬e_name, ¬e_type, ¬e_desc), ElfImageReader::NoteReader::Result::kNoMoreNotes); } diff --git a/snapshot/elf/module_snapshot_elf.cc b/snapshot/elf/module_snapshot_elf.cc index 39385a6d..62a961d3 100644 --- a/snapshot/elf/module_snapshot_elf.cc +++ b/snapshot/elf/module_snapshot_elf.cc @@ -56,17 +56,9 @@ bool ModuleSnapshotElf::Initialize() { kMaxNoteSize); std::string desc; VMAddress info_address; - VMAddress desc_address; - if (notes->NextNote(nullptr, nullptr, &desc, &desc_address) == + if (notes->NextNote(nullptr, nullptr, &desc) == ElfImageReader::NoteReader::Result::kSuccess) { - VMOffset offset; - if (elf_reader_->Memory()->Is64Bit()) { - offset = *reinterpret_cast(&desc[0]); - } else { - int32_t offset32 = *reinterpret_cast(&desc[0]); - offset = offset32; - } - info_address = desc_address + offset; + info_address = *reinterpret_cast(&desc[0]); ProcessMemoryRange range; if (range.Initialize(*elf_reader_->Memory())) { @@ -153,8 +145,7 @@ void ModuleSnapshotElf::UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const { std::unique_ptr notes = elf_reader_->NotesWithNameAndType(ELF_NOTE_GNU, NT_GNU_BUILD_ID, 64); std::string desc; - VMAddress desc_addr; - notes->NextNote(nullptr, nullptr, &desc, &desc_addr); + notes->NextNote(nullptr, nullptr, &desc); desc.insert(desc.end(), 16 - std::min(desc.size(), size_t{16}), '\0'); uuid->InitializeFromBytes(reinterpret_cast(&desc[0]));