This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
PR 9798, objcopy can access freed memory on reporting errors
- From: Alan Modra <amodra at bigpond dot net dot au>
- To: binutils at sourceware dot org
- Date: Thu, 29 Jan 2009 23:40:35 +1030
- Subject: 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