[PATCH] Fix memory leak in dlerror.c

[Ben Boeckel]

Ulrich Drepper <drepper@redhat.com> wrote:
> You have to explain what you are trying to solve.  Without looking into
> this too deeply I'd say it's unnecessary.  The error strings are not
> tied to the loading/unloading.  When a new string is needed the old one
> get freed.  I don't think any other condition is needed.  It might
> indeed be harmful if the string is still in use.
> 
> Provide a self-contained test case which shows a problem.

There is a test case attached to the Red Hat Bugzilla bug I linked to
(attached here for completeness). It only occurs when pthreads is linked
which causes the thread-specific result to be initialized and then it is
not free'd upon unloading. I think the case which gets hit is when a new
string is needed and then the library is unloaded in which case the
memory allocated is leaked.

The fact that free_key_mem is documented but never called is the real
reason I think this patch is (in essence) valid. The check-for-NULL may
be better somewhere else, but that's a technical detail.

I think that the only case when the memory would still be in use would
be when manually unloading the library in which case, I would think, you
would have to manually be aware that you may not use memory libdl may
have allocated (last_result gets free'd so any pointers to that floating
around aren't valid any more; why special-case pthreads-using
applications?). Furthermore, special casing that I have to manually the
last string libdl used isn't viable IMHO. Is the thread-local storage
even accessible outside libdl without holding a pointer that was
returned somewhere?

--Ben
-------------- next part --------------
/* gcc -g -o dlerror_memleak dlerror_memleak.c -ldl -lpthread */

#include <dlfcn.h>
#include <stdio.h>

int main(int argc, char* argv[])
  {
    void* obj;
    int ret;

    if (argc < 2)
      {
	fprintf(stderr, "Warning: Calling `dlopen' on self\n");
      }

    obj = dlopen(argv[1], RTLD_LAZY);
    if (!obj)
      {
	fprintf(stderr, "Error: Failed to open `%s'\n", argv[1]);
	return 1;
      }

    ret = dlclose(obj);
    if (ret)
      {
	fprintf(stderr, "Error: Failed to close `%s'\n", argv[1]);
	return 1;
      }

    return 0;
  }
-------------- next part --------------
Thu Jul 01 00:10:38: ~
boeckb@bronto-burt % gcc -g -o dlerror_memleak dl.c -ldl
Thu Jul 01 00:11:12: ~
boeckb@bronto-burt % gcc -g -o dlerror_memleak_pthread dl.c -ldl -lpthread
Thu Jul 01 00:11:29: ~
boeckb@bronto-burt % valgrind-c ./dlerror_memleak
==24065== Memcheck, a memory error detector
==24065== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==24065== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==24065== Command: ./dlerror_memleak
==24065==
Warning: Calling `dlopen' on self
==24065==
==24065== FILE DESCRIPTORS: 4 open at exit.
==24065== Open AF_UNIX socket 3: <unknown>
==24065==    <inherited from parent>
==24065==
==24065== Open file descriptor 2: /dev/pts/6
==24065==    <inherited from parent>
==24065==
==24065== Open file descriptor 1: /dev/pts/6
==24065==    <inherited from parent>
==24065==
==24065== Open file descriptor 0: /dev/pts/6
==24065==    <inherited from parent>
==24065==
==24065==
==24065== HEAP SUMMARY:
==24065==     in use at exit: 0 bytes in 0 blocks
==24065==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==24065==
==24065== All heap blocks were freed -- no leaks are possible
==24065==
==24065== For counts of detected and suppressed errors, rerun with: -v
==24065== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)
Thu Jul 01 00:11:32: ~
boeckb@bronto-burt % valgrind-c ./dlerror_memleak_pthread
==24073== Memcheck, a memory error detector
==24073== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==24073== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==24073== Command: ./dlerror_memleak_pthread
==24073==
Warning: Calling `dlopen' on self
==24073==
==24073== FILE DESCRIPTORS: 4 open at exit.
==24073== Open AF_UNIX socket 3: <unknown>
==24073==    <inherited from parent>
==24073==
==24073== Open file descriptor 2: /dev/pts/6
==24073==    <inherited from parent>
==24073==
==24073== Open file descriptor 1: /dev/pts/6
==24073==    <inherited from parent>
==24073==
==24073== Open file descriptor 0: /dev/pts/6
==24073==    <inherited from parent>
==24073==
==24073==
==24073== HEAP SUMMARY:
==24073==     in use at exit: 32 bytes in 1 blocks
==24073==   total heap usage: 1 allocs, 0 frees, 32 bytes allocated
==24073==
==24073== 32 bytes in 1 blocks are still reachable in loss record 1 of 1
==24073==    at 0x4C26481: calloc (vg_replace_malloc.c:418)
==24073==    by 0x4E2D30F: _dlerror_run (in /lib64/libdl-2.12.90.so)
==24073==    by 0x4E2CEE0: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.12.90.so)
==24073==    by 0x400694: main (dl.c:16)
==24073==
==24073== LEAK SUMMARY:
==24073==    definitely lost: 0 bytes in 0 blocks
==24073==    indirectly lost: 0 bytes in 0 blocks
==24073==      possibly lost: 0 bytes in 0 blocks
==24073==    still reachable: 32 bytes in 1 blocks
==24073==         suppressed: 0 bytes in 0 blocks
==24073==
==24073== For counts of detected and suppressed errors, rerun with: -v
==24073== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20100807/8ca26bb8/attachment.sig>