Re: PING^1 [PATCH] Call _dl_open_check after relocation is finished [BZ #24259]

[Florian Weimer <fweimer at redhat dot com>]

* H. J. Lu:

> On Wed, Jun 26, 2019 at 10:22 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > diff --git a/elf/dl-open.c b/elf/dl-open.c
>> > index 12a4f8b853..a144a40790 100644
>> > --- a/elf/dl-open.c
>> > +++ b/elf/dl-open.c
>> > @@ -292,8 +292,6 @@ dl_open_worker (void *a)
>> >    _dl_debug_state ();
>> >    LIBC_PROBE (map_complete, 3, args->nsid, r, new);
>> >
>> > -  _dl_open_check (new);
>> > -
>> >    /* Print scope information.  */
>> >    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_SCOPES))
>> >      _dl_show_scope (new, 0);
>> > @@ -366,6 +364,11 @@ dl_open_worker (void *a)
>> >       _dl_relocate_object (l, l->l_scope, reloc_mode, 0);
>> >      }
>> >
>> > +  /* NB: Since _dl_open_check may throw an exception, it must be called
>> > +     after relocation is finished.   Otherwise, a shared object may be
>> > +     mmapped without relocation.  */
>> > +  _dl_open_check (new);
>> > +
>> >    /* If the file is not loaded now as a dependency, add the search
>> >       list of the newly loaded object to the scope.  */
>> >    bool any_tls = false;
>>
>> Presumably you test this using libpthread because it's NODELETE?
>
> Yes.

Ah, so it's related to bug 20839.

>> I'm not sure if this correct.  Won't this leave behind a relocated
>> NODELTE objects whose constructors have not run?  That would still leave
>> the process in a bad state.
>
> Such library is only mapped and relocated, but nothing else.   Next loading
> will continue and finish the rest of loading steps, including calling
> constructors.

But relocation can fail and unwind, too, as it happens in bug 20839.  So
at least the comment is misleading.

Clearly, moving around the check is not the proper fix, but maybe it's
the best option we have at present.

Have you investigate if we can remove again NODELETE mappings which have
been partially relocated?

Thanks,
Florian