Debian Bug report logs - #26147
dlopen() corrupts heap, dlclose() reads free()'d memory

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Philippe Troin <phil@fifi.org>

To: submit@bugs.debian.org

Subject: dlopen() corrupts heap, dlclose() reads free()'d memory

Date: Tue, 25 Aug 1998 23:57:49 -0700

Package: libc6
Version: 2.0.7t-1
Severity: grave

(The above is for the debian bug tracking system)

>Submitter-Id:	net
>Originator:	Philippe Troin <phil@fifi.org>
>Organization:
>Confidential:	no
>Synopsis:	dlopen() corrupts heap, dlclose() reads free()'d memory
>Severity:	serious
>Priority:	high
>Category:	libc
>Class:		sw-bug
>Release:	libc-2.0.7
>Environment:   using libdl
Host type: i586-pc-linux-gnu
System: Linux ceramic 2.1.115 #1 SMP Sun Aug 9 14:58:48 PDT 1998 i686 unknown
Architecture: i686

Addons: crypt linuxthreads localedata
Build CFLAGS: -g -O2
Build CC: gcc -B$(common-objpfx)
Build shared: yes
Build profile: yes
Build omitfp: no
Stdio: libio

>Description:

* dlopen() uses two trailing NULLs to end the link map (cf dl-open.c).
  However the check to expand the link map only verifies that two slots
  are available, one will be used for the new link map entry, the other
  one will be used for the first trailing NULL. The second trailing
  NULL will overwrite whichever data sits after there (generally the
  heap structures).

* When dlclose() releases a library a big loop goes through all the library
  dependencies eventually unmapping them and finally free()ing the
  associated link_map. The problem is the loop is bound by map->l_nsearchlist
  which will be freed by one iteration of the loop (typically the last one).
  This is wrong, as free might blank out the memory or put garbage in it
  or even brk() or unmap the page.

>How-To-Repeat:

* Note that a (fixed) version of mcheck() was used to diagnose and fix these 
  problems (see an other mcheck() bug report).

* Problem one can be shown with this stupid program which will attempt to load 
  all the .so present in /usr/lib. The problem will show even faster if one
  uses some kind of malloc debugger like electric fence.

	#include <stdio.h>
	#include <stdlib.h>
	#include <string.h>
	#include <dirent.h>
	#include <errno.h>
	#include <dlfcn.h>
	#include <sys/types.h>
	
	#define LIB_DIR "/usr/lib"
	#define LIB_EXTENSION ".so"
	
	char *progname;
	
	inline void*
	max(void* lhs, void* rhs) 
	{
	  return lhs > rhs ? lhs : rhs ;
	}
	
	int
	main(int argc, char* argv[])
	{
	  DIR* dir;
	  struct dirent* dent;
	  char *ptr;
	  size_t lib_dir_length;
	  /**/
	  /* mcheck(0); */
	
	  /* Get our name */
	  ptr=progname=argv[0];
	  while ( *ptr ) 
	    {
	      if ( *ptr=='/' )
		progname = ++ptr;
	      else
		++ptr;
	    }
	
	  /* Open all the libraries in LIB_DIR */
	  dir = opendir(LIB_DIR);
	  if (!dir)
	    {
	      fprintf(stderr, "%s: opendir(\"%s\"): %s\n", 
		      progname, LIB_DIR, strerror(errno));
	      exit(1);
	    }
	
	  lib_dir_length = strlen ( LIB_DIR );
	  while ( ( dent = readdir(dir) ) ) 
	    {
	      size_t lib_length;
	      /**/
	
	      lib_length = strlen ( dent->d_name );
	      if ( strcmp ( LIB_EXTENSION, max( dent->d_name + lib_length - 3,
						dent->d_name ) ) == 0 )
		{
		  void* dl_handle;
		  char* lib_path;
		  /**/
	
		  lib_path = malloc ( lib_dir_length + 1 + lib_length + 1 );
		  sprintf ( lib_path, "%s/%s", LIB_DIR, dent->d_name );
		  dl_handle = dlopen ( lib_path , RTLD_GLOBAL | RTLD_LAZY );
		  if ( ! dl_handle ) 
		    {
		      fprintf(stderr, "%s: dlopen(\"%s\",...): %s\n",
			      progname, lib_path, dlerror());
		    }
		  free ( lib_path );
		}
	    }
	
	  exit(0);
	  /* Never reached */
	  return 0;
	}

