mem: fix resource leak

Message ID 9183826f1b6f3bb3d80cdde12fbd59f07f4df348.1540212745.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series mem: fix resource leak |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anatoly Burakov Oct. 22, 2018, 12:57 p.m. UTC
  Segment preallocation code allocates an array of structures on the
heap but does not free the memory afterwards. Fix it by freeing it
at the end of the function, and changing control flow to always go
through that code path.

Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")

Coverity ID: 323524

Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    Not sure if stable release script picks up fixes for commits intended
    for stable releases?

 lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
  

Comments

Thomas Monjalon Oct. 22, 2018, 8:34 p.m. UTC | #1
22/10/2018 14:57, Anatoly Burakov:
> Segment preallocation code allocates an array of structures on the
> heap but does not free the memory afterwards. Fix it by freeing it
> at the end of the function, and changing control flow to always go
> through that code path.
> 
> Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
> 
> Coverity ID: 323524
> 
> Cc: stable@dpdk.org

The right order for these tags is:

Coverity issue: 323524
Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
Cc: stable@dpdk.org

> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     Not sure if stable release script picks up fixes for commits intended
>     for stable releases?

What do you mean? Is this patch for master?
The process is to Cc: stable, then it is backported.
The script check-git-log.sh helps you to correctly set Cc: stable tag.
  
Thomas Monjalon Oct. 23, 2018, 9:37 a.m. UTC | #2
22/10/2018 22:34, Thomas Monjalon:
> 22/10/2018 14:57, Anatoly Burakov:
> > Segment preallocation code allocates an array of structures on the
> > heap but does not free the memory afterwards. Fix it by freeing it
> > at the end of the function, and changing control flow to always go
> > through that code path.
> > 
> > Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
> > 
> > Coverity ID: 323524
> > 
> > Cc: stable@dpdk.org
> 
> The right order for these tags is:
> 
> Coverity issue: 323524
> Fixes: 1dd342d0fdc4 ("mem: improve segment list preallocation")
> Cc: stable@dpdk.org
> 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 19e686eb6..fce86fda6 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -2135,7 +2135,7 @@  memseg_primary_init(void)
 		uint64_t page_sz;
 		int socket_id;
 	} *memtypes = NULL;
-	int i, hpi_idx, msl_idx;
+	int i, hpi_idx, msl_idx, ret = -1; /* fail unless told to succeed */
 	struct rte_memseg_list *msl;
 	uint64_t max_mem, max_mem_per_type;
 	unsigned int max_seglists_per_type;
@@ -2227,7 +2227,7 @@  memseg_primary_init(void)
 	if (max_seglists_per_type == 0) {
 		RTE_LOG(ERR, EAL, "Cannot accommodate all memory types, please increase %s\n",
 			RTE_STR(CONFIG_RTE_MAX_MEMSEG_LISTS));
-		return -1;
+		goto out;
 	}
 
 	/* go through all mem types and create segment lists */
@@ -2287,21 +2287,25 @@  memseg_primary_init(void)
 				RTE_LOG(ERR, EAL,
 					"No more space in memseg lists, please increase %s\n",
 					RTE_STR(CONFIG_RTE_MAX_MEMSEG_LISTS));
-				return -1;
+				goto out;
 			}
 			msl = &mcfg->memsegs[msl_idx++];
 
 			if (alloc_memseg_list(msl, pagesz, n_segs,
 					socket_id, cur_seglist))
-				return -1;
+				goto out;
 
 			if (alloc_va_space(msl)) {
 				RTE_LOG(ERR, EAL, "Cannot allocate VA space for memseg list\n");
-				return -1;
+				goto out;
 			}
 		}
 	}
-	return 0;
+	/* we're successful */
+	ret = 0;
+out:
+	free(memtypes);
+	return ret;
 }
 
 static int