13 Commits

Author SHA1 Message Date
Joshua Peraza
3663b7cbbe Reland "Use a relative address in .note.crashpad.info"
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>
2018-11-06 21:38:33 +00:00
Scott Graham
9ee48fb1be Revert "Use a relative address in .note.crashpad.info"
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>
2018-11-03 03:28:19 +00:00
Joshua Peraza
95e97a32eb 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>
2018-10-31 23:35:50 +00:00
Joshua Peraza
688dcfa22e android: handle modules loaded from zipfiles
Modules mapped from zipfiles will have mappings named for the zipfile
rather than the module name and an offset into that zipfile instead of
0.

Bug: crashpad:253, crashpad:254
Change-Id: I0503d13e7b80ba7bd1cc2d241633d9c68c98f1cd
Reviewed-on: https://chromium-review.googlesource.com/1232294
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-09-20 17:42:56 +00:00
Joshua Peraza
52ff1accbb linux: Fix locating modules with multiple mappings from offset 0
The general strategy used by Crashpad to determine loaded modules is to
read the link_map to get the addresses of the dynamic arrays for all
loaded modules. Those addresses can then be used to query the MemoryMap
to locate the module's mappings, and in particular the base mapping
from which Crashpad can parse the entire loaded ELF file.

ELF modules are typically loaded in several mappings with varying
permissions for different segments. The previous strategy used to find
the base mapping for a module was to search backwards from the mapping
for the dynamic array until a mapping from file offset 0 was found for
the same file. This fails when the file is mapped multiple times from
file offset 0, which can happen if the first page of the file contains
a GNU_RELRO segment.

This new strategy queries the MemoryMap for ALL mappings associated
with the dynamic array's mapping, mapped from offset 0. The consumer
(process_reader_linux.cc) can then determine which mapping is the
correct base by attempting to parse a module at that address and
corroborating the PT_DYNAMIC or program header table address from the
parsed module with the values Crashpad gets from the link_map or
auxiliary vector.

Bug: crashpad:30
Change-Id: Ibfcbba512e8fccc8c65afef734ea5640b71e9f70
Reviewed-on: https://chromium-review.googlesource.com/1139396
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2018-07-26 15:33:15 +00:00
Scott Graham
a45e88602b Skip ELF notes with a p_vaddr of zero
Don't attempt to read data if the note isn't in an allocated segment.
See investigation starting at
https://bugs.chromium.org/p/crashpad/issues/detail?id=220#c27 for
details.

Bug: crashpad:220, crashpad:30, crashpad:196
Change-Id: I60eaacb83ad00ef33bde9079d25cc23a59bdf2c8
Reviewed-on: https://chromium-review.googlesource.com/941507
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-02-28 21:21:44 +00:00
Joshua Peraza
3f0371cce2 Define ElfImageReader::Memory()
Bug: crashpad:30
Change-Id: Ibb6c6423736daa9fb248ac0c8d724244906236d4
Reviewed-on: https://chromium-review.googlesource.com/894447
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
2018-01-31 17:06:59 +00:00
Scott Graham
1f1657d573 Read either DT_HASH or DT_GNU_HASH to determine the size of DT_SYMTAB
Without the section headers for the symbol table, there's no direct way
to calculate the number of entries in the table.

DT_HASH and DT_GNU_HASH are auxiliary tables that are designed to make
symbol lookup faster. DT_HASH is the original and is theoretically
mandatory. DT_GNU_HASH is the new-and-improved, but is more complex.

In practice, however, an Android build (at least vs. API 16) has only
DT_HASH, and not DT_GNU_HASH, and a Fuchsia build has only DT_GNU_HASH
but not DT_HASH. So, both are tried.

This change does not actually use the data in these tables to improve
the speed of symbol lookup, but instead only uses them to correctly
terminate the linear search.

DT_HASH contains the total number of symbols in the symbol table fairly
directly because there is an entry for each symbol table entry in the
hash table, so the number is the same.

DT_GNU_HASH regrettably does not. Instead, it's necessary to walk the
buckets and chain structure to find the largest entry.

DT_GNU_HASH doesn't appear in any "real" documentation that I'm aware
of, other than the binutils code (at least as far as I know). Some
more-and-less-useful references:
- https://flapenguin.me/2017/04/24/elf-lookup-dt-hash/
- https://flapenguin.me/2017/05/10/elf-lookup-dt-gnu-hash/
- http://deroko.phearless.org/dt_gnu_hash.txt
- https://sourceware.org/ml/binutils/2006-10/msg00377.html

Change-Id: I7cfc4372f29efc37446f0931d22a1f790e44076f
Bug: crashpad:213, crashpad:196
Reviewed-on: https://chromium-review.googlesource.com/876879
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2018-01-30 22:40:17 +00:00
Scott Graham
dea19c7374 fuchsia: Port ElfImageReader and (some of) its tests
(Still need to avoid fork()-dependence for the non-self tests.)

Bug: crashpad:196
Change-Id: Ib34fe33c7ec295881c1f555995072d9ff742647f
Reviewed-on: https://chromium-review.googlesource.com/876650
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
2018-01-19 22:22:21 +00:00
Mark Mentovai
c04c05564d linux: Fix gcc -Wparentheses error from 7a0daa6989a9
../../snapshot/elf/elf_image_reader.cc:261:29: error: suggest parentheses around ‘-’ in operand of ‘&’ [-Werror=parentheses]
 #define PAD(x) ((x) + align - 1 & ~(align - 1))
                 ~~~~~~~~~~~~^~~
../../snapshot/elf/elf_image_reader.cc:262:26: note: in expansion of macro ‘PAD’
   size_t padded_namesz = PAD(note_info.n_namesz);
                          ^~~

Change-Id: I5cabc2741c8541ae9cf66933d35658e1cbc20912
Reviewed-on: https://chromium-review.googlesource.com/817575
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-12-08 22:53:11 +00:00
Joshua Peraza
7a0daa6989 Enable reading notes from ELF images
Bug: crashpad:30
Change-Id: Ie6c594b05c6d39a869ed30b7a7b49e6a6301cc65
Reviewed-on: https://chromium-review.googlesource.com/792539
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-12-05 19:27:37 +00:00
Dave Bort
6c9bd10a24 Use generic VM types in snapshot/elf
A step towards making these files usable by non-Linux systems.

Bug: crashpad:196
Change-Id: I2497fd7e3bcb5390ae1e6ae22902ab6f56b59dff
Reviewed-on: https://chromium-review.googlesource.com/685405
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Dave Bort <dbort@google.com>
2017-10-03 00:42:39 +00:00
Dave Bort
9e07a7148c Move linux/elf files to snapshot/elf
A step towards making these files usable by non-Linux systems.

Bug: crashpad:196
Change-Id: I1dc4304b1376a3a5e45228cf40b23f0367d3efa8
Reviewed-on: https://chromium-review.googlesource.com/685404
Commit-Queue: Dave Bort <dbort@google.com>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2017-10-02 21:54:34 +00:00