Re: [PATCH, binutils/ARM] Fix leak of local internal symbols in elf32_arm_size_stubs

[Alan Modra <amodra at gmail dot com>]

On Wed, Oct 28, 2015 at 02:55:24PM +0000, Nick Clifton wrote:
> Hi Thomas,
> 
> >In elf32_arm_size_stubs, when encountering a relocation against a local symbol for the first time in a given input section, bfd_elf_get_elf_syms is called if symtab_hdr->contents is NULL. However, the allocation performed by this function is never freed, hence a potential leak if such a situation occurs. This patch adds a free before exiting the scope in which local_syms is valid.
> 
> Hmm, something seems slightly wrong here...
> 
> >  		      if (elf_section_data (section)->relocs == NULL)
> >  			free (internal_relocs);
> >+		      if (!symtab_hdr->contents)
> >+			free (local_syms);
> >  		      goto error_ret_free_local;
> 
> Why doesn't the code at the error_ret_free_local label actually free the
> local symbols as the name implies ?  [Answer: because the label is outside
> of the scope of local_syms.  But why ?  If the label were inside the scope
> it could free the memory and then return, making the patch above
> unnecessary].
> 
> Also - why do you need to check symtab_hdr->contents ?  Wouldn't it make
> more sense to check "local_syms != NULL" ?
> 
> 
> >+	  if (!symtab_hdr->contents)
> >+	    {
> >+	      free (local_syms);
> >+	      local_syms = NULL;
> >+	    }
> 
> Again it would appear to make more sense to check local_syms than
> symtab_hdr->contents.

Thomas, it might pay to take a look at how elf64-ppc.c
ppc64_elf_size_stubs handles this.

-- 
Alan Modra
Australia Development Lab, IBM