From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96403 invoked by alias); 29 Oct 2015 00:04:54 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 96393 invoked by uid 89); 29 Oct 2015 00:04:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f52.google.com Received: from mail-pa0-f52.google.com (HELO mail-pa0-f52.google.com) (209.85.220.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 29 Oct 2015 00:04:52 +0000 Received: by padhk11 with SMTP id hk11so21243740pad.1 for ; Wed, 28 Oct 2015 17:04:51 -0700 (PDT) X-Received: by 10.67.5.193 with SMTP id co1mr39811000pad.134.1446077090958; Wed, 28 Oct 2015 17:04:50 -0700 (PDT) Received: from bubble.grove.modra.org (CPE-58-160-163-67.gqzg1.fli.bigpond.net.au. [58.160.163.67]) by smtp.gmail.com with ESMTPSA id fl5sm6998296pbd.70.2015.10.28.17.04.49 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Oct 2015 17:04:49 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 19A32EA1537; Thu, 29 Oct 2015 10:34:45 +1030 (ACDT) Date: Thu, 29 Oct 2015 00:04:00 -0000 From: Alan Modra To: Nick Clifton Cc: Thomas Preud'homme , binutils@sourceware.org Subject: Re: [PATCH, binutils/ARM] Fix leak of local internal symbols in elf32_arm_size_stubs Message-ID: <20151029000444.GG13961@bubble.grove.modra.org> References: <002501d10adf$89816b40$9c8441c0$@arm.com> <5630E1DC.9080306@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5630E1DC.9080306@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg00282.txt.bz2 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