From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergio Gonzalez Monroy Subject: Re: [PATCH 1/1] lib/librte_eal: fix resource leak Date: Thu, 21 Apr 2016 12:19:02 +0100 Message-ID: <5718B726.5040300@intel.com> References: <1461083251-31140-1-git-send-email-marcinx.kerlin@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" , Marcin Kerlin To: David Marchand Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id F2EE03990 for ; Thu, 21 Apr 2016 13:19:20 +0200 (CEST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 20/04/2016 10:15, David Marchand wrote: > On Tue, Apr 19, 2016 at 6:27 PM, Marcin Kerlin wrote: >> Fix issue reported by Coverity. >> >> Coverity ID 13295, 13296, 13303: >> Resource leak: The system resource will not be reclaimed >> and reused, reducing the future availability of the resource. >> In rte_eal_hugepage_attach: Leak of memory or pointers to system >> resources. >> >> Fixes: af75078fece3 ("first public release") >> >> Signed-off-by: Marcin Kerlin >> --- >> lib/librte_eal/linuxapp/eal/eal_memory.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c >> index 5b9132c..6320aa0 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c >> @@ -1475,13 +1475,17 @@ rte_eal_hugepage_attach(void) >> "and retry running both primary " >> "and secondary processes\n"); >> } >> + >> + if (base_addr != MAP_FAILED) >> + munmap((void *)(uintptr_t)base_addr, mcfg->memseg[s].len); >> + > What is the point of this casting ? > Idem for the rest of the patch. I don't see the point either. Marcin? > > I can't see cleanup for previously mapped segments when mapping one fails. > Do we want this cleanup as well ? Good point. We are not really leaking resources because we panic if we fail to initialize eal memory. Now, if we are going to do the cleanup, I think that as David points out we should be cleaning up all previous mappings too. Sergio > CC Sergio. > >