I: [PATCH] realloc-related memory leaks fixes

Dmitry V. Levin ldv@alt-linux.org
Wed Dec 26 12:38:00 GMT 2001


Greetings!

I've reviewed current glibc code for common realloc usage errors.
Following problems have been found:

- 10 files contains classic memory leaks (patch attached);
- __argz_add_sep() contains possible memory leak; its semantics differs
  from other argz functions (patch attached);
- resolv/res_hconf.c seems to have memory leak (possible fix also
  attached);
- misc/err.c:convert_and_print() and misc/error.c:error_tail() functions
  contain ugly code filled with memory leaks; it have to be rewritten if
  ever used.
- nis/* also contains classic memory leaks to be fixed.
- io/fts.c contains memory leaks; I'd suggest to update code from BSD; for
  example, OpenBSD have these bugs fixed.


Regards,
	Dmitry

+-------------------------------------------------------------------------+
Dmitry V. Levin     mailto://ldv@alt-linux.org
ALT Linux Team      http://www.altlinux.ru/
Fandra Project      http://www.fandra.org/
+-------------------------------------------------------------------------+
UNIX is user friendly. It's just very selective about who its friends are.
-------------- next part --------------
2001-12-25  Dmitry V. Levin <ldv@alt-linux.org>

	* crypt/md5-crypt.c: Realloc error handling memory leak fix.
	* elf/chroot_canon.c: Likewise.
	* elf/dl-object.c: Likewise.
	* iconv/iconv_charmap.c: Likewise.
	* iconv/iconv_prog.c: Likewise.
	* libio/iogetdelim.c: Likewise.
	* locale/lc-time.c: Likewise.
	* stdlib/canonicalize.c: Likewise.
	* sunrpc/svc.c: Likewise.
	* sysdeps/generic/glob.c: Likewise.

diff -ur glibc-20011223~/crypt/md5-crypt.c glibc-20011223/crypt/md5-crypt.c
--- glibc-20011223~/crypt/md5-crypt.c	Fri Aug 31 05:45:02 2001
+++ glibc-20011223/crypt/md5-crypt.c	Tue Dec 25 13:17:04 2001
@@ -247,9 +247,11 @@
 
   if (buflen < needed)
     {
+      char *new_buffer;
       buflen = needed;
-      if ((buffer = realloc (buffer, buflen)) == NULL)
+      if ((new_buffer = realloc (buffer, buflen)) == NULL)
 	return NULL;
+      buffer = new_buffer;
     }
 
   return __md5_crypt_r (key, salt, buffer, buflen);
diff -ur glibc-20011223~/elf/chroot_canon.c glibc-20011223/elf/chroot_canon.c
--- glibc-20011223~/elf/chroot_canon.c	Fri Jul  6 08:54:45 2001
+++ glibc-20011223/elf/chroot_canon.c	Tue Dec 25 12:45:08 2001
@@ -94,16 +94,18 @@
 	  if (dest + (end - start) >= rpath_limit)
 	    {
 	      ptrdiff_t dest_offset = dest - rpath;
+	      char *new_rpath;
 
 	      new_size = rpath_limit - rpath;
 	      if (end - start + 1 > PATH_MAX)
 		new_size += end - start + 1;
 	      else
 		new_size += PATH_MAX;
-	      rpath = realloc (rpath, new_size);
+	      new_rpath = realloc (rpath, new_size);
+	      if (new_rpath == NULL)
+		goto error;
+	      rpath = new_rpath;
 	      rpath_limit = rpath + new_size;
-	      if (rpath == NULL)
-		return NULL;
 
 	      dest = rpath + dest_offset;
 	    }
diff -ur glibc-20011223~/elf/dl-object.c glibc-20011223/elf/dl-object.c
--- glibc-20011223~/elf/dl-object.c	Fri Nov  9 01:31:53 2001
+++ glibc-20011223/elf/dl-object.c	Tue Dec 25 13:01:19 2001
@@ -121,11 +121,20 @@
 	  origin = NULL;
 	  do
 	    {
+	      char *new_origin;
 	      len += 128;
-	      origin = (char *) realloc (origin, len);
+	      new_origin = (char *) realloc (origin, len);
+	      if (new_origin == NULL)
+	        {
+		  /* We were not able to allocate enough memory.
+		     Note that free(origin) is OK if origin == NULL.  */
+		  free (origin);
+		  origin = (char *) -1;
+		  goto out;
+		}
+	      origin = new_origin;
 	    }
