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

2018-01-24 Thread Stephan Gerhold 
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   0x00 0x 0x 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   0x34 0x8034 0x8034 0x00100 0x00100 R   0x4
LOAD   0x00 0x8000 0x8000 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 
Cc: Emil Velikov 
Cc: Tapani Pälli 
Cc: 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104642
Fixes: 5c98d38 "util: Query build-id by symbol address, not library name"
---
 src/util/build_id.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/util/build_id.c b/src/util/build_id.c
index 536c74360e..fb67d160e3 100644
--- a/src/util/build_id.c
+++ b/src/util/build_id.c
@@ -58,7 +58,18 @@ build_id_find_nhdr_callback(struct dl_phdr_info *info, 
size_t size, void *data_)
 {
struct callback_data *data = data_;
 
-   if ((void *)info->dlpi_addr != data->dli_fbase)
+   /* Calculate address where shared object is mapped into the process space.
+* (Using the base address and the virtual address of the first LOAD 
segment)
+*/
+   void *map_start = NULL;
+   for (unsigned i = 0; i < info->dlpi_phnum; i++) {
+  if (info->dlpi_phdr[i].p_type == PT_LOAD) {
+ map_start = (void *)(info->dlpi_addr + info->dlpi_phdr[i].p_vaddr);
+ break;
+  }
+   }
+
+   if (map_start != data->dli_fbase)
   return 0;
 
for (unsigned i = 0; i < info->dlpi_phnum; i++) {
-- 
2.16.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-01-25 Thread Stephan Gerhold 
On Thu, Jan 25, 2018 at 11:22:10AM +, Emil Velikov wrote:
> On 24 January 2018 at 14:13, Stephan Gerhold  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   0x00 0x 0x 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   0x34 0x8034 0x8034 0x00100 0x00100 R   0x4
> > LOAD   0x00 0x8000 0x8000 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 
> > Cc: Emil Velikov 
> > Cc: Tapani Pälli 
> > Cc: 
> > 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?

[1]: http://www.openwall.com/lists/musl/2013/01/16/10
[2]: https://www.unix.com/man-page/FreeBSD/3/dladdr/

> Patch looks sensible, although input from Chad/Matt would be appreciated.
> Reviewed-by: Emil Velikov 
> 
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev