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]

PR 9798, objcopy can access freed memory on reporting errors


As reported in bugzilla, accessing a bfd after bfd_close is bad since
the bfd is freed.  Made worse by the fact that one of the
RETURN_NONFATAL invocations used the wrong bfd.

	PR 9798
	* bucomm.c (bfd_nonfatal_message): Use bfd_get_archive_filename.
	(bfd_get_archive_filename): Constify param.
	* bucomm.h (bfd_get_archive_filename): Update prototype.
	* objcopy.c (RETURN_NONFATAL): Delete.
	(copy_unknown_object): Don't call bfd_get_archive_filename for
	bfd_nonfatal_message filename, instead just pass bfd.
	(copy_object): Likewise.
	(copy_archive, copy_file): Likewise.  Expand RETURN_NONFATAL.  On
	bfd_close errors, do not pass the bfd to bfd_nonfatal_message.
	(setup_bfd_headers): Fix error message.

Index: binutils/bucomm.c
===================================================================
RCS file: /cvs/src/src/binutils/bucomm.c,v
retrieving revision 1.33
diff -u -p -r1.33 bucomm.c
--- binutils/bucomm.c	9 Jan 2008 10:40:32 -0000	1.33
+++ binutils/bucomm.c	29 Jan 2009 12:49:00 -0000
@@ -1,6 +1,6 @@
 /* bucomm.c -- Bin Utils COMmon code.
    Copyright 1991, 1992, 1993, 1994, 1995, 1997, 1998, 2000, 2001, 2002,
-   2003, 2006, 2007
+   2003, 2006, 2007, 2008, 2009
    Free Software Foundation, Inc.
 
    This file is part of GNU Binutils.
@@ -88,7 +88,7 @@ bfd_nonfatal_message (const char *filena
   if (bfd)
     {
       if (!filename)
-	filename = bfd_get_filename (bfd);
+	filename = bfd_get_archive_filename (bfd);
       if (section)
 	section_name = bfd_get_section_name (bfd, section);
     }
@@ -577,7 +577,7 @@ get_file_size (const char * file_name)
 /* Return the filename in a static buffer.  */
 
 const char *
-bfd_get_archive_filename (bfd *abfd)
+bfd_get_archive_filename (const bfd *abfd)
 {
   static size_t curr = 0;
   static char *buf;
Index: binutils/bucomm.h
===================================================================
RCS file: /cvs/src/src/binutils/bucomm.h,v
retrieving revision 1.28
diff -u -p -r1.28 bucomm.h
--- binutils/bucomm.h	30 Aug 2007 10:19:03 -0000	1.28
+++ binutils/bucomm.h	29 Jan 2009 12:49:00 -0000
@@ -1,6 +1,6 @@
 /* bucomm.h -- binutils common include file.
    Copyright 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000,
-   2001, 2002, 2003, 2005, 2006, 2007 Free Software Foundation, Inc.
+   2001, 2002, 2003, 2005, 2006, 2007, 2009 Free Software Foundation, Inc.
 
    This file is part of GNU Binutils.
 
@@ -23,7 +23,7 @@
 #define _BUCOMM_H
 
 /* Return the filename in a static buffer.  */
-const char *bfd_get_archive_filename (bfd *);
+const char *bfd_get_archive_filename (const bfd *);
 
 void bfd_nonfatal (const char *);
 
Index: binutils/objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.124
diff -u -p -r1.124 objcopy.c
--- binutils/objcopy.c	28 Sep 2008 13:29:18 -0000	1.124
+++ binutils/objcopy.c	29 Jan 2009 12:48:06 -0000
@@ -1,6 +1,6 @@
 /* objcopy.c -- copy object file from input to output, optionally massaging it.
    Copyright 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000,
-   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008
+   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
    Free Software Foundation, Inc.
 
    This file is part of GNU Binutils.
@@ -59,11 +59,6 @@ section_rename;
 /* List of sections to be renamed.  */
 static section_rename *section_rename_list;
 
-#define RETURN_NONFATAL(bfd) \
-  do { \
-    status = 1; bfd_nonfatal_message (NULL, bfd, NULL, NULL); return; \
-  } while (0)
-
 static asymbol **isympp = NULL;	/* Input symbols.  */
 static asymbol **osympp = NULL;	/* Output symbols that survive stripping.  */
 
@@ -1291,7 +1286,7 @@ copy_unknown_object (bfd *ibfd, bfd *obf
 
   if (bfd_stat_arch_elt (ibfd, &buf) != 0)
     {
-      bfd_nonfatal_message (bfd_get_archive_filename (ibfd), NULL, NULL, NULL);
+      bfd_nonfatal_message (NULL, ibfd, NULL, NULL);
       return FALSE;
     }
 
@@ -1324,8 +1319,7 @@ copy_unknown_object (bfd *ibfd, bfd *obf
       if (bfd_bread (cbuf, (bfd_size_type) tocopy, ibfd)
 	  != (bfd_size_type) tocopy)
 	{
-	  bfd_nonfatal_message (bfd_get_archive_filename (ibfd),
-				NULL, NULL, NULL);
+	  bfd_nonfatal_message (NULL, ibfd, NULL, NULL);
 	  free (cbuf);
 	  return FALSE;
 	}
@@ -1404,8 +1398,7 @@ copy_object (bfd *ibfd, bfd *obfd)
       if (!bfd_set_start_address (obfd, start)
 	  || !bfd_set_file_flags (obfd, flags))
 	{
-	  bfd_nonfatal_message (bfd_get_archive_filename (ibfd),
-				NULL, NULL, NULL);
+	  bfd_nonfatal_message (NULL, ibfd, NULL, NULL);
 	  return FALSE;
 	}
     }
@@ -1429,7 +1422,7 @@ copy_object (bfd *ibfd, bfd *obfd)
 
   if (!bfd_set_format (obfd, bfd_get_format (ibfd)))
     {
-      bfd_nonfatal_message (bfd_get_archive_filename (ibfd), NULL, NULL, NULL);
+      bfd_nonfatal_message (NULL, ibfd, NULL, NULL);
       return FALSE;
     }
 
@@ -1445,7 +1438,7 @@ copy_object (bfd *ibfd, bfd *obfd)
   symsize = bfd_get_symtab_upper_bound (ibfd);
   if (symsize < 0)
     {
-      bfd_nonfatal_message (bfd_get_archive_filename (ibfd), NULL, NULL, NULL);
+      bfd_nonfatal_message (NULL, ibfd, NULL, NULL);
       return FALSE;
     }
 
@@ -1856,7 +1849,8 @@ copy_archive (bfd *ibfd, bfd *obfd, cons
     } *list, *l;
   bfd **ptr = &obfd->archive_head;
   bfd *this_element;
-  char * dir;
+  char *dir;
+  const char *filename;
 
   /* Make a temp directory to hold the contents.  */
   dir = make_tempdir (bfd_get_filename (obfd));
@@ -1872,7 +1866,11 @@ copy_archive (bfd *ibfd, bfd *obfd, cons
   this_element = bfd_openr_next_archived_file (ibfd, NULL);
 
   if (!bfd_set_format (obfd, bfd_get_format (ibfd)))
-    RETURN_NONFATAL (obfd);
+    {
+      status = 1;
+      bfd_nonfatal_message (NULL, obfd, NULL, NULL);
+      return;
+    }
 
   while (!status && this_element != NULL)
     {
@@ -1942,7 +1940,7 @@ copy_archive (bfd *ibfd, bfd *obfd, cons
 	    {
 	      if (!bfd_close (output_bfd))
 		{
-		  bfd_nonfatal_message (NULL, output_bfd, NULL, NULL);
+		  bfd_nonfatal_message (output_name, NULL, NULL, NULL);
 		  /* Error in new object file. Don't change archive.  */
 		  status = 1;
 		}
@@ -1952,8 +1950,7 @@ copy_archive (bfd *ibfd, bfd *obfd, cons
 	}
       else
 	{
-	  bfd_nonfatal_message (bfd_get_archive_filename (this_element),
-				NULL, NULL,
+	  bfd_nonfatal_message (NULL, this_element, NULL,
 				_("Unable to recognise the format of file"));
 
 	  output_bfd = bfd_openw (output_name, output_target);
@@ -1961,7 +1958,7 @@ copy_unknown_element:
 	  delete = !copy_unknown_object (this_element, output_bfd);
 	  if (!bfd_close_all_done (output_bfd))
 	    {
-	      bfd_nonfatal_message (NULL, output_bfd, NULL, NULL);
+	      bfd_nonfatal_message (output_name, NULL, NULL, NULL);
 	      /* Error in new object file. Don't change archive.  */
 	      status = 1;
 	    }
@@ -1994,11 +1991,21 @@ copy_unknown_element:
     }
   *ptr = NULL;
 
+  filename = bfd_get_filename (obfd);
   if (!bfd_close (obfd))
-    RETURN_NONFATAL (obfd);
+    {
+      status = 1;
+      bfd_nonfatal_message (filename, NULL, NULL, NULL);
+      return;
+    }
 
+  filename = bfd_get_filename (ibfd);
   if (!bfd_close (ibfd))
-    RETURN_NONFATAL (obfd);
+    {
+      status = 1;
+      bfd_nonfatal_message (filename, NULL, NULL, NULL);
+      return;
+    }
 
   /* Delete all the files that we opened.  */
   for (l = list; l != NULL; l = l->next)
@@ -2087,10 +2094,18 @@ copy_file (const char *input_filename, c
 	status = 1;
 
       if (!bfd_close (obfd))
-	RETURN_NONFATAL (obfd);
+	{
+	  status = 1;
+	  bfd_nonfatal_message (output_filename, NULL, NULL, NULL);
+	  return;
+	}
 
       if (!bfd_close (ibfd))
-	RETURN_NONFATAL (ibfd);
+	{
+	  status = 1;
+	  bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
+	  return;
+	}
     }
   else
     {
@@ -2195,7 +2210,7 @@ setup_bfd_headers (bfd *ibfd, bfd *obfd)
     {
       status = 1;
       bfd_nonfatal_message (NULL, ibfd, NULL,
-			    _("error in private h	eader data"));
+			    _("error in private header data"));
       return;
     }
 

-- 
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]