3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-03-09 22:26:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Use a relative address in .note.crashpad.info"

Browse Source 
This reverts commit 95e97a32eba4d505ab9591e683d2147c441eea48.

Reason for revert: arm64 lto build

Original change's description:
> Use a relative address in .note.crashpad.info
> 
> The desc value in the note is now the offset of CRASHPAD_INFO_SYMBOL
> from desc.
> 
> Making this note writable can trigger a linker error resulting in
> the binary embedding .note.crashpad.info to be rejected by the
> kernel during program loading.
> 
> The error was observed with:
> GNU ld (GNU Binutils for Debian) 2.30
> clang version 4.0.1-10 (tags/RELEASE_401/final)
> Debian 4.17.17-1rodete2
> 
> When the note is made writable, crashpad_snapshot_test contains two
> PT_LOAD segments which map to the same page.
> 
> LOAD         0x0000000000000000 0x0000000000000000 0x0000000000000000
>              0x0000000000000258 0x0000000000000258  R      0x200000
> LOAD         0x0000000000000258 0x0000000000000258 0x0000000000000258
>              0x00000000002b84d8 0x00000000002b8950  RWE    0x200000
> 
> Executing this binary with the execv system call triggers a segfault
> during program loading (an error can't be returned because the original
> process vm has already been discarded).
> 
> I suspect (I haven't set up a debuggable kernel) the failure occurs
> while attempting to map the second load segment because its virtual
> address, 0x258, is in the same page as the first load segment.
> https://elixir.bootlin.com/linux/v4.17.17/source/fs/binfmt_elf.c#L380
> 
> The linker normally produces consecutive load segments where the second
> segment is loaded 0x200000 bytes after the first, which I think is the
> maximum expected page size. Modifying the test executable to load the
> second segment at 0x1258 (4096 byte page size) allows program loading
> to succeed (but of course crashes after control is given to it).
> 
> Bug: crashpad:260
> Change-Id: I2b9f1e66e98919138baef3da991a9710bd970dc4
> Reviewed-on: https://chromium-review.googlesource.com/c/1292232
> Reviewed-by: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Joshua Peraza <jperaza@chromium.org>

TBR=scottmg@chromium.org,jperaza@chromium.org,mark@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: crashpad:260
Change-Id: I7a2c741e6b4c10d3e3b8be3213a8ce2cd93675f7
Reviewed-on: https://chromium-review.googlesource.com/c/1316372
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
This commit is contained in:
Scott Graham 2018-11-03 03:16:40 +00:00 committed by Commit Bot
parent 236ee1076c
commit 9ee48fb1be
7 changed files with 38 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
client/crashpad_info_note.S
View File

@ -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

18
snapshot/crashpad_info_size_test_note.S
View File

@ -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

12
snapshot/elf/elf_image_reader.cc
View File

@ -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<Elf64_Nhdr>(name, type, desc, desc_address)
: ReadNote<Elf32_Nhdr>(name, type, desc, desc_address);
result = range_->Is64Bit() ? ReadNote<Elf64_Nhdr>(name, type, desc)
: ReadNote<Elf32_Nhdr>(name, type, desc);
} while (retry_);
if (result == Result::kSuccess) {
@ -253,8 +251,7 @@ template <typename NhdrType>
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;

16
snapshot/elf/elf_image_reader.h
View File

@ -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 <typename T>
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_;

4
snapshot/elf/elf_image_reader_fuzzer.cc
View File

@ -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(
&note_name, &note_type, &note_desc, &desc_addr)) ==
while ((result = notes->NextNote(&note_name, &note_type, &note_desc)) ==
ElfImageReader::NoteReader::Result::kSuccess) {
LOG(ERROR) << note_name << note_type << note_desc;
}

10
snapshot/elf/elf_image_reader_test.cc
View File

@ -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<ElfImageReader::NoteReader> notes = reader.Notes(-1);
while ((result = notes->NextNote(
&note_name, &note_type, &note_desc, &desc_addr)) ==
while ((result = notes->NextNote(&note_name, &note_type, &note_desc)) ==
ElfImageReader::NoteReader::Result::kSuccess) {
}
EXPECT_EQ(result, ElfImageReader::NoteReader::Result::kNoMoreNotes);
notes = reader.Notes(0);
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_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(&note_name, &note_type, &note_desc, &desc_addr),
ASSERT_EQ(notes->NextNote(&note_name, &note_type, &note_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<decltype(kCrashpadNoteDesc)*>(&note_desc[0]),
kCrashpadNoteDesc);
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_desc),
ElfImageReader::NoteReader::Result::kNoMoreNotes);
}

15
snapshot/elf/module_snapshot_elf.cc
View File

@ -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<VMOffset*>(&desc[0]);
} else {
int32_t offset32 = *reinterpret_cast<int32_t*>(&desc[0]);
offset = offset32;
}
info_address = desc_address + offset;
info_address = *reinterpret_cast<VMAddress*>(&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<ElfImageReader::NoteReader> 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<const uint8_t*>(&desc[0]));