[patch] Fix for BZ 16381 -- explicit loader invocation "ld.so ./a.out" on a PIE binary calls global ctors twice
Carlos O'Donell
carlos@redhat.com
Thu Mar 13 00:01:00 GMT 2014
On 03/12/2014 07:47 PM, Paul Pluzhnikov wrote:
> On Tue, Mar 4, 2014 at 4:01 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>
>> On 01/17/2014 06:36 PM, Paul Pluzhnikov wrote:
>
>>> This patch simply sets correct type for loading of the executable from
>>> the start.
>>>
>>> Tested on Linux / x86_64 with no regressions.
>>
>> OK to checkin if you fix the minor nit and explain my
>> question in the middle of the patch.
>
> Thanks for the review!
>
> Committed after minor fixes.
>
>>> @@ -1075,7 +1076,7 @@ of this helper program; chances are you did not intend to run this program.\n\
>>> else
>>> {
>>> HP_TIMING_NOW (start);
>>> - _dl_map_object (NULL, rtld_progname, lt_library, 0,
>>> + _dl_map_object (NULL, rtld_progname, lt_executable, 0,
>>
>> OK, but I'd like you to tell me you verified that the lt_library here
>> was previously ignored and set to lt_executable by the code in
>> _dl_map_object_from_fd.
>
> This took longer than I expected, _dl_map_object_from_fd is long and
> complicated :-(
It always does when the reviewer asks icky questions like this ;-)
> AFAICT, _dl_map_object_from_fd used passed-in l_type once here:
>
> l = _dl_new_object (realname, name, l_type, loader, mode, nsid);
>
> that sets l->l_type = l_type and performs no conditional logic; so safe
> (since it was resetting l->l_type to l_executable before returning).
>
> It then used l->l_type once here (before resetting it to lt_executable):
>
> case PT_TLS:
> ...
> /* If not loading the initial set of shared libraries,
> check whether we should permit loading a TLS segment. */
> if (__builtin_expect (l->l_type == lt_library, 1)
> /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did
> not set up TLS data structures, so don't use them now. */
> || __builtin_expect (GL(dl_tls_dtv_slotinfo_list) != NULL, 1))
> {
> /* Assign the next available module ID. */
> l->l_tls_modid = _dl_next_tls_modid ();
> break;
> }
>
> Here, I have changed the condition, and so the main executable no longer
> gets its l_tls_modid set to 1.
>
> ... which would be a problem, except that it is unconditionally set to 1
> soon thereafter by this code in rtld.c dl_main():
Agreed. I was only ever aware of the unconditional setting.
> case PT_TLS:
> if (ph->p_memsz > 0)
> {
> ...
> /* This image gets the ID one. */
> GL(dl_tls_max_dtv_idx) = main_map->l_tls_modid = 1;
> }
> break;
>
>
> Almost certainly the code in rtld.c did not expect the _dl_map_object_from_fd
> to have already set its l_tls_modid.
>
> So in the end, main_map->l_tls_modid still ends up as 1, and everything
> is happy. At least that's my story and I am sticking to it until proven
> wrong :-)
That's fine, what you described makes sense and roughly answers my question,
and leaves us with a more detailed trail in the mailing list for later
review if there is a problem.
Cheers,
Carlos.
More information about the Libc-alpha
mailing list