-	  while (origin != NULL
-		 && (result = __getcwd (origin, len - realname_len)) == NULL
+	  while ((result = __getcwd (origin, len - realname_len)) == NULL
 		 && errno == ERANGE);
 
 	  if (result == NULL)
diff -ur glibc-20011223~/iconv/iconv_charmap.c glibc-20011223/iconv/iconv_charmap.c
--- glibc-20011223~/iconv/iconv_charmap.c	Fri Jul  6 08:54:47 2001
+++ glibc-20011223/iconv/iconv_charmap.c	Tue Dec 25 13:21:39 2001
@@ -516,12 +516,17 @@
     while (1)
       {
 	ssize_t n;
+	char *new_inbuf;
 
 	/* Increase the buffer.  */
+	new_inbuf = realloc (inbuf, maxlen + 32768);
+	if (new_inbuf == NULL)
+	  {
+	    error (0, errno, _("unable to allocate buffer for input"));
+	    return -1;
+	  }
 	maxlen += 32768;
-	inbuf = realloc (inbuf, maxlen);
-	if (inbuf == NULL)
-	  error (0, errno, _("unable to allocate buffer for input"));
+	inbuf = new_inbuf;
 	inptr = inbuf + actlen;
 
 	do
diff -ur glibc-20011223~/iconv/iconv_prog.c glibc-20011223/iconv/iconv_prog.c
--- glibc-20011223~/iconv/iconv_prog.c	Thu Nov 29 07:59:08 2001
+++ glibc-20011223/iconv/iconv_prog.c	Tue Dec 25 13:26:37 2001
@@ -516,12 +516,17 @@
     while (1)
       {
 	ssize_t n;
+	char *new_inbuf;
 
 	/* Increase the buffer.  */
+	new_inbuf = realloc (inbuf, maxlen + 32768);
+	if (new_inbuf == NULL)
+	  {
+	    error (0, errno, _("unable to allocate buffer for input"));
+	    return -1;
+	  }
 	maxlen += 32768;
-	inbuf = realloc (inbuf, maxlen);
-	if (inbuf == NULL)
-	  error (0, errno, _("unable to allocate buffer for input"));
+	inbuf = new_inbuf;
 	inptr = inbuf + actlen;
 
 	do
diff -ur glibc-20011223~/libio/iogetdelim.c glibc-20011223/libio/iogetdelim.c
--- glibc-20011223~/libio/iogetdelim.c	Fri Jul  6 08:54:55 2001
+++ glibc-20011223/libio/iogetdelim.c	Tue Dec 25 13:37:17 2001
@@ -96,15 +96,22 @@
       needed = cur_len + len + 1;
       if (needed > *n)
 	{
+	  char *new_lineptr;
 	  if (needed < 2 * *n)
 	    needed = 2 * *n;  /* Be generous. */
 	  *n = needed;
-	  *lineptr = (char *) realloc (*lineptr, needed);
-	  if (*lineptr == NULL)
+	  new_lineptr = (char *) realloc (*lineptr, needed);
+	  if (new_lineptr == NULL)
 	    {
+	      if (*lineptr)
+	        {
+	          free (*lineptr);
+		  lineptr = NULL;
+		}
 	      result = -1;
 	      goto unlock_return;
 	    }
+	  *lineptr = new_lineptr;
 	}
       memcpy (*lineptr + cur_len, (void *) fp->_IO_read_ptr, len);
       fp->_IO_read_ptr += len;
diff -ur glibc-20011223~/locale/lc-time.c glibc-20011223/locale/lc-time.c
--- glibc-20011223~/locale/lc-time.c	Fri Aug 10 06:02:17 2001
+++ glibc-20011223/locale/lc-time.c	Tue Dec 25 14:10:00 2001
@@ -74,19 +74,25 @@
 	}
       else
 	{
+	  struct era_entry *new_eras = eras;
 	  if (num_eras != new_num_eras)
-	    eras = (struct era_entry *) realloc (eras,
+	    new_eras = (struct era_entry *) realloc (eras,
 						 new_num_eras
 						 * sizeof (struct era_entry));
-	  if (eras == NULL)
+	  if (new_eras == NULL)
 	    {
+	      if (eras)
+	        {
+		  free (eras);
+		  eras = NULL;
+		}
 	      num_eras = 0;
-	      eras = NULL;
 	    }
           else
 	    {
 	      const char *ptr = _NL_CURRENT (LC_TIME, _NL_TIME_ERA_ENTRIES);
 	      num_eras = new_num_eras;
+	      eras = new_eras;
 
 	      for (cnt = 0; cnt < num_eras; ++cnt)
 		{
diff -ur glibc-20011223~/stdlib/canonicalize.c glibc-20011223/stdlib/canonicalize.c
--- glibc-20011223~/stdlib/canonicalize.c	Fri Jul  6 08:55:41 2001
+++ glibc-20011223/stdlib/canonicalize.c	Mon Dec 24 21:30:00 2001
@@ -122,6 +122,7 @@
 	  if (dest + (end - start) >= rpath_limit)
 	    {
 	      ptrdiff_t dest_offset = dest - rpath;
+	      char *new_rpath;
 
 	      if (resolved)
 		{
@@ -136,10 +137,11 @@
 		new_size += end - start + 1;
 	      else
 		new_size += path_max;
-	      rpath = realloc (rpath, new_size);
+	      new_rpath = realloc (rpath, new_size);
+	      if (new_rpath == NULL)
+		goto error;
+	      rpath = new_rpath;
 	      rpath_limit = rpath + new_size;
-	      if (rpath == NULL)
-		return NULL;
 
 	      dest = rpath + dest_offset;
 	    }
diff -ur glibc-20011223~/sunrpc/svc.c glibc-20011223/sunrpc/svc.c
--- glibc-20011223~/sunrpc/svc.c	Tue Mar 20 21:34:22 2001
+++ glibc-20011223/sunrpc/svc.c	Tue Dec 25 15:20:44 2001
@@ -86,6 +86,8 @@
 
   if (sock < _rpc_dtablesize ())
     {
+      struct pollfd *new_svc_pollfd;
+
       xports[sock] = xprt;
       if (sock < FD_SETSIZE)
 	FD_SET (sock, &svc_fdset);
@@ -100,12 +102,13 @@
 	    return;
 	  }
 
-      ++svc_max_pollfd;
-      svc_pollfd = realloc (svc_pollfd,
-			    sizeof (struct pollfd) * svc_max_pollfd);
-      if (svc_pollfd == NULL) /* Out of memory */
+      new_svc_pollfd = realloc (svc_pollfd,
+			    sizeof (struct pollfd) * (svc_max_pollfd + 1));
+      if (new_svc_pollfd == NULL) /* Out of memory */
 	return;
 
+      ++svc_max_pollfd;
+      svc_pollfd = new_svc_pollfd;
       svc_pollfd[svc_max_pollfd - 1].fd = sock;
       svc_pollfd[svc_max_pollfd - 1].events = (POLLIN | POLLPRI |
 					       POLLRDNORM | POLLRDBAND);
diff -ur glibc-20011223~/sysdeps/generic/glob.c glibc-20011223/sysdeps/generic/glob.c
--- glibc-20011223~/sysdeps/generic/glob.c	Mon Dec 10 16:00:27 2001
+++ glibc-20011223/sysdeps/generic/glob.c	Tue Dec 25 14:00:21 2001
@@ -836,13 +836,13 @@
 	       : (__stat64 (dirname, &st64) == 0 && S_ISDIR (st64.st_mode)))))
 	{
 	  int newcount = pglob->gl_pathc + pglob->gl_offs;
-
-	  pglob->gl_pathv
-	    = (char **) realloc (pglob->gl_pathv,
+	  char **new_gl_pathv = (char **) realloc (pglob->gl_pathv,
 				 (newcount + 1 + 1) * sizeof (char *));
-	  if (pglob->gl_pathv == NULL)
+	  if (new_gl_pathv == NULL)
 	    return GLOB_NOSPACE;
 
+	  pglob->gl_pathv = new_gl_pathv;
+
 #if defined HAVE_STRDUP || defined _LIBC
 	   pglob->gl_pathv[newcount] = strdup (dirname);
 #else
@@ -954,17 +954,15 @@
 	  if (flags & GLOB_NOCHECK)
 	    {
 	      int newcount = pglob->gl_pathc + pglob->gl_offs;
-
-	      pglob->gl_pathv
-		= (char **) realloc (pglob->gl_pathv,
-				     (newcount + 2)
-				     * sizeof (char *));
-	      if (pglob->gl_pathv == NULL)
+	      char **new_gl_pathv = (char **) realloc (pglob->gl_pathv,
+				     (newcount + 2) * sizeof (char *));
+	      if (new_gl_pathv == NULL)
 		{
 		  globfree (&dirs);
 		  return GLOB_NOSPACE;
 		}
 
+	      pglob->gl_pathv = new_gl_pathv;
 	      pglob->gl_pathv[newcount] = __strdup (pattern);
 	      if (pglob->gl_pathv[newcount] == NULL)
 		{
@@ -1386,12 +1384,13 @@
 
   if (nfound != 0)
     {
-      pglob->gl_pathv
-	= (char **) realloc (pglob->gl_pathv,
+      char **new_gl_pathv = (char **) realloc (pglob->gl_pathv,
 			     (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
 			     * sizeof (char *));
-      if (pglob->gl_pathv == NULL)
+      if (new_gl_pathv == NULL)
 	goto memory_error;
+
+      pglob->gl_pathv = new_gl_pathv;
 
       for (; names != NULL; names = names->next)
 	pglob->gl_pathv[pglob->gl_offs + pglob->gl_pathc++] = names->name;
-------------- next part --------------
2001-12-25  Dmitry V. Levin <ldv@alt-linux.org>

	* string/argz-addsep.c: Realloc error handling possible memory leak fix.
	
--- glibc-20011223~/string/argz-addsep.c	Fri Jul  6 08:55:41 2001
+++ glibc-20011223/string/argz-addsep.c	Mon Dec 24 21:40:59 2001
@@ -32,11 +32,12 @@
     {
       const char *rp;
       char *wp;
+      char *new_argz = (char *) realloc (*argz, *argz_len + nlen);
 
-      *argz = (char *) realloc (*argz, *argz_len + nlen);
-      if (*argz == NULL)
+      if (new_argz == NULL)
 	return ENOMEM;
 
+      *argz = new_argz;
       wp = *argz + *argz_len;
       rp = string;
       do
-------------- next part --------------
2001-12-25  Dmitry V. Levin <ldv@alt-linux.org>

	* resolv/res_hconf.c: Possible memory leak fix.
	
--- glibc-20011223~/resolv/res_hconf.c	Fri Aug 17 10:53:50 2001
+++ glibc-20011223/resolv/res_hconf.c	Tue Dec 25 15:16:40 2001
@@ -527,6 +527,7 @@
       int sd, num, i;
       /* Save errno.  */
       int save = errno;
+      struct netaddr *new_ifaddrs;
 
       /* Initialize interface table.  */
 
@@ -544,10 +545,12 @@
       if (!ifr)
 	goto cleanup;
 
-      ifaddrs = malloc (num * sizeof (ifaddrs[0]));
-      if (!ifaddrs)
+      new_ifaddrs = realloc (ifaddrs, num * sizeof (ifaddrs[0]));
+      if (!new_ifaddrs)
 	goto cleanup1;
 
+      ifaddrs = new_ifaddrs;
+
       /* Copy usable interfaces in ifaddrs structure.  */
       for (cur_ifr = ifr, i = 0;  i < num; ++cur_ifr, ++i)
 	{
@@ -656,7 +659,11 @@
 static void __attribute__ ((unused))
 free_mem (void)
 {
-  free (ifaddrs);
+  if (ifaddrs)
+    {
+      free (ifaddrs);
+      ifaddrs = NULL;
+    }
 }
 
 text_set_element (__libc_subfreeres, free_mem);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20011226/1897fac2/attachment.sig>


More information about the Libc-alpha mailing list