Re: [PATCH 2/2] libdwfl: elf_from_remote_memory only trust shdrs of last file-only segment.

[Mark Wielaard <mjw at redhat dot com>]

On Tue, 2014-03-11 at 19:58 +0100, Jan Kratochvil wrote:
> Just I do not agree with the 'segments_end == segments_end_mem'
> and 'segments_end != segments_end_mem' conditions.  It does not try to test
> where the sections headers are located, just the conditions themselves are
> unrelated to the problem.  The conditions just accidentally match the Elfhack
> specific layout but they may both miss similar problem with other layouts and
> they also may have false positives.

The test whether the shdrs are included in the mapped in segments is
contents_size >= shdrs_end. And contents_size depends on segments_end
(the last actual mapped file contents of the segment). segments_end is
rounded up based on the page_size. The segments_end == segments_end_mem
check sees whether this rounding up can be trusted to actually contain
the file contents, or that the extra space will be used for something
else. It is no accident that catches the issue in this case. That was
the intent. Do you have a better check in mind? I don't see an easy way
to do it otherwise and not always loose the shdrs. Have you found any
case where the code gets it wrong?

> I would rather adjust elf_from_remote_memory() so that it considers
> 'contents_size' to span
>  * either just the first PT_LOAD segment

There is no guarantee that the first PT_LOAD contains all of the mapped
in ELF file contents.

>  * or just all non-writable PT_LOAD segments

This would be a nice one. But interesting sections like .dynamic (and
sometimes .dynsym and .dynstr) are not in read-only segments, also there
is no guarantee that a program/linker marks all read-only contents
really as a separate read-only segments, so we cannot just ignore
non-writable segments.

> or similar heuristics.  This is enough for all backtracing elfutils needs.
> The former use has been only for vDSO which matches all heuristics one can
> imagine:

The vDSO is indeed a little boring because it just has one fake PT_LOAD
segment that covers everything. So in that case we have everything we
need (not just the unwind data, but all interesting ELF data
accessible). The fixes to elf_from_remote_memory make sure that is also
the case for other mapped ELF files.

Cheers,

Mark