[PATCH] Fix memory leak in bfd_get_file_window

[Bernd Jendrissek <bernd dot jendrissek at gmail dot com>]

It turns out I was able to use realloc without too much difficulty to
grow an array, but while running my patched objdump under valgrind, I
found another leak.  Here is the fix.

commit f9d90575392fcda950d4a614388df531ecc52560
Author: Bernd Jendrissek <bernd.jendrissek@gmail.com>
Date:   Fri Sep 2 07:38:58 2011 +0200

    Fix memory leak if HAVE_MMAP.  Don't mutate on error.

    bfd_get_file_window leaked memory when HAVE_MMAP is true, since the
    window's refcount didn't get bumped to 1, so bfd_free_window would have
    to get called 2^31 times before freeing the memory.

    Mutating user-owned data when an error occurs is never a good idea when
    it is avoidable.  In this case it is avoidable, and helps to plug the
    memory leak.

    2011-09-02  Bernd Jendrissek  <bernd.jendrissek@gmail.com>

        * bfdwin.c (bfd_get_file_window): Fix memory leak.

diff --git a/bfd/bfdwin.c b/bfd/bfdwin.c
index 63ad5ed..43265cd 100644
--- a/bfd/bfdwin.c
+++ b/bfd/bfdwin.c
@@ -131,7 +131,6 @@ bfd_get_file_window (bfd *abfd,
   if (i == 0)
     {
       i = bfd_zmalloc (sizeof (bfd_window_internal));
-      windowp->i = i;
       if (i == 0)
 	return FALSE;
       i->data = 0;
@@ -156,7 +155,7 @@ bfd_get_file_window (bfd *abfd,
       if (abfd->iostream == NULL
 	  && (abfd->iovec == NULL
 	      || abfd->iovec->bseek (abfd, offset, SEEK_SET) != 0))
-	return FALSE;
+	goto free_and_fail;
       fd = fileno ((FILE *) abfd->iostream);

       /* Compute offsets and size for mmap and for the user's data.  */
@@ -189,7 +188,7 @@ bfd_get_file_window (bfd *abfd,
 	  windowp->data = 0;
 	  if (debug_windows)
 	    fprintf (stderr, "\t\tmmap failed!\n");
-	  return FALSE;
+	  goto free_and_fail;
 	}
       if (debug_windows)
 	fprintf (stderr, "\n\tmapped %ld at %p, offset is %ld\n",
@@ -198,6 +197,8 @@ bfd_get_file_window (bfd *abfd,
       windowp->data = (bfd_byte *) i->data + offset2;
       windowp->size = size;
       i->mapped = 1;
+      i->refcount = 1;
+      windowp->i = i;
       return TRUE;
     }
   else if (debug_windows)
@@ -228,15 +229,18 @@ bfd_get_file_window (bfd *abfd,
   if (i->data == NULL)
     {
       if (size_to_alloc == 0)
-	return TRUE;
-      return FALSE;
+	{
+	  windowp->i = i;
+	  return TRUE;
+	}
+      goto free_and_fail;
     }
   i->refcount = 1;
   if (bfd_seek (abfd, offset, SEEK_SET) != 0)
-    return FALSE;
+    goto free_and_fail;
   i->size = bfd_bread (i->data, size, abfd);
   if (i->size != size)
-    return FALSE;
+    goto free_and_fail;
   i->mapped = 0;
 #ifdef HAVE_MPROTECT
   if (!writable)
@@ -249,7 +253,13 @@ bfd_get_file_window (bfd *abfd,
 #endif
   windowp->data = i->data;
   windowp->size = i->size;
+  windowp->i = i;
   return TRUE;
+
+free_and_fail:
+  /* We have a bfd_window_internal, but an error occurred.  Free it. */
+  free (i);
+  return FALSE;
 }

 #endif /* USE_MMAP */