[PATCH] Race in dl code: dl_open_worker and _dl_fini

[Sripathi Kodi]

Hi Jakub,

On Friday 13 April 2007 21:56, Jakub Jelinek wrote:
> On Mon, Dec 25, 2006 at 11:04:26AM +0530, Sripathi Kodi wrote:
> > I have attached a patch to this mail that implements the second solution
> > I have  explained below. I have  added a flag l_fini_called  to link_map
> > structure  and  I  have  used  that  flag  in  _dl_fini  to  ensure  the
> > destructors are called only once.
> >
> > Please let me know if either of these two approaches is acceptable.
>
> Using a bit-field for this has issues (but so did the old code).
> As that part of _dl_fini is executed without dl_load_lock, some other
> thread modifying any of the bit-fields in the same word might race
> with it.  Similarly --l->l_direct_opencount is unsafe, wonder if
> we need that at all, the app is going to exit anyway.

In _dl_fini, we could delay unlocking dl_load_lock till the end of the 
function, but it looks like other functions that modify these bit fields 
don't hold dl_load_lock. Since nobody else has reported races while accessing 
these bit fields, I wonder if it is okay to integrate my patch and leave the 
rest of the code as it is :-) What do you think?

>
> The more important question is, what is supposed to happen when
> calling functions from a library whose destructors have been run already.
>
> 	Jakub

Yes, _dl_fini seems to run the destructors even as other threads are using the 
library. However, I noticed that dl_close, in most cases, doesn't run the 
destructor of the shared library it is closing. It just comes out at this 
check:

  if (__builtin_expect (map->l_flags_1 & DF_1_NODELETE, 0)
      && map->l_init_called)
    /* Nope.  Do nothing.  */
    return;

We could put the same check in _dl_fini to prevent running the destructors if 
the library is in use. But since the process is exiting, I guess it is less 
harmful to run destructors in _dl_fini than running them in dl_close. 
However, running the destructor could surely cause some problem for other 
threads even before process exit happens. Do you have any suggestion about 
how we should handle this?

Thanks,
Sripathi.