3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2024-12-26 23:01:05 +08:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
This is a reland of 95e97a32eba4d505ab9591e683d2147c441eea48

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>

Bug: crashpad:260
Change-Id: I66713de84cc26c9119e0454d19c9c189263fe054
Reviewed-on: https://chromium-review.googlesource.com/c/1318066
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
This commit is contained in:
Joshua Peraza 2018-11-06 13:19:06 -08:00 committed by Commit Bot
parent bf10ed0a69
commit 3663b7cbbe
7 changed files with 74 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
client/crashpad_info_note.S
View File

@ -26,16 +26,13 @@
#define NOTE_ALIGN 4
// This section must be "a"llocated so that it appears in the final binary at
// runtime, and "w"ritable so that the relocation to CRASHPAD_INFO_SYMBOL can
// be performed.
.section .note.crashpad.info,"aw",%note
// 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
.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.
.globl CRASHPAD_NOTE_REFERENCE
.hidden CRASHPAD_NOTE_REFERENCE
.type CRASHPAD_NOTE_REFERENCE, %object
CRASHPAD_NOTE_REFERENCE:
CRASHPAD_NOTE:
.long name_end - name // namesz
.long desc_end - desc // descsz
.long CRASHPAD_ELF_NOTE_TYPE_CRASHPAD_INFO // type
@ -45,15 +42,26 @@ name_end:
.balign NOTE_ALIGN
desc:
#if defined(__LP64__)
.quad CRASHPAD_INFO_SYMBOL
.quad CRASHPAD_INFO_SYMBOL - desc
#else
#if defined(__LITTLE_ENDIAN__)
.long CRASHPAD_INFO_SYMBOL
.long 0
#else
.long 0
.long CRASHPAD_INFO_SYMBOL
#endif // __LITTLE_ENDIAN__
.long CRASHPAD_INFO_SYMBOL - desc
#endif // __LP64__
desc_end:
.size CRASHPAD_NOTE_REFERENCE, .-CRASHPAD_NOTE_REFERENCE
.size CRASHPAD_NOTE, .-CRASHPAD_NOTE
// CRASHPAD_NOTE can't be referenced directly by GetCrashpadInfo() because the
// relocation used to make the reference may require that the address be
// 8-byte aligned and notes must have 4-byte alignment.
.section .rodata,"a",%progbits
.balign 8
# .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.
.globl CRASHPAD_NOTE_REFERENCE
.hidden CRASHPAD_NOTE_REFERENCE
.type CRASHPAD_NOTE_REFERENCE, %object
CRASHPAD_NOTE_REFERENCE:
// The value of this quad isn't important. It exists to reference
// CRASHPAD_NOTE, causing the linker to include the note into the binary
// linking Crashpad. The subtraction from |name| is a convenience to allow the
// value to be computed statically.
.quad name - CRASHPAD_NOTE

18
snapshot/crashpad_info_size_test_note.S
View File

@ -26,9 +26,11 @@
#define NOTE_ALIGN 4
// This section must be "a"llocated so that it appears in the final binary at
// runtime, and "w"ritable so that the relocation to TEST_CRASHPAD_INFO_SYMBOL
// can be performed.
.section .note.crashpad.info,"aw",%note
// 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
.balign NOTE_ALIGN
.type info_size_test_note, %object
info_size_test_note:
@ -41,15 +43,9 @@ name_end:
.balign NOTE_ALIGN
desc:
#if defined(__LP64__)
.quad TEST_CRASHPAD_INFO_SYMBOL
.quad TEST_CRASHPAD_INFO_SYMBOL - desc
#else
#if defined(__LITTLE_ENDIAN__)
.long TEST_CRASHPAD_INFO_SYMBOL
.long 0
#else
.long 0
.long TEST_CRASHPAD_INFO_SYMBOL
#endif // __LITTLE_ENDIAN__
.long TEST_CRASHPAD_INFO_SYMBOL - desc
#endif // __LP64__
desc_end:
.size info_size_test_note, .-info_size_test_note

12
snapshot/elf/elf_image_reader.cc
View File

@ -191,7 +191,8 @@ ElfImageReader::NoteReader::~NoteReader() = default;
ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::NextNote(
std::string* name,
NoteType* type,
std::string* desc) {
std::string* desc,
VMAddress* desc_address) {
if (!is_valid_) {
LOG(ERROR) << "invalid note reader";
return Result::kError;
@ -215,8 +216,9 @@ ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::NextNote(
}
retry_ = false;
result = range_->Is64Bit() ? ReadNote<Elf64_Nhdr>(name, type, desc)
: ReadNote<Elf32_Nhdr>(name, type, desc);
result = range_->Is64Bit()
? ReadNote<Elf64_Nhdr>(name, type, desc, desc_address)
: ReadNote<Elf32_Nhdr>(name, type, desc, desc_address);
} while (retry_);
if (result == Result::kSuccess) {
@ -251,7 +253,8 @@ template <typename NhdrType>
ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::ReadNote(
std::string* name,
NoteType* type,
std::string* desc) {
std::string* desc,
VMAddress* desc_address) {
static_assert(sizeof(*type) >= sizeof(NhdrType::n_namesz),
"Note field size mismatch");
DCHECK_LT(current_address_, segment_end_address_);
@ -317,6 +320,7 @@ 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,9 +70,14 @@ 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.
//! \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);
//! \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);
// private
NoteReader(const ElfImageReader* elf_reader_,
@ -88,7 +93,10 @@ 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);
Result ReadNote(std::string* name,
NoteType* type,
std::string* desc,
VMAddress* desc_addr);
VMAddress current_address_;
VMAddress segment_end_address_;

4
snapshot/elf/elf_image_reader_fuzzer.cc
View File

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

10
snapshot/elf/elf_image_reader_test.cc
View File

@ -154,22 +154,24 @@ 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)) ==
while ((result = notes->NextNote(
&note_name, &note_type, &note_desc, &desc_addr)) ==
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),
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
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),
ASSERT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
ElfImageReader::NoteReader::Result::kSuccess);
EXPECT_EQ(note_name, CRASHPAD_ELF_NOTE_NAME);
EXPECT_EQ(note_type,
@ -178,7 +180,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),
EXPECT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
ElfImageReader::NoteReader::Result::kNoMoreNotes);
}

15
snapshot/elf/module_snapshot_elf.cc
View File

@ -56,9 +56,17 @@ bool ModuleSnapshotElf::Initialize() {
kMaxNoteSize);
std::string desc;
VMAddress info_address;
if (notes->NextNote(nullptr, nullptr, &desc) ==
VMAddress desc_address;
if (notes->NextNote(nullptr, nullptr, &desc, &desc_address) ==
ElfImageReader::NoteReader::Result::kSuccess) {
info_address = *reinterpret_cast<VMAddress*>(&desc[0]);
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;
ProcessMemoryRange range;
if (range.Initialize(*elf_reader_->Memory())) {
@ -145,7 +153,8 @@ 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;
notes->NextNote(nullptr, nullptr, &desc);
VMAddress desc_addr;
notes->NextNote(nullptr, nullptr, &desc, &desc_addr);
desc.insert(desc.end(), 16 - std::min(desc.size(), size_t{16}), '\0');
uuid->InitializeFromBytes(reinterpret_cast<const uint8_t*>(&desc[0]));