Re: [review v3] Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]

[Carlos O'Donell <codonell@redhat.com>]

On 12/2/19 11:07 AM, Florian Weimer wrote:
> * Florian Weimer:
> 
>> +/* Mark the objects as NODELETE if required.  This is delayed until
>> +   after dlopen failure is not possible, so that _dl_close can clean
>> +   up objects if necessary.  */
>> +static void
>> +activate_nodelete (struct link_map *new, int mode)
>> +{
>> +  if (mode & RTLD_NODELETE || new->l_nodelete == link_map_nodelete_pending)
>> +    {
>> +      if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
>> +	_dl_debug_printf ("activating NODELETE for %s [%lu]\n",
>> +			  new->l_name, new->l_ns);
>> +      new->l_nodelete = link_map_nodelete_active;
>> +    }
>> +
>> +  for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
>> +    {
>> +      struct link_map *imap = new->l_searchlist.r_list[i];
>> +      if (imap->l_nodelete == link_map_nodelete_pending)
>> +	{
>> +	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
>> +	    _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
>> +			      imap->l_name, imap->l_ns);
>> +
>> +	  /* Only new objects should have set
>> +	     link_map_nodelete_pending.  Existing objects should not
>> +	     have gained any new dependencies and therefore cannot
>> +	     reach NODELETE status.  */
>> +	  assert (!imap->l_init_called || imap->l_type != lt_loaded);
> 
> This assert is incorrect because the NODELETE markers actually go in the
> other direction (from new loaded libraries to their dependencies).
> 
> I still need to write tests for this.  But I can submit a patch for the
> removal of the assert immediately, if so desired.

I'm happy with seeing two patches, one which fixes the regression for
Fedora Rawhide and OpenSUSE Tumbleweed, and another which expands the
testsuite coverage.

If I understood correctly it's because:

- A dlopen of libstdc++ ...
- Can be used by something else dlopen'd later with a STB_GNU_UNIQUE symbol...
- Causing libstdc++ to be marked NODLETE.

So libstdc++ is l_type == lt_loaded, and l_init_called == 1, which means bot
sides of the || are false.

-- 
Cheers,
Carlos.