This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: two patches for bugs in BFD/peXXigen.c


On Mon, Sep 06, 2010 at 01:33:08PM +0200, Marcus Brinkmann wrote:
> On 09/06/2010 06:53 AM, Alan Modra wrote:
> > -	  for (j = 0; j < datasize; j += 8)
> > +	  for (j = 0; j < limit_size; j += 8)
> >  	    {
> >  	      unsigned long member = bfd_get_32 (abfd, data + idx + j);
> >  	      unsigned long member_high = bfd_get_32 (abfd, data + idx + j + 4);
> > 
> > Won't changing the loop endpoint possibly affect reading "member"?
> 
> That's the whole point of the change.  datasize is not a valid limit for the
> thunk table, at all.

OK, but limit_size is not correct either.  When hint_addr != 0
&& first_thunk != hint_addr && ft_section != section you set
limit_size to something based on ft_section->size, but data points to
some place within a different section contents.  I'm committing the
following.

2010-09-07  Alan Modra  <amodra@gmail.com>
	    Marcus Brinkmann  <marcus.brinkmann@ruhr-uni-bochum.de>

	* peXXigen.c: Whitespace.
	(pe_print_idata): Correct section limit calculations.  Tidy array
	indexing.
	(_bfd_XX_print_ce_compressed_pdata): Don't leak memory.

Index: bfd/peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.64
diff -u -p -w -r1.64 peXXigen.c
--- bfd/peXXigen.c	27 Jun 2010 04:07:53 -0000	1.64
+++ bfd/peXXigen.c	6 Sep 2010 23:44:13 -0000
@@ -1110,7 +1110,6 @@ pe_print_idata (bfd * abfd, void * vfile
 	   section->name, (unsigned long) addr);
 
   dataoff = addr - section->vma;
-  datasize -= dataoff;
 
 #ifdef POWERPC_LE_PE
   if (rel_section != 0 && rel_section->size != 0)
@@ -1183,7 +1182,7 @@ pe_print_idata (bfd * abfd, void * vfile
   adj = section->vma - extra->ImageBase;
 
   /* Print all image import descriptors.  */
-  for (i = 0; i < datasize; i += onaline)
+  for (i = dataoff; i + onaline <= datasize; i += onaline)
     {
       bfd_vma hint_addr;
       bfd_vma time_stamp;
@@ -1195,12 +1194,12 @@ pe_print_idata (bfd * abfd, void * vfile
       char *dll;
 
       /* Print (i + extra->DataDirectory[PE_IMPORT_TABLE].VirtualAddress).  */
-      fprintf (file, " %08lx\t", (unsigned long) (i + adj + dataoff));
-      hint_addr = bfd_get_32 (abfd, data + i + dataoff);
-      time_stamp = bfd_get_32 (abfd, data + i + 4 + dataoff);
-      forward_chain = bfd_get_32 (abfd, data + i + 8 + dataoff);
-      dll_name = bfd_get_32 (abfd, data + i + 12 + dataoff);
-      first_thunk = bfd_get_32 (abfd, data + i + 16 + dataoff);
+      fprintf (file, " %08lx\t", (unsigned long) (i + adj));
+      hint_addr = bfd_get_32 (abfd, data + i);
+      time_stamp = bfd_get_32 (abfd, data + i + 4);
+      forward_chain = bfd_get_32 (abfd, data + i + 8);
+      dll_name = bfd_get_32 (abfd, data + i + 12);
+      first_thunk = bfd_get_32 (abfd, data + i + 16);
 
       fprintf (file, "%08lx %08lx %08lx %08lx %08lx\n",
 	       (unsigned long) hint_addr,
@@ -1225,15 +1224,16 @@ pe_print_idata (bfd * abfd, void * vfile
 	  bfd_vma ft_addr;
 	  bfd_size_type ft_datasize;
 	  int ft_idx;
-	  int ft_allocated = 0;
+	  int ft_allocated;
 
 	  fprintf (file, _("\tvma:  Hint/Ord Member-Name Bound-To\n"));
 
 	  idx = hint_addr - adj;
 	  
 	  ft_addr = first_thunk + extra->ImageBase;
-	  ft_data = data;
 	  ft_idx = first_thunk - adj;
+	  ft_data = data + ft_idx;
+	  ft_datasize = datasize - ft_idx;
 	  ft_allocated = 0; 
 
 	  if (first_thunk != hint_addr)
@@ -1243,9 +1243,8 @@ pe_print_idata (bfd * abfd, void * vfile
 		   ft_section != NULL;
 		   ft_section = ft_section->next)
 		{
-		  ft_datasize = ft_section->size;
 		  if (ft_addr >= ft_section->vma
-		      && ft_addr < ft_section->vma + ft_datasize)
+		      && ft_addr < ft_section->vma + ft_section->size)
 		    break;
 		}
 
@@ -1258,34 +1257,28 @@ pe_print_idata (bfd * abfd, void * vfile
 
 	      /* Now check to see if this section is the same as our current
 		 section.  If it is not then we will have to load its data in.  */
-	      if (ft_section == section)
-		{
-		  ft_data = data;
-		  ft_idx = first_thunk - adj;
-		}
-	      else
+	      if (ft_section != section)
 		{
 		  ft_idx = first_thunk - (ft_section->vma - extra->ImageBase);
-		  ft_data = (bfd_byte *) bfd_malloc (datasize);
+		  ft_datasize = ft_section->size - ft_idx;
+		  ft_data = (bfd_byte *) bfd_malloc (ft_datasize);
 		  if (ft_data == NULL)
 		    continue;
 
-		  /* Read datasize bfd_bytes starting at offset ft_idx.  */
-		  if (! bfd_get_section_contents
-		      (abfd, ft_section, ft_data, (bfd_vma) ft_idx, datasize))
+		  /* Read ft_datasize bytes starting at offset ft_idx.  */
+		  if (!bfd_get_section_contents (abfd, ft_section, ft_data,
+						 (bfd_vma) ft_idx, ft_datasize))
 		    {
 		      free (ft_data);
 		      continue;
 		    }
-
-		  ft_idx = 0;
 		  ft_allocated = 1;
 		}
 	    }
 
 	  /* Print HintName vector entries.  */
 #ifdef COFF_WITH_pex64
-	  for (j = 0; j < datasize; j += 8)
+	  for (j = 0; idx + j + 8 <= datasize; j += 8)
 	    {
 	      unsigned long member = bfd_get_32 (abfd, data + idx + j);
 	      unsigned long member_high = bfd_get_32 (abfd, data + idx + j + 4);
@@ -1310,13 +1303,14 @@ pe_print_idata (bfd * abfd, void * vfile
 		 table holds actual addresses.  */
 	      if (time_stamp != 0
 		  && first_thunk != 0
-		  && first_thunk != hint_addr)
+		  && first_thunk != hint_addr
+		  && j + 4 <= ft_datasize)
 		fprintf (file, "\t%04lx",
-			 (unsigned long) bfd_get_32 (abfd, ft_data + ft_idx + j));
+			 (unsigned long) bfd_get_32 (abfd, ft_data + j));
 	      fprintf (file, "\n");
 	    }
 #else
-	  for (j = 0; j < datasize; j += 4)
+	  for (j = 0; idx + j + 4 <= datasize; j += 4)
 	    {
 	      unsigned long member = bfd_get_32 (abfd, data + idx + j);
 
@@ -1342,9 +1336,10 @@ pe_print_idata (bfd * abfd, void * vfile
 		 table holds actual addresses.  */
 	      if (time_stamp != 0
 		  && first_thunk != 0
-		  && first_thunk != hint_addr)
+		  && first_thunk != hint_addr
+		  && j + 4 <= ft_datasize)
 		fprintf (file, "\t%04lx",
-			 (unsigned long) bfd_get_32 (abfd, ft_data + ft_idx + j));
+			 (unsigned long) bfd_get_32 (abfd, ft_data + j));
 
 	      fprintf (file, "\n");
 	    }
@@ -1828,7 +1823,6 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
       bfd_vma other_data;
       bfd_vma prolog_length, function_length;
       int flag32bit, exception_flag;
-      bfd_byte *tdata = 0;
       asection *tsection;
 
       if (i + PDATA_ROW_SIZE > stop)
@@ -1860,12 +1854,13 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
       if (tsection && coff_section_data (abfd, tsection)
 	  && pei_section_data (abfd, tsection))
 	{
-	  if (bfd_malloc_and_get_section (abfd, tsection, & tdata))
-	    {
-	      int xx = (begin_addr - 8) - tsection->vma;
+	  bfd_vma eh_off = (begin_addr - 8) - tsection->vma;
+	  bfd_byte *tdata;
 
 	      tdata = (bfd_byte *) bfd_malloc (8);
-	      if (bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8))
+	  if (tdata)
+	    {
+	      if (bfd_get_section_contents (abfd, tsection, tdata, eh_off, 8))
 		{
 		  bfd_vma eh, eh_data;
 
@@ -1883,11 +1878,6 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
 		}
 	      free (tdata);
 	    }
-	  else
-	    {
-	      if (tdata)
-		free (tdata);
-	    }
 	}
 
       fprintf (file, "\n");

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]