Home Original page

[patch] bfd_fopen() & co. should strdup (filename)

Jan Kratochvil jan.kratochvil@redhat.com
Fri Aug 10 10:49:00 GMT 2007 
Hi,

a recent GDB change:

2007-08-08  Michael Snyder  <msnyder@access-company.com>

        * solib-svr4.c (open_symbol_file_object): Memory leak.
        (svr4_current_sos): Ditto.
        (enable_break): Ditto.

(solib-svr4.c 1.71 -> 1.72)

it broke the GDB testsuite, visible at least by `gdb.base/call-sc.exp':
	(gdb) run
	Starting program: /home/jkratoch/redhat/sources/gdb/testsuite/gdb.base/call-sc-tc
	BFD: reopening <NON-ASCII GARBAGE>: No such file or directory
	(general FAIL)

I believe it is more a bug in BFD as the doc for neither of BFD_FOPEN,
BFD_OPENSTREAMR, BFD_OPENR_IOVEC, BFD_OPENW nor BFD_CREATE says anything about
the required validity of their FILENAME parameter.  Therefore it is generally
assumed it can be free(3)d after the function call returns.

Providing two variants of the patch as I am not sure which one gets accepted.

IMO the only function which must not get FILENAME as NULL is BFD_OPENW - this
simplifies a bit only the `inlined' variant of the patch.


Regards,
Jan
-------------- next part --------------
2007-08-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* opncls.c (filename_dup): New function.
	(bfd_fopen): Call FILENAME_DUP for the FILENAME parameter of NBFD.
	(bfd_openstreamr): Likewise.
	(bfd_openr_iovec): Likewise.
	(bfd_openw): Likewise.
	(bfd_create): Likewise.

--- bfd/opncls.c	9 Aug 2007 14:22:03 -0000	1.49
+++ bfd/opncls.c	10 Aug 2007 10:29:25 -0000
@@ -42,6 +42,21 @@
 
 static unsigned int _bfd_id_counter = 0;
 
+static bfd_boolean
+filename_dup (bfd *abfd, const char *filename)
+{
+  if (filename == NULL)
+    abfd->filename = NULL;
+  else
+    {
+      abfd->filename = bfd_alloc (abfd, strlen (filename) + 1);
+      if (abfd->filename == NULL)
+        return FALSE;
+      strcpy ((char *) abfd->filename, filename);
+    }
+  return TRUE;
+}
+
 /* fdopen is a loser -- we should use stdio exclusively.  Unfortunately
    if we do that we can't use fcntl.  */
 
@@ -209,7 +224,11 @@ bfd_fopen (const char *filename, const c
     }
 
   /* OK, put everything where it belongs.  */
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
 
   /* Figure out whether the user is opening the file for reading,
      writing, or both, by looking at the MODE argument.  */
@@ -358,7 +377,13 @@ bfd_openstreamr (const char *filename, c
     }
 
   nbfd->iostream = stream;
-  nbfd->filename = filename;
+
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = read_direction;
 
   if (! bfd_cache_init (nbfd))
@@ -542,7 +567,12 @@ bfd_openr_iovec (const char *filename, c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = read_direction;
 
   /* `open (...)' would get expanded by an the open(2) syscall macro.  */
@@ -604,7 +634,12 @@ bfd_openw (const char *filename, const c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = write_direction;
 
   if (bfd_open_file (nbfd) == NULL)
@@ -762,7 +797,13 @@ bfd_create (const char *filename, bfd *t
   nbfd = _bfd_new_bfd ();
   if (nbfd == NULL)
     return NULL;
-  nbfd->filename = filename;
+
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   if (templ)
     nbfd->xvec = templ->xvec;
   nbfd->direction = no_direction;
-------------- next part --------------
2007-08-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* opncls.c (bfd_fopen): STRDUP the FILENAME parameter for NBFD.
	(bfd_openstreamr): Likewise.
	(bfd_openr_iovec): Likewise.
	(bfd_openw): Likewise.
	(bfd_create): Likewise.

--- bfd/opncls.c	9 Aug 2007 14:22:03 -0000	1.49
+++ bfd/opncls.c	10 Aug 2007 10:45:20 -0000
@@ -209,7 +209,18 @@ bfd_fopen (const char *filename, const c
     }
 
   /* OK, put everything where it belongs.  */
-  nbfd->filename = filename;
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
 
   /* Figure out whether the user is opening the file for reading,
      writing, or both, by looking at the MODE argument.  */
@@ -358,7 +369,20 @@ bfd_openstreamr (const char *filename, c
     }
 
   nbfd->iostream = stream;
-  nbfd->filename = filename;
+
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   nbfd->direction = read_direction;
 
   if (! bfd_cache_init (nbfd))
@@ -542,7 +566,19 @@ bfd_openr_iovec (const char *filename, c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   nbfd->direction = read_direction;
 
   /* `open (...)' would get expanded by an the open(2) syscall macro.  */
@@ -604,7 +640,14 @@ bfd_openw (const char *filename, const c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+  if (nbfd->filename == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+  strcpy ((char *) nbfd->filename, filename);
+
   nbfd->direction = write_direction;
 
   if (bfd_open_file (nbfd) == NULL)
@@ -762,7 +805,20 @@ bfd_create (const char *filename, bfd *t
   nbfd = _bfd_new_bfd ();
   if (nbfd == NULL)
     return NULL;
-  nbfd->filename = filename;
+
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   if (templ)
     nbfd->xvec = templ->xvec;
   nbfd->direction = no_direction;
More information about the Binutils mailing list