* Problem 2 is more tricky to show, but using the malloc hooks present in libc
  to write random bits in free()'d memory will cause libdl to crash.

>Fix:

The following patch will cure both problems. Simple patch, but tough to find
:-)

diff -ru ../glibc-2.0.7t.deborig/elf/dl-close.c glibc-2.0.7t/elf/dl-close.c
--- ../glibc-2.0.7t.deborig/elf/dl-close.c	Thu May 21 12:01:35 1998
+++ glibc-2.0.7t/elf/dl-close.c	Tue Aug 25 22:14:20 1998
@@ -118,10 +118,12 @@
 	    imap->l_next->l_prev = imap->l_prev;
 	  if (imap->l_searchlist && imap->l_searchlist != list)
 	    free (imap->l_searchlist);
-	  free (imap);
+	  if (imap != map)
+	    free (imap);
 	}
     }
 
+  free (map);
   free (list);
 
   /* Notify the debugger those objects are finalized and gone.  */
diff -ru ../glibc-2.0.7t.deborig/elf/dl-open.c glibc-2.0.7t/elf/dl-open.c
--- ../glibc-2.0.7t.deborig/elf/dl-open.c	Thu May 21 12:01:37 1998
+++ glibc-2.0.7t/elf/dl-open.c	Tue Aug 25 21:44:19 1998
@@ -119,7 +119,7 @@
       else
 	{
 	  if (_dl_global_scope_alloc <
-	      (size_t) (_dl_global_scope_end - _dl_global_scope + 2))
+	      (size_t) (_dl_global_scope_end - _dl_global_scope + 3))
 	    {
 	      /* Must extend the list.  */
 	      struct link_map **new = realloc (_dl_global_scope,

Hopefully, this will be the last bug with libdl :-)

Phil.

Message #10 received at 26147@bugs.debian.org (full text, mbox, reply):

From: Philippe Troin <phil@fifi.org>

To: Andreas Jaeger <aj@arthur.rhein-neckar.de>

Cc: 26147@bugs.debian.org

Subject: Re: libc/759: dlopen() corrupts heap, dlclose() reads free()'d memory

Date: Thu, 27 Aug 1998 20:25:28 -0700

They're absolutely equivalent, and the _dl_close patch is more elegant too...
Will there be a 2.0.7.1 or a 2.7.0.8 as the _dl_open problem can crash any
application which opens shared libraries ? (the _dl_close is only for
"correctness" shall we say...)

Phil.

Andreas Jaeger wrote:

> Hi Philippe,
>
> Uli has made the appended changes for glibc 2.1 - and will change this
> also for glibc 2.0.7.  glibc 2.1 and 2.0.7 are slightly different and
> therefore the line numbers might not match.
>
> I've closed the report already since I'm convinced that Uli's patch is
> equivalent to yours.  If you see any problems  with them, please tell
> us.
>
> Thanks,
> Andreas
>
> 1998-08-26 17:48  Ulrich Drepper  <drepper@cygnus.com>
>
>         * elf/dl-close.c (_dl_close): Move map->l_nsearchlist value into local
>         variable so that map can be freed.
>         Reported by Philippe Troin <phil@fifi.org>.
>
>         * elf/dl-open.c (dl_open_worker): Correct test for extending global
>         scope array.
>         Patch by Philippe Troin <phil@fifi.org>.
>
> Index: elf/dl-close.c
> ===================================================================
> RCS file: /glibc/cvsfiles/libc/elf/dl-close.c,v
> retrieving revision 1.18
> retrieving revision 1.19
> diff -u -r1.18 -r1.19
> --- dl-close.c  1998/07/13 12:12:20     1.18
> +++ dl-close.c  1998/08/26 18:03:33     1.19
> @@ -38,6 +38,7 @@
>  _dl_close (struct link_map *map)
>  {
>    struct link_map **list;
> +  unsigned nsearchlist;
>    unsigned int i;
>
>    if (map->l_opencount == 0)
> @@ -56,9 +57,10 @@
>      }
>
>    list = map->l_searchlist;
> +  nsearchlist = map->l_nsearchlist;
>
>    /* Call all termination functions at once.  */
> -  for (i = 0; i < map->l_nsearchlist; ++i)
> +  for (i = 0; i < nsearchlist; ++i)
>      {
>        struct link_map *imap = list[i];
>        if (imap->l_opencount == 1 && imap->l_type == lt_loaded
> @@ -83,12 +85,12 @@
>    /* The search list contains a counted reference to each object it
>       points to, the 0th elt being MAP itself.  Decrement the reference
>       counts on all the objects MAP depends on.  */
> -  for (i = 0; i < map->l_nsearchlist; ++i)
> +  for (i = 0; i < nsearchlist; ++i)
>      --list[i]->l_opencount;
>
>    /* Check each element of the search list to see if all references to
>       it are gone.  */
> -  for (i = 0; i < map->l_nsearchlist; ++i)
> +  for (i = 0; i < nsearchlist; ++i)
>      {
>        struct link_map *imap = list[i];
>        if (imap->l_opencount == 0 && imap->l_type == lt_loaded)
> Index: elf/dl-open.c
> ===================================================================
> RCS file: /glibc/cvsfiles/libc/elf/dl-open.c,v
> retrieving revision 1.29
> retrieving revision 1.31
> diff -u -r1.29 -r1.31
> --- dl-open.c   1998/07/13 12:12:32     1.29
> +++ dl-open.c   1998/08/26 18:03:42     1.31
> @@ -165,8 +169,8 @@
>         }
>        else
>         {
> -         if (_dl_global_scope_end + 2
> -             == _dl_global_scope + _dl_global_scope_alloc)
> +         if (_dl_global_scope_end + 3
> +             > _dl_global_scope + _dl_global_scope_alloc)
>             {
>               /* Must extend the list.  */
>               struct link_map **new = realloc (_dl_global_scope,
>
> --
>  Andreas Jaeger   aj@arthur.rhein-neckar.de    jaeger@informatik.uni-kl.de
>   for pgp-key finger ajaeger@aixd1.rhrk.uni-kl.de
>

Message #15 received at 26147@bugs.debian.org (full text, mbox, reply):

From: Andreas Jaeger <aj@arthur.rhein-neckar.de>

To: Philippe Troin <phil@fifi.org>

Cc: 26147@bugs.debian.org

Subject: Re: libc/759: dlopen() corrupts heap, dlclose() reads free()'d memory

Date: 28 Aug 1998 07:00:38 +0200

>>>>> Philippe Troin writes:

 > They're absolutely equivalent, and the _dl_close patch is more elegant too...
 > Will there be a 2.0.7.1 or a 2.7.0.8 as the _dl_open problem can crash any
 > application which opens shared libraries ? (the _dl_close is only for
 > "correctness" shall we say...)

There's still no 2.0.7 yet.  What you're using as 2.0.7 is a snapshot
- a snapshot which should be better than the released 2.0.6, but still 
not a released version.  I'm expecting the great and real glibc 2.0.7
which includes your patch - and some more - since some weeks already
every day.  But every critical bug throws us back ;-(.

Andreas
-- 
 Andreas Jaeger   aj@arthur.rhein-neckar.de    jaeger@informatik.uni-kl.de
  for pgp-key finger ajaeger@aixd1.rhrk.uni-kl.de

Send a report that this bug log contains spam.