This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Synthetic symbol leak in in elf_x86_64_get_synthetic_symtab and elf_read_minimal_symbols
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: Alex Lindsay <alexlindsay239 at gmail dot com>, GDB <gdb-patches at sourceware dot org>
- Date: Fri, 11 Aug 2017 08:30:21 -0700
- Subject: Re: Synthetic symbol leak in in elf_x86_64_get_synthetic_symtab and elf_read_minimal_symbols
- Authentication-results: sourceware.org; auth=none
- References: <ce856f01-6e4b-5e74-fe8a-8e4aa0cb89e6@gmail.com> <20170811092709.GH8039@1170ee0b50d5>
On Fri, Aug 11, 2017 at 2:27 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> On 17-08-07 10:19:15, Alex Lindsay wrote:
>>
>> We perform a couple of dynamic allocations in
>> elf_x86_64_get_synthetic_symtab (elf64-x86-64.c):
>>
>> s = *ret = (asymbol *) bfd_zmalloc (size);
>>
>> names = (char *) bfd_malloc (size);
>>
>> that appear to never get freed. My patch for this:
>
> Good catch! It is more complicated that other bfd targets allocate
> memory for asymbol in a different way as if asymbol.name is defined
> as an zero-length array, so memory allocated for both asymbol and .name
> in one bfd_malloc call, like,
>
> sym = *r->sym_ptr_ptr;
> if (!sym_exists_at (syms, opdsymend, symcount,
> sym->section->id, sym->value + r->addend))
> {
> ++count;
> size += sizeof (asymbol);
> size += strlen (syms[i]->name) + 2;
> }
> }
>
> if (size == 0)
> goto done;
> s = *ret = bfd_malloc (size);
>
> or
>
> size = count * sizeof (asymbol);
> p = relplt->relocation;
> for (i = 0; i < count; i++, p += elf32_arm_size_info.int_rels_per_ext_rel)
> {
> size += strlen ((*p->sym_ptr_ptr)->name) + sizeof ("@plt");
> if (p->addend != 0)
> size += sizeof ("+0x") - 1 + 8;
> }
>
> s = *ret = (asymbol *) bfd_malloc (size);
>
>>
>> diff --git a/gdb/elfread.c b/gdb/elfread.c
>> index ece704ca7c..5ed8a6f957 100644
>> --- a/gdb/elfread.c
>> +++ b/gdb/elfread.c
>> @@ -1144,6 +1144,9 @@ elf_read_minimal_symbols (struct objfile *objfile, int
>> symfile_flags,
>>
>> if (symtab_create_debug)
>> fprintf_unfiltered (gdb_stdlog, "Done reading minimal symbols.\n");
>> + if (synthcount > 0)
>> + xfree ((char *) synthsyms->name);
>
> We can't do this for some bfd targets.
>
>> + xfree (synthsyms);
>
> We can only safely do this, but .name is leaked for x86_64. Other
> tools using bfd, like objdump, nm, and gprof may have this issue too.
> I'll ask binutils people on asymbol allocation and de-allocation.
>
This is:
https://sourceware.org/bugzilla/show_bug.cgi?id=21943
i386 and x86-64 get_synthetic_symtab don't know if @plt should
be added to symbol name for a PLT entry. The first pass checks
if @plt is needed and extra space is allocated in the second pass.
We can assume @plt is needed and waste some space if it isn't.
--
H.J.