This is the mail archive of the binutils@sources.redhat.com 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]

Re: [patch] bfd/som.c: fix local buffer overrun (was: gas (binutils) 2.10: SIGSEGV on hppa1.1-hp-hpux10.20)


> Thank you.  That's precisely what I needed to know (you had a symbol with a
> very long name).

Using the C++ STL, most symbols have a very long name. ;-)

> I just checked the SOM reference and the limit on symbol names is 2^32
> characters; so the name is valid according to the SOM reference.

BTW, that figure also means we do need size_t rather than int, although it
will hardly ever matter.

> We do need the fix applied to the code which handles space/subspace
> strings.  We do not need the fix applied to the code which handles
> relocations/fixups (no single fixup can be 8kbytes).

Thank you.  That's precisely what I needed to know.  Below is my revised
patch.

>   > Doesn't that depend on whether the error is something like "file system
>   > full" or whether it is a bug in the code itself?
> No, it doesn't depend on anything like that.

Can you give me a pointer to an archive of the discussion on that design
discussion?

>   > But maybe you can rule out anyway that this might ever happen. I don't
>   > fully understand what is going on here. Can space names (what are those?)
>   > become arbitrarily long because they directly correspond to names in the
>   > input file?
> Yes.  See above.

I don't know how to create a space name nor what it is at all.  That means, my
change to som_write_space_strings is untested so far.

>   > >   > -  memset (tmp_space, 0, SOM_TMP_BUFSIZE);
> Actually, for the symbol and space/subspace strings it is not needed since any
> gaps in them are explicitly zero filled.  Sorry about the confusion.  It's
> been about 7 years since I had to look at that code.
>
> You should just delete the zero-ing of the buffers for space/subspace and
> symbol strings.

Ok, deleted.  That was easy.
 
> The UofU no longer is involved in PA compiler tools work (and hasn't been
> since roughly 1995).  Feel free to submit a separate patch to just remove
> that address from the header of som.c.

A separate patch?  Well, I included the change in the other patch below.  I
trust you won't reject it for that reason.  

(A separate patch would probably need to change the copyright statement as
well.  Then you could not apply both patches without manually resolving the
conflict.)

> Jeff

Marco

diff -U5 /usr/gnu/src/binutils-2.10/bfd/som.c.orig /usr/gnu/src/binutils-2.10/bfd/som.c
--- /usr/gnu/src/binutils-2.10/bfd/som.c.orig	Sat Feb 26 01:45:22 2000
+++ /usr/gnu/src/binutils-2.10/bfd/som.c	Tue Sep 12 19:18:33 2000
@@ -1,11 +1,11 @@
 /* bfd back-end for HP PA-RISC SOM objects.
-   Copyright (C) 1990, 91, 92, 93, 94, 95, 96, 97, 1998
+   Copyright (C) 1990, 91, 92, 93, 94, 95, 96, 97, 1998, 2000
    Free Software Foundation, Inc.
 
    Contributed by the Center for Software Science at the
-   University of Utah (pa-gdb-bugs@cs.utah.edu).
+   University of Utah.
 
    This file is part of BFD, the Binary File Descriptor library.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -3045,46 +3045,62 @@
      unsigned long current_offset;
      unsigned int *string_sizep;
 {
   /* Chunk of memory that we can use as buffer space, then throw
      away.  */
-  unsigned char tmp_space[SOM_TMP_BUFSIZE];
-  unsigned char *p;
+  size_t tmp_space_size = SOM_TMP_BUFSIZE;
+  unsigned char *tmp_space = alloca (tmp_space_size);
+  unsigned char *p = tmp_space;
   unsigned int strings_size = 0;
   asection *section;
 
