[PATCH] ldso: fix dlsym hang when reloading DSOs

[Timo Teras]

On Thu, 27 Jun 2013 21:25:12 +0200
"Bernhard Reutner-Fischer" <rep.dot.nop at gmail.com> wrote:

> On 27 June 2013 21:07:38 Timo Teras <timo.teras at iki.fi> wrote:
> > On Thu, 27 Jun 2013 20:36:44 +0200
> > "Bernhard Reutner-Fischer" <rep.dot.nop at gmail.com> wrote:
> >
> > > On 27 June 2013 20:16:26 Rich Felker <dalias at aerifal.cx> wrote:
> > > > On Thu, Jun 27, 2013 at 09:34:32AM +0300, Timo Teräs wrote:
> > > > > It can happen under certain cases that the DSO had refcount 0,
> > > > > but was already loaded. (NODELETE flag is set, or it is pulled
> > > > > in via both NEEDED dependency and explicit dlopen()).
> > > >
> > > > Wouldn't it be more logical to prevent this from happening by
> > > > not using a refcount of zero? For example, NODELETE could just
> > > > perform an extra increment on the refcnt so that it never
> > > > reaches 0. In essence, the NODELETE flag is a permanent
> > > > reference to the DSO.
> > > I was thinking the same, without having looked yet. This does not
> > > sound like the correct fix.
> >
> > This would be the alternate patch - but somehow I lost my original
> > test case. And qemu seems to now depend on GTK making the other
> > known issue also go away.
> >
> > Paolo, could you test if this fixes your problem also?
> >
> > diff --git a/ldso/ldso/dl-elf.c b/ldso/ldso/dl-elf.c
> > index 96459f3..71133dd 100644
> > --- a/ldso/ldso/dl-elf.c
> > +++ b/ldso/ldso/dl-elf.c
> > @@ -831,6 +831,8 @@ struct elf_resolve 
> > *_dl_load_elf_shared_library(unsigned rflags,
> >  #endif
> >  	(*rpnt)->dyn = tpnt;
> >  	tpnt->usage_count++;
> > +	if (rtld_flags & RTLD_NODELETE)
> > +		tpnt->usage_count++;
> >  #ifdef __LDSO_STANDALONE_SUPPORT__
> >  	tpnt->libtype = (epnt->e_type == ET_DYN) ? elf_lib :
> > elf_executable; #else
> > diff --git a/ldso/libdl/libdl.c b/ldso/libdl/libdl.c
> > index c451b63..035b56d 100644
> > --- a/ldso/libdl/libdl.c
> > +++ b/ldso/libdl/libdl.c
> > @@ -822,7 +822,7 @@ static int do_dlclose(void *vhandle, int
> > need_fini) _dl_handles = rpnt->next_handle;
> >  	_dl_if_debug_print("%s: usage count: %d\n",
> >  			handle->dyn->libname,
> > handle->dyn->usage_count);
> > -	if (handle->dyn->usage_count != 1 ||
> > (handle->dyn->rtld_flags & RTLD_NODELETE)) {
> > +	if (handle->dyn->usage_count != 1) {
> >  		handle->dyn->usage_count--;
> >  		free(handle);
> >  		return 0;
> > @@ -844,7 +844,7 @@ static int do_dlclose(void *vhandle, int
> > need_fini) for (j = 0; j < handle->init_fini.nlist; ++j) {
> >  		tpnt = handle->init_fini.init_fini[j];
> >  		tpnt->usage_count--;
> 
> We want to keep usage_count at at least 1 if NODELETE here in dlclose:
> tpnt->usage_count -= !(tpnt->rtld_flags & RTLD_NODELETE);
> Instead, or, better -- and then
> usage_count = MAX(usage_count, usage_count & RTLD_NODELETE);
> or something to that effect.

The first chunk's:
+	if (rtld_flags & RTLD_NODELETE)
+		tpnt->usage_count++;

Will ensure that the reference count never drops below one. There's no
way this extra reference gets deleted.

However, it seems that do_dlopen() has:
        if (tpnt->usage_count > 1) { // assume already dlopened }

So the extra ref count actually messes up this. This should be simple
enough to replace with test like:
	if (tpnt->init_flag & DL_OPENED)

But then I believe the the following search for previous dlopen handle
will fail. Perhaps it is not a problem since the same happens with .so
files that where NEEDED for the application and get later dlopened.

- Timo