The mlx5_ipool_free() function was called with a NULL pool pointer
during HW flow destruction, causing a segmentation fault. This occurred
when flow creation failed and the cleanup path attempted to free
resources from an uninitialized flow pool.
The crash happened in the following scenario:
1. During device start, a default NTA copy action flow is created
2. If the flow creation fails, mlx5_flow_hw_list_destroy() is called
3. In hw_cmpl_flow_update_or_destroy(), table->flow pool could be NULL
4. mlx5_ipool_free(table->flow, flow->idx) was called without checking
if table->flow is NULL
5. Inside mlx5_ipool_free(), accessing pool->cfg.per_core_cache caused
a segmentation fault due to NULL pointer dereference
The fix adds two layers of protection,
1. Add NULL check for table->flow before calling mlx5_ipool_free() in
hw_cmpl_flow_update_or_destroy(), consistent with the existing check
for table->resource on the previous line
2. Add NULL check for pool parameter in mlx5_ipool_free() as a defensive
measure to prevent similar crashes in other code paths
The fix also renames the ‘flow’ field in rte_flow_template_table
to ‘flow_pool’ for better code readability.
Stack trace of the fault:
mlx5_ipool_free (pool=0x0) at mlx5_utils.c:753
hw_cmpl_flow_update_or_destroy at mlx5_flow_hw.c:4481
mlx5_flow_hw_destroy at mlx5_flow_hw.c:14219
mlx5_flow_hw_list_destroy at mlx5_flow_hw.c:14279
flow_hw_list_create at mlx5_flow_hw.c:14415
mlx5_flow_start_default at mlx5_flow.c:8263
mlx5_dev_start at mlx5_trigger.c:1420
Fixes: 27d171b88031 ("net/mlx5: abstract flow action and enable reconfigure")
Cc: [email protected]
Signed-off-by: Maayan Kashani <[email protected]>
Acked-by: Bing Zhao <[email protected]>
---
drivers/net/mlx5/mlx5_flow.h | 2 +-
drivers/net/mlx5/mlx5_flow_hw.c | 25 +++++++++++++------------
drivers/net/mlx5/mlx5_utils.c | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 83a4adc971f..71e7c1f6bb9 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1744,7 +1744,7 @@ struct rte_flow_template_table {
struct rte_flow_pattern_template *its[MLX5_HW_TBL_MAX_ITEM_TEMPLATE];
/* Action templates bind to the table. */
struct mlx5_hw_action_template ats[MLX5_HW_TBL_MAX_ACTION_TEMPLATE];
- struct mlx5_indexed_pool *flow; /* The table's flow ipool. */
+ struct mlx5_indexed_pool *flow_pool; /* The table's flow ipool. */
struct rte_flow_hw_aux *flow_aux; /**< Auxiliary data stored per flow.
*/
struct mlx5_indexed_pool *resource; /* The table's resource ipool. */
struct mlx5_flow_template_table_cfg cfg;
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index e0f79932a56..52e42422ce7 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3952,7 +3952,7 @@ flow_hw_async_flow_create_generic(struct rte_eth_dev *dev,
items, pattern_template_index, actions,
action_template_index, error))
return NULL;
}
- flow = mlx5_ipool_malloc(table->flow, &flow_idx);
+ flow = mlx5_ipool_malloc(table->flow_pool, &flow_idx);
if (!flow) {
rte_errno = ENOMEM;
goto error;
@@ -4042,7 +4042,7 @@ flow_hw_async_flow_create_generic(struct rte_eth_dev *dev,
if (table->resource && res_idx)
mlx5_ipool_free(table->resource, res_idx);
if (flow_idx)
- mlx5_ipool_free(table->flow, flow_idx);
+ mlx5_ipool_free(table->flow_pool, flow_idx);
if (sub_error.cause != RTE_FLOW_ERROR_TYPE_NONE && error != NULL)
*error = sub_error;
else
@@ -4492,7 +4492,8 @@ hw_cmpl_flow_update_or_destroy(struct rte_eth_dev *dev,
if (!flow->nt_rule) {
if (table->resource)
mlx5_ipool_free(table->resource, res_idx);
- mlx5_ipool_free(table->flow, flow->idx);
+ if (table->flow_pool)
+ mlx5_ipool_free(table->flow_pool, flow->idx);
}
}
}
@@ -4780,7 +4781,7 @@ flow_hw_q_flow_flush(struct rte_eth_dev *dev,
LIST_FOREACH(tbl, &priv->flow_hw_tbl, next) {
if (!tbl->cfg.external)
continue;
- MLX5_IPOOL_FOREACH(tbl->flow, fidx, flow) {
+ MLX5_IPOOL_FOREACH(tbl->flow_pool, fidx, flow) {
if (flow_hw_async_flow_destroy(dev,
MLX5_DEFAULT_FLUSH_QUEUE,
&attr,
@@ -5102,8 +5103,8 @@ flow_hw_table_create(struct rte_eth_dev *dev,
goto error;
tbl->cfg = *table_cfg;
/* Allocate flow indexed pool. */
- tbl->flow = mlx5_ipool_create(&cfg);
- if (!tbl->flow)
+ tbl->flow_pool = mlx5_ipool_create(&cfg);
+ if (!tbl->flow_pool)
goto error;
/* Allocate table of auxiliary flow rule structs. */
tbl->flow_aux = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct
rte_flow_hw_aux) * nb_flows,
@@ -5258,8 +5259,8 @@ flow_hw_table_create(struct rte_eth_dev *dev,
&tbl->grp->entry);
if (tbl->flow_aux)
mlx5_free(tbl->flow_aux);
- if (tbl->flow)
- mlx5_ipool_destroy(tbl->flow);
+ if (tbl->flow_pool)
+ mlx5_ipool_destroy(tbl->flow_pool);
mlx5_free(tbl);
}
if (error != NULL) {
@@ -5489,10 +5490,10 @@ flow_hw_table_destroy(struct rte_eth_dev *dev,
/* Build ipool allocated object bitmap. */
if (table->resource)
mlx5_ipool_flush_cache(table->resource);
- mlx5_ipool_flush_cache(table->flow);
+ mlx5_ipool_flush_cache(table->flow_pool);
/* Check if ipool has allocated objects. */
if (table->refcnt ||
- mlx5_ipool_get_next(table->flow, &fidx) ||
+ mlx5_ipool_get_next(table->flow_pool, &fidx) ||
(table->resource && mlx5_ipool_get_next(table->resource, &ridx))) {
DRV_LOG(WARNING, "Table %p is still in use.", (void *)table);
return rte_flow_error_set(error, EBUSY,
@@ -5522,7 +5523,7 @@ flow_hw_table_destroy(struct rte_eth_dev *dev,
if (table->resource)
mlx5_ipool_destroy(table->resource);
mlx5_free(table->flow_aux);
- mlx5_ipool_destroy(table->flow);
+ mlx5_ipool_destroy(table->flow_pool);
mlx5_free(table);
return 0;
}
@@ -15310,7 +15311,7 @@ flow_hw_table_resize(struct rte_eth_dev *dev,
return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
table, "shrinking table is not
supported");
- ret = mlx5_ipool_resize(table->flow, nb_flows, error);
+ ret = mlx5_ipool_resize(table->flow_pool, nb_flows, error);
if (ret)
return ret;
/*
diff --git a/drivers/net/mlx5/mlx5_utils.c b/drivers/net/mlx5/mlx5_utils.c
index cba8cc3f490..defcf80dd7d 100644
--- a/drivers/net/mlx5/mlx5_utils.c
+++ b/drivers/net/mlx5/mlx5_utils.c
@@ -748,7 +748,7 @@ mlx5_ipool_free(struct mlx5_indexed_pool *pool, uint32_t
idx)
uint32_t trunk_idx;
uint32_t entry_idx;
- if (!idx)
+ if (!pool || !idx)
return;
if (pool->cfg.per_core_cache) {
mlx5_ipool_free_cache(pool, idx);
--
2.21.0
