[dpdk-dev,v2,2/3] malloc: fix potential out-of-bounds array access

Message ID 157358c48a85cad762a1afb850d130be98997726.1524651111.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Anatoly Burakov April 25, 2018, 10:15 a.m. UTC
  Technically, while the pointer would've been invalid if msl_idx
were invalid, we wouldn't have actually attempted to access the
pointer until verifying the index. Fix it by moving array access
to after we've verified validity of the index.

Coverity issue: 272574

Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/malloc_heap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Van Haaren, Harry April 27, 2018, 3:57 p.m. UTC | #1
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Wednesday, April 25, 2018 11:16 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH v2 2/3] malloc: fix potential out-of-bounds array
> access
> 
> Technically, while the pointer would've been invalid if msl_idx
> were invalid, we wouldn't have actually attempted to access the
> pointer until verifying the index. Fix it by moving array access
> to after we've verified validity of the index.
> 
> Coverity issue: 272574
> 
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>


Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
  

Patch

diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index 590e9e3..5cf7231 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -99,11 +99,12 @@  malloc_add_seg(const struct rte_memseg_list *msl,
 
 	/* msl is const, so find it */
 	msl_idx = msl - mcfg->memsegs;
-	found_msl = &mcfg->memsegs[msl_idx];
 
 	if (msl_idx < 0 || msl_idx >= RTE_MAX_MEMSEG_LISTS)
 		return -1;
 
+	found_msl = &mcfg->memsegs[msl_idx];
+
 	malloc_heap_add_memory(heap, found_msl, ms->addr, len);
 
 	RTE_LOG(DEBUG, EAL, "Added %zuM to heap on socket %i\n", len >> 20,