Re: [patch] validate binary before use

Aleksandar Ristovski
2013-02-21T21:00:54Z

On 13-02-02 07:25 AM, Jan Kratochvil wrote:
> On Fri, 01 Feb 2013 22:31:41 +0100, Aleksandar Ristovski wrote:
>> I will not claim familiarity with how linux works, but auxv
>> typically has information for the executable only, not the shared
>> objects
>
> OK, sorry, my comment was not appropriate, I remembered the function which
> finally end up in GDB incorrectly.
>
> If you are interested in shared library base VMA you could take link_map->l_ld
> and search /proc/PID/maps where it is located, subtract mapping offset and you
> get the base VMA where ELF headers starts. This is possible in
> gdbserver/linux-low.c but not possible in solib-svr4.c (which is cross-OS).

No, I will use l_addr if it works.

>
> gdbserver/linux-low.c currently does such computation for the main executable
> but it does not do it for any of the shared libraries.
>
> From ELF headers one can find Program headers, PT_NOTE and the build-id.
>
> BTW your patch currently verifies only shared libraries. I do not request so
> but a complete solution could verify also build-id of the executable.
>

That is deliberate since executable is already being validated. What
should be done is turn it into a 'reject' instead of a warning when a
mismatch is detected. The code paths are sufficiently different for
executable and other shared objects that executable validation can be
done in a separate patch.

>
>> Therefore, the only clue for deterministic and straight forward
>> relocation calculus for a tool like gdb is l_addr from the link map.
>
> Not from l_addr but l_ld is an absolute address of the DYNAMIC
> segment/section. From that one can derive something but only with
> /proc/PID/maps one can derive the ELF header VMA.
>
>
>> If gnu ld
>
> BTW this is more ld.so (PT_INTERP, /lib64/ld-linux-x86-64.so.2), not
> /usr/bin/ld.

Sorry, I didn't think there could have been any confusion - yes, we are
talking about dynamic linker, not link editor, I should have been clearer.

>
>
>> is not setting it up when "successful prelink" happens it
>> is making a mistake:
>
> As I said it is not a mistake but it was rather an incorrect comment at
> l_addr. l_addr is now officially defined as:
> + ElfW(Addr) l_addr; /* Difference between the address in the ELF
> + file and the addresses in memory. */
> http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=542f94662e8235d9917b0783df70bcdf9d729503
>

Yes, your new comment makes it clearer, but doesn't change my first
observation: this is a mistake.

>
>> 0 load base may be legitimate and true in some
>> cases on some systems. Furhter, it unnecessary introduces this
>> difference when it would probably change very little (nothing?) if
>> it set it up correctly to what the load base really is,
>
> Setting it to the absolute address would break tools like GDB which already
> expect it is the "difference" described above.

We set it to absolute load address - the sole purpose of link_map is to
allow kernel-agnostic (therefore /proc/ agnostic) way of traversing the
list of shared objects. Dynamic linker can have whatever internal
structures it wants to keep e.g. displacement (l_addr for gnu).

We do it in the logical way: l_addr is where the object is mapped, which
corresponds to first PT_LOAD. l_ld is, as on gnu (based on gdb code),
relocated address of dynamic section.

>
>
>> just as it does for "unsuccessful" prelinks or non-prelinked objects.
>
> It is not just "0 or the address". It can be arbitrary number if you prelink a
> library to address X and X is not free upon its loading so it gets placed at Y.
> Then L_ADDR is set to "Y - X" which is none of X, Y or 0.

The prelinking here introduces unnecessary confusion. Correct term would
be base address. Base address can be changed in the link-editor linker
script or with link-editor command line options. Dynamic linker is free
to load such shared object (providing it is shared i.e. DYN) at an
arbitrary, correctly aligned, virtual address. Hence 'load address' term
and differentiation between it and virtual address from pheaders.

This, however, has little to do with the semantics of l_addr which, as I
already stated, should be load base and not displacement.

In any case, this does not concern me too much, though I will try to
propose a patch where l_addr semantics will be configurable in the OS
abi somehow - something for another day.

Thanks,

Aleksandar