[PATCH] net/mlx5: fix segmentation fault in flow destruction path

[PATCH] net/mlx5: fix segmentation fault in flow destruction path

[Maayan Kashani]

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: stable@dpdk.org

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Bing Zhao <bingz@nvidia.com>
---
 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