[Mesa-stable] [PATCH] util/build-id: Fix address comparison for binaries with LOAD vaddr > 0

[Emil Velikov]

On 25 January 2018 at 15:11, Stephan Gerhold <stephan at gerhold.net> wrote:
> On Thu, Jan 25, 2018 at 11:22:10AM +0000, Emil Velikov wrote:
>> On 24 January 2018 at 14:13, Stephan Gerhold <stephan at gerhold.net> wrote:
>> > build_id_find_nhdr_for_addr() fails to find the build-id if the first LOAD
>> > segment has a virtual address other than 0x0.
>> >
>> > For most shared libraries, the first LOAD segment has vaddr=0x0:
>> >
>> >     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>> >     LOAD           0x000000 0x00000000 0x00000000 0x2d2e26 0x2d2e26 R E 0x1000
>> >     LOAD           0x2d2e54 0x002d3e54 0x002d3e54 0x2e248 0x2f148 RW  0x1000
>> >
>> > However, compiling the Intel Vulkan driver as 32-bit binary on Android produces
>> > the following ELF header with vaddr=0x8000 instead:
>> >
>> >     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>> >     PHDR           0x000034 0x00008034 0x00008034 0x00100 0x00100 R   0x4
>> >     LOAD           0x000000 0x00008000 0x00008000 0x224a04 0x224a04 R E 0x1000
>> >     LOAD           0x225710 0x0022e710 0x0022e710 0x25988 0x27364 RW  0x1000
>> >
>> > build_id_find_nhdr_callback() compares the address of dli_fbase from dladdr()
>> > and dlpi_addr from dl_iterate_phdr(). With vaddr > 0, these point to a
>> > different memory address, e.g.:
>> >
>> >     dli_fbase=0xd8395000 (offset 0x8000)
>> >     dlpi_addr=0xd838d000
>> >
>> > At least on glibc and bionic (Android) dli_fbase refers to the address where
>> > the shared object is mapped into the process space, whereas dlpi_addr is just
>> > the base address for the vaddrs declared in the ELF header.
>> >
>> > To compare them correctly, we need to calculate the start of the mapping
>> > by adding the vaddr of the first LOAD segment to the base address.
>> >
>> > Cc: Chad Versace <chadversary at chromium.org>
>> > Cc: Emil Velikov <emil.velikov at collabora.com>
>> > Cc: Tapani Pälli <tapani.palli at intel.com>
>> > Cc: <mesa-stable at lists.freedesktop.org>
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104642
>> > Fixes: 5c98d38 "util: Query build-id by symbol address, not library name"
>> > ---
>> Based on my observation of glibc code and reading at the spec, I think
>> this is correct.
>> Admittedly the man page could be improved.
>>
>> FWIW I've poked the #musl people about this change last night, and
>> haven't heard any feedback yet.
>> Be that about a) our understanding of how it should work or b) musl's
>> implementation on the topic.
>
> I found a related discussion about the implementation of dli_fbase on the
> musl mailing list[1]. The FreeBSD man page for dladdr()[2] linked in the
> message on the musl mailing list is a bit more specific about dli_fbase:
>
>         "The base address at which the shared object is mapped into the
>         address space of the calling process."
>
> ... which is - at least as far as I understand it - exactly how glibc and
> bionic behave and the reason why we need this patch for LOAD vaddrs != 0.
>
> However, from what I've noticed when testing with musl, they seem to handle
> it unlike glibc/bionic/the FreeBSD man page. musl always returns the base
> address without the offset where the shared object is mapped.
>
> Technically, this means that this patch will break on musl in the rare
> situation that you actually link a shared library with LOAD vaddr != 0.
> However, considering that only they seem to handle it differently, this might
> be worth reporting to them instead?
>
musl does not have a bug tracker - according to the musl FAQ [A] and a
quick search.
Instead people should report bugs on IRC or ML :-\

I've poked the for the second time yesterday [on IRC] and will do a
final one tomorrow.
Modulo any objections (mesa or musl devs) I'll push this.

Thanks again Stephan.
Emil

[A] https://www.musl-libc.org/faq.html