Re: [patch] Dangling Pointer in libltdl
* quoting myself:
* Dave Brolley wrote on Wed, Jan 24, 2007 at 11:14:56PM CET:
Given time, I should be able to come up with a test case if necessary.
I have a test. Will post and apply both when I have it cleaned up.
Here's what I've come up with and applied. The HEAD test can also be
made to work with branch-1-5's libltdl, by something like
make check-local TESTSUITEFLAGS='-k lt_dlexit' \
LTDLINCL=/path/to/branch-1-5/libltdl/ \
LIBLTDL=/path/to/branch-1-5/libltdl/libltdlc.la
LIBTOOL=/path/to/branch-1-5/libtool
(writing from memory, I think that was it).
Cheers,
Ralf
branch-1-5:
2007-01-28 Dave Brolley [EMAIL PROTECTED]
* libltdl/ltdl.c (lt_dlexit): Make sure that 'cur' is not NULL
before checking that it is still in the list.
Index: libltdl/ltdl.c
===
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.174.2.26
diff -u -r1.174.2.26 ltdl.c
--- libltdl/ltdl.c 28 Jan 2007 13:40:49 - 1.174.2.26
+++ libltdl/ltdl.c 28 Jan 2007 14:51:56 -
@@ -2341,6 +2341,19 @@
lt_dlhandle tmp = cur;
cur = cur-next;
if (!LT_DLIS_RESIDENT (tmp))
+ /* Make sure that the handle pointed to by 'cur' still exists.
+lt_dlclose recursively closes dependent libraries which
removes
+them from the linked list. One of these might be the one
+pointed to by 'cur'. */
+ if (cur)
+ {
+ for (tmp = handles; tmp; tmp = tmp-next)
+ if (tmp == cur)
+ break;
+ if (! tmp)
+ cur = handles;
+ }
+
saw_nonresident = 1;
if (!LT_DLIS_RESIDENT (tmp) tmp-info.ref_count = level)
{
HEAD:
2007-01-28 Dave Brolley [EMAIL PROTECTED]
Ralf Wildenhues [EMAIL PROTECTED]
* libltdl/ltdl.c (lt_dlexit): Make sure that 'cur' is not NULL
before checking that it is still in the list.
* tests/lt_dlexit.at: New test.
* Makefile.am (TESTSUITE_AT): Adjust.
(check-local): Also depend on libltdl/libltdlc.la.
(check-recursive): Removed, unnecessary use of Automake
internals.
Index: Makefile.am
===
RCS file: /cvsroot/libtool/libtool/Makefile.am,v
retrieving revision 1.204
diff -u -r1.204 Makefile.am
--- Makefile.am 28 Jan 2007 12:43:36 - 1.204
+++ Makefile.am 28 Jan 2007 14:50:59 -
@@ -411,6 +411,7 @@
tests/search-path.at \
tests/old-m4-iface.at \
tests/am-subdir.at \
+ tests/lt_dlexit.at \
tests/standalone.at \
tests/subproject.at \
tests/nonrecursive.at \
@@ -444,8 +445,6 @@
LIBTOOL=$(bindir)/`echo libtool | sed '$(program_transform_name)'` \
tst_aclocaldir=$(aclocaldir)
-check-recursive: $(srcdir)/$(TESTSUITE)
-
# Use `$(srcdir)' for the benefit of non-GNU makes: this is
# how `testsuite' appears in our dependencies.
$(srcdir)/$(TESTSUITE): $(srcdir)/tests/package.m4 $(TESTSUITE_AT) Makefile.am
@@ -471,7 +470,7 @@
CD_TESTDIR = abs_srcdir=`$(lt__cd) $(srcdir) pwd`; cd tests
# Hook the test suite into the check rule
-check-local: tests/atconfig $(srcdir)/$(TESTSUITE)
+check-local: tests/atconfig $(srcdir)/$(TESTSUITE) libltdl/libltdlc.la
$(CD_TESTDIR); \
CONFIG_SHELL=$(SHELL) $(SHELL) $$abs_srcdir/$(TESTSUITE) \
$(TESTS_ENVIRONMENT) $(BUILDCHECK_ENVIRONMENT) $(TESTSUITEFLAGS)
Index: libltdl/ltdl.c
===
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.245
diff -u -r1.245 ltdl.c
--- libltdl/ltdl.c 13 Oct 2006 14:11:18 - 1.245
+++ libltdl/ltdl.c 28 Jan 2007 14:50:59 -
@@ -283,6 +283,18 @@
{
++errors;
}
+ /* Make sure that the handle pointed to by 'cur' still
exists.
+lt_dlclose recursively closes dependent libraries
which removes
+them from the linked list. One of these might be the
one
+pointed to by 'cur'. */
+ if (cur)
+ {
+ for (tmp = handles; tmp; tmp = tmp-next)
+ if (tmp == cur)
+ break;
+ if (! tmp)
+ cur = handles;
+ }
}
}
}
--- /dev/null 2007-01-26 00:38:36.692344249 +0100
+++ tests/lt_dlexit.at 2007-01-28 15:44:32.0 +0100
@@ -0,0 +1,137 @@
+# Hand
Re: [patch] Dangling Pointer in libltdl
Hi Ralf and thanks for looking at this!
Thanks for the bug report.
* Dave Brolley wrote on Thu, Jan 18, 2007 at 07:39:23PM CET:
/ /
/ The attached patch fixes a problem with a dangling pointer in
lt_dlexit /
/ withing libltdl. The problem is that lt_dlclose is recursively
called /
/ (via unload_deplibs) in order to close dependent libraries. One
of these /
/ might be (and was for me!) the one pointed to by 'cur'./
I have trouble reproducing this bug easily. Which system does it happen
on?
It happened in a cygwin 1.6.9 environment on Windows XP Home edition.
How does the graph formed by modules/libraries and
interdependencies (linking against/dlopening) look like?
The linked list looked at the time like this:
lib1 - lib2 - lib3 - lib4 - etc.
What happed was that 'cur' was at the head of the list (pointing at
lib1). 'tmp' was then set to 'cur' and 'cur' was advanced to point to
lib2. At this point lt_dlclose was called using 'tmp' to close lib1.
This call determined that lib2 and lib3 were dependent libraries and
they were closed (and removed from the list) by recursive calls to
lt_dlclose. This left 'cur' pointing to unallocated memory which
eventually caused a crash.
In what order
are things opened/linked against, which ones are closed explicitly, for
this to trigger? Do you mix calls to lt_dlopen with direct calls to
dlopen? Do you mix libraries created with libtool with libraries
created without?
These questions will take some time to sort out. Hopefully you can
create a test case using the information I've given above in the mean
time. The application in question is Red Hat's SID simulator
(sources.redhat.com). Given time, I should be able to come up with a
test case if necessary.
/ @@ -283,10 +283,19 @@ lt_dlexit (void)/
/ {/
/ ++errors;/
/ }/
/ }/
/ }/
/ + /* Make sure that the handle pointed to by 'cur' still exists./
/ + lt_dlclose recursively closes dependent libraries which removes/
/ + them from the linked list. One of these might be the one/
/ + pointed to by 'cur'. *//
/ + for (tmp = handles; tmp; tmp = tmp-next)/
/ + if (tmp == cur)/
/ + break;/
/ + if (! tmp)/
/ + cur = handles;/
If the description is correct, the whole addition could go in the true
branch of the `if (tmp-info.ref_count = level)' test, no?
You are correct. I have attached a new patch which corrects this and
also corrects a problem with my previous patch. My previous patch causes
an infinite loop in the case that a resident library is in the linked
list. In this case 'cur' gets reset to 'handles' when the end of the
list is reached because 'tmp' ends up being NULL in my new loop. Because
of the resident library, 'handles' is not NULL and the list is traversed
repeatedly ad-infinitum. The fix is to make sure that 'cur' is not NULL
before searching for it in the list.
Thanks,
Dave
2007-01-24 Dave Brolley [EMAIL PROTECTED]
* libltdl/ltdl.c (lt_dlexit): Make sure that 'cur' is not NULL before
checking that it is still in the list.
Index: libltdl/ltdl.c
===
RCS file: /sources/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.245
diff -c -p -r1.245 ltdl.c
*** libltdl/ltdl.c 13 Oct 2006 14:11:18 - 1.245
--- libltdl/ltdl.c 24 Jan 2007 21:52:46 -
*** lt_dlexit (void)
*** 283,288
--- 283,300
{
++errors;
}
+ /* Make sure that the handle pointed to by 'cur' still
exists.
+lt_dlclose recursively closes dependent libraries
which removes
+them from the linked list. One of these might be the
one
+pointed to by 'cur'. */
+ if (cur)
+ {
+ for (tmp = handles; tmp; tmp = tmp-next)
+ if (tmp == cur)
+ break;
+ if (! tmp)
+ cur = handles;
+ }
}
}
}
___
Bug-libtool mailing list
Bug-libtool@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-libtool
Re: [patch] Dangling Pointer in libltdl
Hello Dave,
Thanks for the bug report.
* Dave Brolley wrote on Thu, Jan 18, 2007 at 07:39:23PM CET:
The attached patch fixes a problem with a dangling pointer in lt_dlexit
withing libltdl. The problem is that lt_dlclose is recursively called
(via unload_deplibs) in order to close dependent libraries. One of these
might be (and was for me!) the one pointed to by 'cur'.
I have trouble reproducing this bug easily. Which system does it happen
on? How does the graph formed by modules/libraries and
interdependencies (linking against/dlopening) look like? In what order
are things opened/linked against, which ones are closed explicitly, for
this to trigger? Do you mix calls to lt_dlopen with direct calls to
dlopen? Do you mix libraries created with libtool with libraries
created without?
I would like to apply a test case along with the fix. Easiest for me
would be to see a small example reproducing this (if you need help, I
can post an unfinished test case for this), or, failing that, the
original code that exposes the bug. Failing that, you could put up an
strace output of a program that exposes the bug. That way we should
hopefully be able to infer most information. But be sure to bzip2-pack
it if you must post it rather than putting it on some web page.
@@ -283,10 +283,19 @@ lt_dlexit (void)
{
++errors;
}
}
}
+ /* Make sure that the handle pointed to by 'cur' still exists.
+ lt_dlclose recursively closes dependent libraries which removes
+ them from the linked list. One of these might be the one
+ pointed to by 'cur'. */
+ for (tmp = handles; tmp; tmp = tmp-next)
+ if (tmp == cur)
+ break;
+ if (! tmp)
+ cur = handles;
If the description is correct, the whole addition could go in the true
branch of the `if (tmp-info.ref_count = level)' test, no?
Cheers, and thanks again,
Ralf
___
Bug-libtool mailing list
Bug-libtool@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-libtool
[patch] Dangling Pointer in libltdl
Hi,
The attached patch fixes a problem with a dangling pointer in lt_dlexit
withing libltdl. The problem is that lt_dlclose is recursively called
(via unload_deplibs) in order to close dependent libraries. One of these
might be (and was for me!) the one pointed to by 'cur'.
The patch makes sure that the handle pointed to by 'cur' still exists in
the linked list pointed to by 'handles' and, if it doesn't, resets it.
This patch is against the latest CVS sources of the libtool project as
of today.
Dave
2007-01-17 Dave Brolley [EMAIL PROTECTED]
* libltdl/ltdl.c (lt_dlexit): After each call to lt_dlclose, make sure
that the handle pointed to by 'cur' still exists.
Index: libltdl/ltdl.c
===
RCS file: /sources/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.245
diff -c -p -u -5 -r1.245 ltdl.c
--- libltdl/ltdl.c 13 Oct 2006 14:11:18 - 1.245
+++ libltdl/ltdl.c 18 Jan 2007 18:31:21 -
@@ -1,7 +1,7 @@
/* ltdl.c -- system independent dlopen wrapper
- Copyright (C) 1998, 1999, 2000, 2004, 2005, 2006 Free Software Foundation,
Inc.
+ Copyright (C) 1998, 1999, 2000, 2004, 2005, 2006, 2007 Free Software
Foundation, Inc.
Originally by Thomas Tanner [EMAIL PROTECTED]
NOTE: The canonical source of this file is maintained with the
GNU Libtool package. Report bugs to [EMAIL PROTECTED]
@@ -283,10 +283,19 @@ lt_dlexit (void)
{
++errors;
}
}
}
+ /* Make sure that the handle pointed to by 'cur' still exists.
+lt_dlclose recursively closes dependent libraries which removes
+them from the linked list. One of these might be the one
+pointed to by 'cur'. */
+ for (tmp = handles; tmp; tmp = tmp-next)
+ if (tmp == cur)
+ break;
+ if (! tmp)
+ cur = handles;
}
/* done if only resident modules are left */
if (!saw_nonresident)
break;
}
___
Bug-libtool mailing list
Bug-libtool@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-libtool
