* [PATCH v1] graph: fix out of bounds access when re-allocate node objs
@ 2022-07-27 2:39 Zhirun Yan
2022-08-01 13:13 ` Jerin Jacob
2022-08-04 6:02 ` [PATCH v2] " Zhirun Yan
0 siblings, 2 replies; 5+ messages in thread
From: Zhirun Yan @ 2022-07-27 2:39 UTC (permalink / raw)
To: dev, jerinj, kirankumark; +Cc: Zhirun Yan, Liang, Cunming
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: Liang, Cunming <cunming.liang@intel.com>
---
lib/graph/rte_graph_worker.h | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
index 0c0b9c095a..b7d145c3cb 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker.h
@@ -218,13 +218,16 @@ static __rte_always_inline void
__rte_node_enqueue_prologue(struct rte_graph *graph, struct rte_node *node,
const uint16_t idx, const uint16_t space)
{
+ uint32_t req_size;
/* Add to the pending stream list if the node is new */
if (idx == 0)
__rte_node_enqueue_tail_update(graph, node);
- if (unlikely(node->size < (idx + space)))
- __rte_node_stream_alloc(graph, node);
+ if (unlikely(node->size < (idx + space))) {
+ req_size = rte_align32pow2(node->size + space);
+ __rte_node_stream_alloc_size(graph, node, req_size);
+ }
}
/**
@@ -430,9 +433,12 @@ rte_node_next_stream_get(struct rte_graph *graph, struct rte_node *node,
node = __rte_node_next_node_get(node, next);
const uint16_t idx = node->idx;
uint16_t free_space = node->size - idx;
+ uint32_t req_size;
- if (unlikely(free_space < nb_objs))
- __rte_node_stream_alloc_size(graph, node, nb_objs);
+ if (unlikely(free_space < nb_objs)) {
+ req_size = rte_align32pow2(node->size + nb_objs);
+ __rte_node_stream_alloc_size(graph, node, req_size);
+ }
return &node->objs[idx];
}
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] graph: fix out of bounds access when re-allocate node objs
2022-07-27 2:39 [PATCH v1] graph: fix out of bounds access when re-allocate node objs Zhirun Yan
@ 2022-08-01 13:13 ` Jerin Jacob
2022-08-04 6:02 ` [PATCH v2] " Zhirun Yan
1 sibling, 0 replies; 5+ messages in thread
From: Jerin Jacob @ 2022-08-01 13:13 UTC (permalink / raw)
To: Zhirun Yan; +Cc: dpdk-dev, Jerin Jacob, Kiran Kumar K, Liang, Cunming
On Wed, Jul 27, 2022 at 8:10 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> 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: Liang, Cunming <cunming.liang@intel.com>
> ---
> lib/graph/rte_graph_worker.h | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h
> index 0c0b9c095a..b7d145c3cb 100644
> --- a/lib/graph/rte_graph_worker.h
> +++ b/lib/graph/rte_graph_worker.h
> @@ -218,13 +218,16 @@ static __rte_always_inline void
> __rte_node_enqueue_prologue(struct rte_graph *graph, struct rte_node *node,
> const uint16_t idx, const uint16_t space)
> {
> + uint32_t req_size;
>
> /* Add to the pending stream list if the node is new */
> if (idx == 0)
> __rte_node_enqueue_tail_update(graph, node);
>
> - if (unlikely(node->size < (idx + space)))
> - __rte_node_stream_alloc(graph, node);
> + if (unlikely(node->size < (idx + space))) {
> + req_size = rte_align32pow2(node->size + space);
> + __rte_node_stream_alloc_size(graph, node, req_size);
> + }
Change looks good to me.
Please have an inline function to avoid code duplication(Same change
in rte_node_next_stream_get())
With above change:
Acked-by: Jerin Jacob <jerinj@marvell.com>
> }
>
> /**
> @@ -430,9 +433,12 @@ rte_node_next_stream_get(struct rte_graph *graph, struct rte_node *node,
> node = __rte_node_next_node_get(node, next);
> const uint16_t idx = node->idx;
> uint16_t free_space = node->size - idx;
> + uint32_t req_size;
>
> - if (unlikely(free_space < nb_objs))
> - __rte_node_stream_alloc_size(graph, node, nb_objs);
> + if (unlikely(free_space < nb_objs)) {
> + req_size = rte_align32pow2(node->size + nb_objs);
> + __rte_node_stream_alloc_size(graph, node, req_size);
> + }
>
> return &node->objs[idx];
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] graph: fix out of bounds access when re-allocate node objs
2022-07-27 2:39 [PATCH v1] graph: fix out of bounds access when re-allocate node objs Zhirun Yan
2022-08-01 13:13 ` Jerin Jacob
@ 2022-08-04 6:02 ` Zhirun Yan
2022-09-20 8:05 ` [EXT] " Jerin Jacob Kollanukkaran
1 sibling, 1 reply; 5+ messages in thread
From: Zhirun Yan @ 2022-08-04 6:02 UTC (permalink / raw)
To: dev, jerinj, kirankumark; +Cc: Zhirun Yan, Cunming Liang
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(-)
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [EXT] [PATCH v2] graph: fix out of bounds access when re-allocate node objs
2022-08-04 6:02 ` [PATCH v2] " Zhirun Yan
@ 2022-09-20 8:05 ` Jerin Jacob Kollanukkaran
2022-10-10 15:31 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2022-09-20 8:05 UTC (permalink / raw)
To: Zhirun Yan, dev, Kiran Kumar Kokkilagadda; +Cc: Cunming Liang
> -----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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [EXT] [PATCH v2] graph: fix out of bounds access when re-allocate node objs
2022-09-20 8:05 ` [EXT] " Jerin Jacob Kollanukkaran
@ 2022-10-10 15:31 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2022-10-10 15:31 UTC (permalink / raw)
To: Zhirun Yan, Jerin Jacob Kollanukkaran
Cc: dev, Kiran Kumar Kokkilagadda, Cunming Liang
> > 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-10 15:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-27 2:39 [PATCH v1] graph: fix out of bounds access when re-allocate node objs Zhirun Yan
2022-08-01 13:13 ` Jerin Jacob
2022-08-04 6:02 ` [PATCH v2] " Zhirun Yan
2022-09-20 8:05 ` [EXT] " Jerin Jacob Kollanukkaran
2022-10-10 15:31 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).