-  memset (tmp_space, 0, SOM_TMP_BUFSIZE);
-  p = tmp_space;
-
   /* Seek to the start of the space strings in preparation for writing
      them out.  */
   if (bfd_seek (abfd, current_offset, SEEK_SET) < 0)
     return false;
 
   /* Walk through all the spaces and subspaces (order is not important)
      building up and writing string table entries for their names.  */
   for (section = abfd->sections; section != NULL; section = section->next)
     {
-      int length;
+      size_t length;
 
       /* Only work with space/subspaces; avoid any other sections
 	 which might have been made (.text for example).  */
       if (!som_is_space (section) && !som_is_subspace (section))
 	continue;
 
       /* Get the length of the space/subspace name.  */
       length = strlen (section->name);
 
       /* If there is not enough room for the next entry, then dump the
-	 current buffer contents now.  Each entry will take 4 bytes to
-	 hold the string length + the string itself + null terminator.  */
-      if (p - tmp_space + 5 + length > SOM_TMP_BUFSIZE)
+         current buffer contents now and maybe allocate a larger
+         buffer.  Each entry will take 4 bytes to hold the string
+         length + the string itself + null terminator.  */
+      if (p - tmp_space + 5 + length > tmp_space_size)
 	{
+	  /* Flush buffer before refilling or reallocating.  */
 	  if (bfd_write ((PTR) &tmp_space[0], p - tmp_space, 1, abfd)
 	      != p - tmp_space) 
 	    return false;
-	  /* Reset to beginning of the buffer space.  */
+
+	  /* Reallocate if now empty buffer still too small.  */
+	  if (5 + length > tmp_space_size)
+	    {
+	      /* Ensure a minimum growth factor to avoid O(n**2) space
+                 consumption for n strings.  The optimal minimum
+                 factor seems to be 2, as no other value can guarantee
+                 wasting less then 50% space.  (Note that we cannot
+                 deallocate space allocated by `alloca' without
+                 returning from this function.)  The same technique is
+                 used a few more times below when a buffer is
+                 reallocated.  */
+	      tmp_space_size = MAX (2 * tmp_space_size, 5 + length);
+	      tmp_space = alloca (tmp_space_size);
+	    }
+
+	  /* Reset to beginning of the (possibly new) buffer space.  */
 	  p = tmp_space;
 	}
 
       /* First element in a string table entry is the length of the
 	 string.  Alignment issues are already handled.  */
@@ -3134,12 +3150,14 @@
 {
   unsigned int i;
   
   /* Chunk of memory that we can use as buffer space, then throw
      away.  */
-  unsigned char tmp_space[SOM_TMP_BUFSIZE];
-  unsigned char *p;
+  size_t tmp_space_size = SOM_TMP_BUFSIZE;
+  unsigned char *tmp_space = alloca (tmp_space_size);
+  unsigned char *p = tmp_space;
+
   unsigned int strings_size = 0;
   unsigned char *comp[4];
 
   /* This gets a bit gruesome because of the compilation unit.  The
      strings within the compilation unit are part of the symbol
@@ -3153,32 +3171,41 @@
       comp[1] = compilation_unit->language_name.n_name;
       comp[2] = compilation_unit->product_id.n_name;
       comp[3] = compilation_unit->version_id.n_name;
     }
 
-  memset (tmp_space, 0, SOM_TMP_BUFSIZE);
-  p = tmp_space;
-
   /* Seek to the start of the space strings in preparation for writing
      them out.  */
   if (bfd_seek (abfd, current_offset, SEEK_SET) < 0)
     return false;
 
   if (compilation_unit)
     {
       for (i = 0; i < 4; i++)
 	{
-	  int length = strlen (comp[i]);
+	  size_t length = strlen (comp[i]);
 
 	  /* If there is not enough room for the next entry, then dump
-	     the current buffer contents now.  */
-	  if (p - tmp_space + 5 + length > SOM_TMP_BUFSIZE)
+	     the current buffer contents now and maybe allocate a
+	     larger buffer.  */
+	  if (p - tmp_space + 5 + length > tmp_space_size)
 	    {
+	      /* Flush buffer before refilling or reallocating. */
 	      if (bfd_write ((PTR) &tmp_space[0], p - tmp_space, 1, abfd)
 		  != p - tmp_space)
 		return false;
-	      /* Reset to beginning of the buffer space.  */
+
+	      /* Reallocate if now empty buffer still too small.  */
+	      if (5 + length > tmp_space_size)
+		{
+		  /* See alloca above for discussion of new size.  */
+		  tmp_space_size = MAX (2 * tmp_space_size, 5 + length);
+		  tmp_space = alloca (tmp_space_size);
+		}
+
+	      /* Reset to beginning of the (possibly new) buffer
+                 space.  */
 	      p = tmp_space;
 	    }
 
 	  /* First element in a string table entry is the length of
 	     the string.  This must always be 4 byte aligned.  This is
@@ -3223,20 +3250,30 @@
 	}
     }
 
   for (i = 0; i < num_syms; i++)
     {
-      int length = strlen (syms[i]->name);
+      size_t length = strlen (syms[i]->name);
 
       /* If there is not enough room for the next entry, then dump the
-	 current buffer contents now.  */
-     if (p - tmp_space + 5 + length > SOM_TMP_BUFSIZE)
+	 current buffer contents now and maybe allocate a larger buffer.  */
+     if (p - tmp_space + 5 + length > tmp_space_size)
 	{
+	  /* Flush buffer before refilling or reallocating. */
 	  if (bfd_write ((PTR) &tmp_space[0], p - tmp_space, 1, abfd)
 	      != p - tmp_space)
 	    return false;
-	  /* Reset to beginning of the buffer space.  */
+
+	  /* Reallocate if now empty buffer still too small.  */
+	  if (5 + length > tmp_space_size)
+	    {
+	      /* See alloca above for discussion of new size.  */
+	      tmp_space_size = MAX (2 * tmp_space_size, 5 + length);
+	      tmp_space = alloca (tmp_space_size);
+	    }
+
+	  /* Reset to beginning of the (possibly new) buffer space.  */
 	  p = tmp_space;
 	}
 
       /* First element in a string table entry is the length of the
 	 string.  This must always be 4 byte aligned.  This is also
-----------------------------------------------------------------
This email is confidential and intended solely for the use of the
individual to whom it is addressed.
Any views or opinions presented are solely those of the author
and do not necessarily represent those of Thyron Limited.
If you are not the intended recipient then please be advised
that you have received this email in error and that any use,
dissemination, forwarding, printing or copying of this email 
is strictly prohibited.
If you have received this email in error, please notify the 
Thyron IT Administrator on +44 (0)1923 236 050 or 
send an email to mail-admin@thyron.com.
Thank You

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