[RFAv2] Fix buffer overflow regression due to minsym malloc-ed instead of obstack-ed.

Tom Tromey tom@tromey.com
Mon Mar 25 15:31:00 GMT 2019


>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Before this commit, the array of 'struct minimal_symbol'
Philippe> contained a last element that was a "null symbol".  The comment in
Philippe> minimal_symbol_reader::install was:

Sorry about this.

Philippe> Note that a bunch of comments in minimal_symbol_reader::install
Philippe> are still referring to allocations being done in obstack.  These
Philippe> comments seem obsolete.  I have not fixed them, as I have not
Philippe> understood what they are explaining (e.g. related to language
Philippe> auto, demangling, etc : I have not seen where all this is done).

The comment about language_auto is mildly incorrect, and I think
probably has been for quite some time.

There are some other incorrect comments in there.  I'll send a patch.

Philippe> +  int n_after_msymbol = minsym.objfile->per_bfd->minimal_symbol_count
Philippe> +    - (msymbol - minsym.objfile->per_bfd->msymbols.get ())
Philippe> +    - 1;

What do you think of the appended instead?
The idea is to make the last element more explicit.

Tom

diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index b95e9ef6e8b..03743e3062b 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1480,11 +1480,10 @@ find_solib_trampoline_target (struct frame_info *frame, CORE_ADDR pc)
 CORE_ADDR
 minimal_symbol_upper_bound (struct bound_minimal_symbol minsym)
 {
-  int i;
   short section;
   struct obj_section *obj_section;
   CORE_ADDR result;
-  struct minimal_symbol *msymbol;
+  struct minimal_symbol *iter, *msymbol;
 
   gdb_assert (minsym.minsym != NULL);
 
@@ -1499,21 +1498,24 @@ minimal_symbol_upper_bound (struct bound_minimal_symbol minsym)
      other sections, to find the next symbol in this section with a
      different address.  */
 
+  struct minimal_symbol *last
+    = (minsym.objfile->per_bfd->msymbols.get ()
+       + minsym.objfile->per_bfd->minimal_symbol_count);
   msymbol = minsym.minsym;
   section = MSYMBOL_SECTION (msymbol);
-  for (i = 1; MSYMBOL_LINKAGE_NAME (msymbol + i) != NULL; i++)
+  for (iter = msymbol + 1; iter < last; ++iter)
     {
-      if ((MSYMBOL_VALUE_RAW_ADDRESS (msymbol + i)
+      if ((MSYMBOL_VALUE_RAW_ADDRESS (iter)
 	   != MSYMBOL_VALUE_RAW_ADDRESS (msymbol))
-	  && MSYMBOL_SECTION (msymbol + i) == section)
+	  && MSYMBOL_SECTION (iter) == section)
 	break;
     }
 
   obj_section = MSYMBOL_OBJ_SECTION (minsym.objfile, minsym.minsym);
-  if (MSYMBOL_LINKAGE_NAME (msymbol + i) != NULL
-      && (MSYMBOL_VALUE_ADDRESS (minsym.objfile, msymbol + i)
+  if (iter < last
+      && (MSYMBOL_VALUE_ADDRESS (minsym.objfile, iter)
 	  < obj_section_endaddr (obj_section)))
-    result = MSYMBOL_VALUE_ADDRESS (minsym.objfile, msymbol + i);
+    result = MSYMBOL_VALUE_ADDRESS (minsym.objfile, iter);
   else
     /* We got the start address from the last msymbol in the objfile.
        So the end address is the end of the section.  */



More information about the Gdb-patches mailing list