Re: memory corruption while reading lineno entries from COFF files?

[Alan Modra <amodra at bigpond dot net dot au>]

On Mon, Aug 04, 2008 at 06:15:55PM +0200, Jaka MoÄnik wrote:
> my problem is that this seems way too big a bug to have gone unnoticed,

Well, the problem was only introduced 20070801, and COFF is thankfully
less popular today than it once was.

> so: can someone confirm my observation and its correctnes or tell me
> that it is me doing/understanding something brutally wrong?

You are correct.

> anyhow, if my diagnosis is correct, the fix seems fairly simple - as a
> quick grep over the source shows that the lineno cache of a section
> never gets freed at all, it can be simply bfd_malloc()ed and forgotten
> about. patch attached.

This works but is not ideal.  The nice thing about bfd_alloc memory is
that it is automatically freed when closing a bfd.  If we malloc, the
memory is not freed until program exit.  A program that opens and
closes numerous bfds should use less memory if we use bfd_alloc here.

So..

	* coffcode.h (coff_slurp_line_table): bfd_alloc lineno_cache first
	so that we don't inadvertently free it.  Use bfd_alloc for sort
	arrays, and memcpy sorted line table.

Index: bfd/coffcode.h
===================================================================
RCS file: /cvs/src/src/bfd/coffcode.h,v
retrieving revision 1.143
diff -u -p -r1.143 coffcode.h
--- bfd/coffcode.h	18 Jul 2008 11:30:22 -0000	1.143
+++ bfd/coffcode.h	5 Aug 2008 02:36:04 -0000
@@ -4296,21 +4296,23 @@ coff_slurp_line_table (bfd *abfd, asecti
 
   BFD_ASSERT (asect->lineno == NULL);
 
+  amt = ((bfd_size_type) asect->lineno_count + 1) * sizeof (alent);
+  lineno_cache = bfd_alloc (abfd, amt);
+  if (lineno_cache == NULL)
+    return FALSE;
+
   amt = (bfd_size_type) bfd_coff_linesz (abfd) * asect->lineno_count;
   native_lineno = (LINENO *) buy_and_read (abfd, asect->line_filepos, amt);
   if (native_lineno == NULL)
     {
       (*_bfd_error_handler)
         (_("%B: warning: line number table read failed"), abfd);
+      bfd_release (abfd, lineno_cache);
       return FALSE;
     }
 
-  amt = ((bfd_size_type) asect->lineno_count + 1) * sizeof (alent);
-  lineno_cache = bfd_alloc (abfd, amt);
-  if (lineno_cache == NULL)
-    return FALSE;
-
   cache_ptr = lineno_cache;
+  asect->lineno = lineno_cache;
   src = native_lineno;
   nbr_func = 0;
 
@@ -4364,6 +4366,7 @@ coff_slurp_line_table (bfd *abfd, asecti
       src++;
     }
   cache_ptr->line_number = 0;
+  bfd_release (abfd, native_lineno);
 
   /* On some systems (eg AIX5.3) the lineno table may not be sorted.  */
   if (!ordered)
@@ -4373,7 +4376,7 @@ coff_slurp_line_table (bfd *abfd, asecti
       alent *n_lineno_cache;
 
       /* Create a table of functions.  */
-      func_table = bfd_malloc (nbr_func * sizeof (alent *));
+      func_table = bfd_alloc (abfd, nbr_func * sizeof (alent *));
       if (func_table != NULL)
 	{
 	  alent **p = func_table;
@@ -4387,6 +4390,7 @@ coff_slurp_line_table (bfd *abfd, asecti
 	  qsort (func_table, nbr_func, sizeof (alent *), coff_sort_func_alent);
 
 	  /* Create the new sorted table.  */
+	  amt = ((bfd_size_type) asect->lineno_count + 1) * sizeof (alent);
 	  n_lineno_cache = bfd_alloc (abfd, amt);
 	  if (n_lineno_cache != NULL)
 	    {
@@ -4409,15 +4413,12 @@ coff_slurp_line_table (bfd *abfd, asecti
 		    *n_cache_ptr++ = *old_ptr++;
 		}
 	      n_cache_ptr->line_number = 0;
-	      bfd_release (abfd, lineno_cache);
-	      lineno_cache = n_lineno_cache;
+	      memcpy (lineno_cache, n_lineno_cache, amt);
 	    }
-	  free (func_table);
+	  bfd_release (abfd, func_table);
 	}
     }
 
-  asect->lineno = lineno_cache;
-  bfd_release (abfd, native_lineno);
   return TRUE;
 }
 

-- 
Alan Modra
Australia Development Lab, IBM