[v2] graph: fix out of bounds access when re-allocate node objs

Message ID 20220804060241.1581110-1-zhirun.yan@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] graph: fix out of bounds access when re-allocate node objs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed

Commit Message

Yan, Zhirun Aug. 4, 2022, 6:02 a.m. UTC
  For __rte_node_enqueue_prologue(), If the number of objs is more than
the node->size * 2, the extra objs will write out of bounds memory.
It should use __rte_node_stream_alloc_size() to request enough memory.

And for rte_node_next_stream_put(), it will re-allocate a small size,
when the node free space is small and new objs is less than the current
node->size. Some objs pointers behind new size may be lost. And it will
cause memory leak. It should request enough size of memory, containing
the original objs and new objs at least.

Fixes: 40d4f51403ec ("graph: implement fastpath routines")

Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/graph/rte_graph_worker.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Jerin Jacob Kollanukkaran Sept. 20, 2022, 8:05 a.m. UTC | #1
> -----Original Message-----
> From: Zhirun Yan <zhirun.yan@intel.com>
> Sent: Thursday, August 4, 2022 11:33 AM
> To: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Kiran
> Kumar Kokkilagadda <kirankumark@marvell.com>
> Cc: Zhirun Yan <zhirun.yan@intel.com>; Cunming Liang
> <cunming.liang@intel.com>
> Subject: [EXT] [PATCH v2] graph: fix out of bounds access when re-allocate
> node objs
> 
> External Email
> 
> ----------------------------------------------------------------------
> For __rte_node_enqueue_prologue(), If the number of objs is more than the
> node->size * 2, the extra objs will write out of bounds memory.
> It should use __rte_node_stream_alloc_size() to request enough memory.
> 
> And for rte_node_next_stream_put(), it will re-allocate a small size, when the
> node free space is small and new objs is less than the current
> node->size. Some objs pointers behind new size may be lost. And it will
> cause memory leak. It should request enough size of memory, containing the
> original objs and new objs at least.
> 
> Fixes: 40d4f51403ec ("graph: implement fastpath routines")
> 
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>


Acked-by: Jerin Jacob <jerinj@marvell.com>

> ---
>  lib/graph/rte_graph_worker.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h index
> 0c0b9c095a..6dc7461659 100644
> --- a/lib/graph/rte_graph_worker.h
> +++ b/lib/graph/rte_graph_worker.h
> @@ -224,7 +224,7 @@ __rte_node_enqueue_prologue(struct rte_graph
> *graph, struct rte_node *node,
>  		__rte_node_enqueue_tail_update(graph, node);
> 
>  	if (unlikely(node->size < (idx + space)))
> -		__rte_node_stream_alloc(graph, node);
> +		__rte_node_stream_alloc_size(graph, node, node->size +
> space);
>  }
> 
>  /**
> @@ -432,7 +432,7 @@ rte_node_next_stream_get(struct rte_graph *graph,
> struct rte_node *node,
>  	uint16_t free_space = node->size - idx;
> 
>  	if (unlikely(free_space < nb_objs))
> -		__rte_node_stream_alloc_size(graph, node, nb_objs);
> +		__rte_node_stream_alloc_size(graph, node, node->size +
> nb_objs);
> 
>  	return &node->objs[idx];
>  }
> --
> 2.25.1
  
Thomas Monjalon Oct. 10, 2022, 3:31 p.m. UTC | #2
> > For __rte_node_enqueue_prologue(), If the number of objs is more than the
> > node->size * 2, the extra objs will write out of bounds memory.
> > It should use __rte_node_stream_alloc_size() to request enough memory.
> > 
> > And for rte_node_next_stream_put(), it will re-allocate a small size, when the
> > node free space is small and new objs is less than the current
> > node->size. Some objs pointers behind new size may be lost. And it will
> > cause memory leak. It should request enough size of memory, containing the
> > original objs and new objs at least.
> > 
> > Fixes: 40d4f51403ec ("graph: implement fastpath routines")
> > 
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> 
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied, thanks.
  

Patch

diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index 0c0b9c095a..6dc7461659 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -224,7 +224,7 @@  __rte_node_enqueue_prologue(struct rte_graph *graph, struct rte_node *node,
 		__rte_node_enqueue_tail_update(graph, node);
 
 	if (unlikely(node->size < (idx + space)))
-		__rte_node_stream_alloc(graph, node);
+		__rte_node_stream_alloc_size(graph, node, node->size + space);
 }
 
 /**
@@ -432,7 +432,7 @@  rte_node_next_stream_get(struct rte_graph *graph, struct rte_node *node,
 	uint16_t free_space = node->size - idx;
 
 	if (unlikely(free_space < nb_objs))
-		__rte_node_stream_alloc_size(graph, node, nb_objs);
+		__rte_node_stream_alloc_size(graph, node, node->size + nb_objs);
 
 	return &node->objs[idx];
 }