Re: [PATCH 03/23] elf: Partially initialize ld.so after static dlopen (bug 20802)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 13/05/2021 17:09, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> diff --git a/elf/dl-open.c b/elf/dl-open.c
>>> index 0887fc5cc5..7e018bb44c 100644
>>> --- a/elf/dl-open.c
>>> +++ b/elf/dl-open.c
>>> @@ -35,6 +35,7 @@
>>>  #include <libc-internal.h>
>>>  #include <array_length.h>
>>>  #include <libc-early-init.h>
>>> +#include <gnu/lib-names.h>
>>>  
>>>  #include <dl-dst.h>
>>>  #include <dl-prop.h>
>>> @@ -590,8 +591,20 @@ dl_open_worker (void *a)
>>>    /* So far, so good.  Now check the versions.  */
>>>    for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
>>>      if (new->l_searchlist.r_list[i]->l_real->l_versions == NULL)
>>> -      (void) _dl_check_map_versions (new->l_searchlist.r_list[i]->l_real,
>>> -				     0, 0);
>>> +      {
>>> +	struct link_map *map = new->l_searchlist.r_list[i]->l_real;
>>> +	(void) _dl_check_map_versions (map, 0, 0);
>>
>> Why the cast here?
> 
> I don't know, it awas in the old code.  Presumably to indicate that it's
> okay to ignore the return status.

I think it should be safer to remove it, as the current code does
(the function does not have the attribute to emit an warning when
it is ignored).

> 
>>> +void
>>> +__rtld_static_init (struct link_map *map)
>>> +{
>>> +  const ElfW(Sym) *sym
>>> +    = _dl_lookup_direct (map, "_rtld_global_ro",
>>> +                         0x9f28436a, /* dl_new_hash output.  */
>>> +                         "GLIBC_PRIVATE",
>>> +                         0x0963cf85); /* _dl_elf_hash output.  */
>>> +  assert (sym != NULL);
>>> +  struct rtld_global_ro *dl = DL_SYMBOL_ADDRESS (map, sym);
>>> +
>>> +  /* Perform partial initialization here.  Note that this runs before
>>> +     ld.so is relocated, so only members initialized without
>>> +     relocations can be written here.  */
>>
>> This is tricky, how can we be sure about which members do not need
>> relocations?
> 
> I think only initializers that have an address dependency trigger
> relocations.  Anything else will evaluate to a constant and directly be
> emitted by the assembler (as this is C, not C++).

My point is how can we be used that adding a initialization on
__rtld_static_init is safe? If it requires relocations, will loader
emit and issue to warn us?