rust-minidump / minidump-writer

Rust rewrite of breakpad's minidump_writer
MIT License
68 stars 17 forks source link

Read ELF build ids directly from the target process instead of mmap()ing libraries #71

Closed gabrielesvelto closed 7 months ago

gabrielesvelto commented 1 year ago

When populating the module list on Linux we extract the GNU build ID from each executable. To do so we first mmap() the entire file in the process writing the minidump and then extract the data from there. This has a couple of important drawbacks:

We could avoid all these issues by reading the ELF headers directly from the process we're dumping. goblin supports parsing an ELF file lazily, though one has to do it manually (see this example).

jld commented 1 year ago

goblin's API is a little suboptimal here; Elf::iter_note_headers seems to expect a &[u8] that starts at the beginning of the file and covers all of the PT_NOTE segments. But what we really want to do is:

  1. Read the ELF header
  2. Read the program headers
  3. Read the note segments

Although, if we expect that those will all be close to the start of the file — and in practice they seem to be within the first ~1k — we could just copy in some convenient small amount like 4k (in one syscall, if we have #72), and fall back to larger sizes if we get goblin::error::Error::Scroll(scroll::error::Error::BadOffset(…)).

We'd still want to use that lazy_parse feature for that, I think, because there are going to be headers that point to things outside the area we're reading and that we don't care about.

gabrielesvelto commented 1 year ago

We'd need to fetch some strings from the STRTAB to find the appropriate note, but we could do that lazily and it would require only a handful of extra system calls. The whole STRTAB for libxul.so is ~5MiB in my local build so we could also load it in its entirety if lazy-parsing doesn't work.

gabrielesvelto commented 1 year ago

Note: I've experimented with lazy parsing via goblin and it's working just fine.

lissyx commented 8 months ago

there's https://github.com/m4b/goblin/pull/391 to fix some of it?

lissyx commented 8 months ago

And in fact whole feature is in https://phabricator.services.mozilla.com/D199710

afranchuk commented 7 months ago

I'm working on this now (can't assign myself though), based off of https://phabricator.services.mozilla.com/D199710. Though depending on how expensive/slow ptrace PEEKDATA is, it'd still be a lot less memory reading (fewer ptrace calls) to lazy parse ourselves rather than using goblin (for portions of it, at least).