[bug-libextractor] [patch] fix textproc/libextractor double free() bug

[Vasil Dimov]

>Submitter-Id:  current-users
>Originator:    Vasil Dimov
>Organization:  DataMax
>Confidential:  no 
>Synopsis:      [patch] textproc/libextractor fix double free() bug
>Severity:      serious
>Priority:      high
>Category:      ports
>Class:         sw-bug
>Release:       FreeBSD 6.0-STABLE amd64
>Environment:

System: FreeBSD qlovarnika.bg.datamax 6.0-STABLE FreeBSD 6.0-STABLE #0: Tue Nov 
 8 09:24:59 EET 2005     address@hidden:/usr/obj/usr/src/sys/QLOVARNIKA  amd64

>Description:

Problem: `make check' for textproc/libextractor fails:
...
make  check-TESTS
lt-trivialtest in free(): error: chunk is already free
Abort trap (core dumped)
FAIL: trivialtest
PASS: keywordlisttest
lt-plugintest in free(): error: chunk is already free
Abort trap (core dumped)
FAIL: plugintest
lt-multiload in free(): error: chunk is already free
Abort trap (core dumped)
FAIL: multiload
=========================================
3 of 4 tests failed
Please report to address@hidden
=========================================
...

(NOTE: options AJ are set via malloc.conf(3))

Lets look closer to one of the failing progs:

$ cd libextractor-0.5.6a/src/test
$ ./.libs/lt-trivialtest
lt-trivialtest in free(): error: chunk is already free
Abort trap: 6 (core dumped)
$

gdb is useless here, because it segfaults itself, so I used printfs to
locate the problem.

the following is self-explanatory trace of the program, created by
inserting printfs in the appropriate places:
(the prog is being run via electricfence to make it crash as soon as
possible)

$ ./.libs/lt-trivialtest
extractor.c:216 le_ltdl_init begin
extractor.c:253 le_ltdl_init end
trivialtest.c:24 main begin
extractor.c:216 le_ltdl_init begin
extractor.c:235 le_ltdl_init old_dlsearchpath=0x801122fcc
extractor.c:237 le_ltdl_init 
old_dlsearchpath=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:253 le_ltdl_init end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x801122fcc
ltdl.c:4060 lt_dlsetsearchpath 
search_path=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:274 le_ltdl_fini end
trivialtest.c:28 main between
extractor.c:216 le_ltdl_init begin
extractor.c:235 le_ltdl_init old_dlsearchpath=0x802cd3fcc
extractor.c:237 le_ltdl_init 
old_dlsearchpath=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:253 le_ltdl_init end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x802cd3fcc
ltdl.c:4060 lt_dlsetsearchpath 
search_path=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:274 le_ltdl_fini end
trivialtest.c:31 main end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x802cd3fcc
Bus error: 10 (core dumped)
$

You see, the destructor le_ltdl_fini() is being called "unexpectedly" when
the program exits, but search_path=0x802cd3fcc has already been free()d by
the previous invocation.

Fix: it seems quite trivial, we just need to old_dlsearchpath = NULL
after free()ing it - lt_dlsetsearchpath() is checking if called with NULL
pointer and we are calling free() only if old_dlsearchpath is non-NULL.

With the included patch, output of trivialtest looks like:

$ ./.libs/lt-trivialtest
extractor.c:216 le_ltdl_init begin
extractor.c:253 le_ltdl_init end
trivialtest.c:24 main begin
extractor.c:216 le_ltdl_init begin
extractor.c:235 le_ltdl_init old_dlsearchpath=0x801122fcc
extractor.c:237 le_ltdl_init 
old_dlsearchpath=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:253 le_ltdl_init end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x801122fcc
ltdl.c:4060 lt_dlsetsearchpath 
search_path=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:274 le_ltdl_fini end
trivialtest.c:28 main between
extractor.c:216 le_ltdl_init begin
extractor.c:235 le_ltdl_init old_dlsearchpath=0x802cd3fcc
extractor.c:237 le_ltdl_init 
old_dlsearchpath=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:253 le_ltdl_init end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x802cd3fcc
ltdl.c:4060 lt_dlsetsearchpath 
search_path=/usr/lib/libextractor:/usr/local/lib/libextractor
extractor.c:274 le_ltdl_fini end
trivialtest.c:31 main end
extractor.c:258 le_ltdl_fini begin
ltdl.c:4058 lt_dlsetsearchpath search_path=0x0
ltdl.c:4060 lt_dlsetsearchpath search_path=(null)
extractor.c:274 le_ltdl_fini end
$

And of-course...
$ make check
...
==================
All 4 tests passed
==================
$

>How-To-Repeat:

export MALLOC_OPTIONS=AJ
cd /usr/ports/textproc/libextractor
make check

>Fix:

--- libextractor_free.diff begins here ---
diff -urN --exclude=CVS libextractor.orig/Makefile libextractor/Makefile
--- libextractor.orig/Makefile  Sun Nov  6 02:20:45 2005
+++ libextractor/Makefile       Wed Nov  9 12:55:44 2005
@@ -7,7 +7,7 @@
 
 PORTNAME=      libextractor
 PORTVERSION=   0.5.6a
-PORTREVISION=  1
+PORTREVISION=  2
 CATEGORIES=    textproc
 MASTER_SITES=  http://gnunet.org/libextractor/download/ \
                ${MASTER_SITE_GNU}
diff -urN --exclude=CVS libextractor.orig/files/patch-src::main::extractor.c 
libextractor/files/patch-src::main::extractor.c
--- libextractor.orig/files/patch-src::main::extractor.c        Thu Jan  1 
02:00:00 1970
+++ libextractor/files/patch-src::main::extractor.c     Wed Nov  9 12:02:48 2005
@@ -0,0 +1,14 @@
+--- src/main/extractor.c.orig  Wed Nov  9 11:57:54 2005
++++ src/main/extractor.c       Wed Nov  9 11:58:31 2005
+@@ -246,8 +246,10 @@
+ 
+ void __attribute__ ((destructor)) le_ltdl_fini(void) {
+   lt_dlsetsearchpath(old_dlsearchpath);
+-  if (old_dlsearchpath != NULL)
++  if (old_dlsearchpath != NULL) {
+     free(old_dlsearchpath);
++    old_dlsearchpath = NULL;
++  }
+ #ifdef MINGW
+   ShutdownWinEnv();
+ #endif
--- libextractor_free.diff ends here ---