[PATCH] ldso: fix dlsym hang when reloading DSOs
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()).
In these cases we must not re-add the DSO to the global symbol
scope as it is already there. Or we end up corrupting the linked
list and further dlsym lookups will hang.
Signed-off-by: Timo Teräs
---
ldso/libdl/libdl.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ldso/libdl/libdl.c b/ldso/libdl/libdl.c
index c451b63..c901512 100644
--- a/ldso/libdl/libdl.c
+++ b/ldso/libdl/libdl.c
@@ -543,15 +543,23 @@ static void *do_dlopen(const char *libname, int flag,
ElfW(Addr) from)
#ifdef SHARED
/*
-* Get the tail of the list.
* In the static case doesn't need to extend the global scope, it is
* ready to be used as it is, because _dl_loaded_modules already points
* to the dlopened library.
+*
+* Extend the global scope by adding the local scope of the dlopened
DSO.
+* But only if it's not there. 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()).
*/
- for (ls = &_dl_loaded_modules->symbol_scope; ls && ls->next; ls =
ls->next);
-
- /* Extend the global scope by adding the local scope of the dlopened
DSO. */
- ls->next = &dyn_chain->dyn->symbol_scope;
+ for (ls = &_dl_loaded_modules->symbol_scope; ; ls = ls->next) {
+ if (ls == &dyn_chain->dyn->symbol_scope)
+ break;
+ if (ls->next == NULL) {
+ ls->next = &dyn_chain->dyn->symbol_scope;
+ break;
+ }
+ }
#endif
#ifdef __mips__
/*
--
1.8.3.1
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On 27/06/2013 8.34, 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()).
>
> In these cases we must not re-add the DSO to the global symbol
> scope as it is already there. Or we end up corrupting the linked
> list and further dlsym lookups will hang.
>
> Signed-off-by: Timo Teräs
Acked-by: Carmelo Amoroso
Cheers,
Carmelo
> ---
> ldso/libdl/libdl.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/ldso/libdl/libdl.c b/ldso/libdl/libdl.c
> index c451b63..c901512 100644
> --- a/ldso/libdl/libdl.c
> +++ b/ldso/libdl/libdl.c
> @@ -543,15 +543,23 @@ static void *do_dlopen(const char *libname, int flag,
> ElfW(Addr) from)
>
> #ifdef SHARED
> /*
> - * Get the tail of the list.
>* In the static case doesn't need to extend the global scope, it is
>* ready to be used as it is, because _dl_loaded_modules already points
>* to the dlopened library.
> + *
> + * Extend the global scope by adding the local scope of the dlopened
> DSO.
> + * But only if it's not there. 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()).
>*/
> - for (ls = &_dl_loaded_modules->symbol_scope; ls && ls->next; ls =
> ls->next);
> -
> - /* Extend the global scope by adding the local scope of the dlopened
> DSO. */
> - ls->next = &dyn_chain->dyn->symbol_scope;
> + for (ls = &_dl_loaded_modules->symbol_scope; ; ls = ls->next) {
> + if (ls == &dyn_chain->dyn->symbol_scope)
> + break;
> + if (ls->next == NULL) {
> + ls->next = &dyn_chain->dyn->symbol_scope;
> + break;
> + }
> + }
> #endif
> #ifdef __mips__
> /*
>
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
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. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On 27 June 2013 20:16:26 Rich Felker 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. Sent with AquaMail for Android http://www.aqua-mail.com ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On Thu, 27 Jun 2013 14:16:26 -0400 Rich Felker 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. It seems this is not the only case. As mentioned, NEEDED dependencies together with dlopen()/dlclose() can result in this too in some cases. Apparently usage_count refers to active handles from dlopen() - and can be zero if the lib is pulled in via NEEDED dependency only. Thus, this is the least intrusive fix. While restructuring dlopen() and dlclose() could fix this better, I did not have the time or interest to rewrite most of the libdl code. - Timo ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On Thu, 27 Jun 2013 21:36:14 +0300 Timo Teras wrote: > On Thu, 27 Jun 2013 14:16:26 -0400 > Rich Felker 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. > > It seems this is not the only case. As mentioned, NEEDED dependencies > together with dlopen()/dlclose() can result in this too in some cases. > Apparently usage_count refers to active handles from dlopen() - and > can be zero if the lib is pulled in via NEEDED dependency only. > > Thus, this is the least intrusive fix. While restructuring dlopen() > and dlclose() could fix this better, I did not have the time or > interest to rewrite most of the libdl code. Hum. Actually it might be that this happens with NODELETE flag only. Seems my test case had that instead of the weird combo. I'll test with NODELETE flag just adding usage_count by extra reference. Will post new patch if it works (or report back if it didn't). - Timo ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On Thu, Jun 27, 2013 at 09:36:14PM +0300, Timo Teras wrote: > On Thu, 27 Jun 2013 14:16:26 -0400 > Rich Felker 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. > > It seems this is not the only case. As mentioned, NEEDED dependencies > together with dlopen()/dlclose() can result in this too in some cases. > Apparently usage_count refers to active handles from dlopen() - and can > be zero if the lib is pulled in via NEEDED dependency only. This is also a bug in the reference counting. DT_NEEDED and dlopen should both be considered references for counting purposes. Otherwise you can end up with a reference count of zero when the library is still in use. Rich ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On Thu, 27 Jun 2013 20:36:44 +0200
"Bernhard Reutner-Fischer" wrote:
> On 27 June 2013 20:16:26 Rich Felker 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--;
- if (tpnt->usage_count == 0 && !(tpnt->rtld_flags &
RTLD_NODELETE)) {
+ if (tpnt->usage_count == 0) {
if ((tpnt->dynamic_info[DT_FINI]
|| tpnt->dynamic_info[DT_FINI_ARRAY])
&& need_fini
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On 27 June 2013 21:07:38 Timo Teras wrote:
On Thu, 27 Jun 2013 20:36:44 +0200
"Bernhard Reutner-Fischer" wrote:
> On 27 June 2013 20:16:26 Rich Felker 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.
Thanks,
- if (tpnt->usage_count == 0 && !(tpnt->rtld_flags &
RTLD_NODELETE)) {
+ if (tpnt->usage_count == 0) {
if ((tpnt->dynamic_info[DT_FINI]
|| tpnt->dynamic_info[DT_FINI_ARRAY])
&& need_fini
Sent with AquaMail for Android
http://www.aqua-mail.com
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] ldso: fix dlsym hang when reloading DSOs
On Thu, 27 Jun 2013 21:25:12 +0200
"Bernhard Reutner-Fischer" wrote:
> On 27 June 2013 21:07:38 Timo Teras wrote:
> > On Thu, 27 Jun 2013 20:36:44 +0200
> > "Bernhard Reutner-Fischer" wrote:
> >
> > > On 27 June 2013 20:16:26 Rich Felker 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
___
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc
