From: Philippe Waroquiers Date: Wed, 27 Mar 2019 15:38:34 +0000 (-0600) Subject: Fix buffer overflow regression due to minsym malloc-ed instead of obstack-ed. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=20dc7e9b059edff1d0fab3bd197c460c11cc7a06;p=binutils-gdb.git Fix buffer overflow regression due to minsym malloc-ed instead of obstack-ed. Valgrind detects the following error in a bunch of tests, e.g. in gdb.base/foll-fork.exp. ==15155== VALGRIND_GDB_ERROR_BEGIN ==15155== Invalid read of size 8 ==15155== at 0x55BE04: minimal_symbol_upper_bound(bound_minimal_symbol) (minsyms.c:1504) ==15155== by 0x3B2E9C: find_pc_partial_function(unsigned long, char const**, unsigned long*, unsigned long*, block const**) (blockframe.c:340) ==15155== by 0x3B3135: find_function_entry_range_from_pc(unsigned long, char const**, unsigned long*, unsigned long*) (blockframe.c:385) ==15155== by 0x4F5597: fill_in_stop_func(gdbarch*, execution_control_state*) [clone .part.16] (infrun.c:4124) ==15155== by 0x4FBE01: fill_in_stop_func (infrun.c:7636) ==15155== by 0x4FBE01: process_event_stop_test(execution_control_state*) (infrun.c:6279) ... ==15155== Address 0x715bec8 is 0 bytes after a block of size 2,952 alloc'd ==15155== at 0x4C2E2B3: realloc (vg_replace_malloc.c:836) ==15155== by 0x405F2C: xrealloc (common-utils.c:62) ==15155== by 0x55BA4E: xresizevec (poison.h:170) ==15155== by 0x55BA4E: minimal_symbol_reader::install() (minsyms.c:1399) ==15155== by 0x4981C7: elf_read_minimal_symbols (elfread.c:1165) ... This seems to be a regression created by: commit 042d75e42c5572f333e0e06dabd3c5c4afab486c Author: Tom Tromey AuthorDate: Sat Mar 2 12:29:48 2019 -0700 Commit: Tom Tromey CommitDate: Fri Mar 15 16:02:10 2019 -0600 Allocate minimal symbols with malloc Before this commit, the array of 'struct minimal_symbol' contained a last element that was a "null symbol". The comment in minimal_symbol_reader::install was: /* We also terminate the minimal symbol table with a "null symbol", which is *not* included in the size of the table. This makes it easier to find the end of the table when we are handed a pointer to some symbol in the middle of it. Zero out the fields in the "null symbol" allocated at the end of the array. Note that the symbol count does *not* include this null symbol, which is why it is indexed by mcount and not mcount-1. */ memset (&msymbols[mcount], 0, sizeof (struct minimal_symbol)); However, minimal_symbol_upper_bound was still based on the assumption that the array of minsym is terminated by a minsym with a null symbol: it is looping with: for (i = 1; MSYMBOL_LINKAGE_NAME (msymbol + i) != NULL; i++) Replace this NULL comparison by a logic that calculates how many msymbol are following the msymbols from which we are starting from. (Re-)tested on debian/amd64, natively and under valgrind. gdb/ChangeLog 2019-03-24 Philippe Waroquiers Tom Tromey * minsyms.c (minimal_symbol_upper_bound): Fix buffer overflow. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4ef8a5c9780..0c658e2cfe8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2019-03-24 Philippe Waroquiers + Tom Tromey + + * minsyms.c (minimal_symbol_upper_bound): Fix buffer overflow. + 2019-03-26 Joel Brobecker * gdb-gdb.py.in (StructMainTypePrettyPrinter.bound_img): New method. diff --git a/gdb/minsyms.c b/gdb/minsyms.c index 777f545d84a..51b65f51421 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -1445,11 +1445,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); @@ -1464,21 +1463,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 *past_the_end + = (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 != past_the_end; ++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 != past_the_end + && (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. */