[PATCH] net/mlx5: fix segfault on indirect action age query with conntrack

[PATCH] net/mlx5: fix segfault on indirect action age query with conntrack

[Khadem Ullah]

This patch fixes a segmentation fault that occurs when querying the
age action of an indirect flow rule using connection tracking.

Steps to reproduce:
 1. Create an indirect action:
    flow indirect_action 0 create ingress action conntrack / end

 2. Create a root flow rule with a jump:
    flow create 0 ingress pattern eth / ipv4 / tcp / end /
         actions jump group 3 / end

 3. Create a group 3 rule using the indirect action:
    flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end /
         actions indirect 0 / jump group 5 / end

 4. Create a group 5 rule matching on conntrack state:
    flow create 0 group 5 ingress pattern eth / ipv4 / tcp /
         conntrack is 1 / end actions queue index 5 / end

 5. Querying the first rule causes a segmentation fault:
    flow query 0 1 age

This patch ensures proper handling of the indirect action with
conntrack to prevent this crash.

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 .mailmap                        | 1 +
 drivers/net/mlx5/mlx5_flow.c    | 2 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++++
 3 files changed, 8 insertions(+)

diff --git a/.mailmap b/.mailmap
index 8483d96ec5..5c9ea95346 100644
--- a/.mailmap
+++ b/.mailmap
@@ -812,6 +812,7 @@ Kevin Scott <kevin.c.scott@intel.com>
 Kevin Traynor <ktraynor@redhat.com>
 Ke Xu <ke1.xu@intel.com>
 Ke Zhang <ke1x.zhang@intel.com>
+Khadem Ullah <14pwcse@uetpeshawar.edu.pk>
 Khoa To <khot@microsoft.com>
 Kiran KN <kirankn@juniper.net>
 Kiran Kumar K <kirankumark@marvell.com> <kkokkilagadda@caviumnetworks.com> <kiran.kokkilagadda@caviumnetworks.com>
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 3d49a2d833..5c799ea4ce 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4550,6 +4550,8 @@ flow_aso_age_get_by_idx(struct rte_eth_dev *dev, uint32_t age_idx)
 	struct mlx5_aso_age_pool *pool;
 
 	rte_rwlock_read_lock(&mng->resize_rwl);
+	if (mng->pools == NULL)
+		return NULL;
 	pool = mng->pools[pool_idx];
 	rte_rwlock_read_unlock(&mng->resize_rwl);
 	return &pool->actions[offset - 1];
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c217634d9b..f81ce20385 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -18086,6 +18086,11 @@ flow_dv_query_age(struct rte_eth_dev *dev, struct rte_flow *flow,
 	if (flow->age) {
 		struct mlx5_aso_age_action *act =
 				     flow_aso_age_get_by_idx(dev, flow->age);
+		if (!act)
+			return rte_flow_error_set
+					(error, EINVAL,
+					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					 NULL, "cannot read age data");
 
 		age_param = &act->age_params;
 	} else if (flow->counter) {
-- 
2.43.0

Re: [PATCH] net/mlx5: fix segfault on indirect action age query with conntrack

[Dariusz Sosnowski]

Hi,

+Cc mlx5 maintainers

Thank you for the contribution. Please see the comments inline.

On Tue, Jun 24, 2025 at 01:10:15AM -0400, Khadem Ullah wrote:
> This patch fixes a segmentation fault that occurs when querying the
> age action of an indirect flow rule using connection tracking.
> 
> Steps to reproduce:
>  1. Create an indirect action:
>     flow indirect_action 0 create ingress action conntrack / end
> 
>  2. Create a root flow rule with a jump:
>     flow create 0 ingress pattern eth / ipv4 / tcp / end /
>          actions jump group 3 / end
> 
>  3. Create a group 3 rule using the indirect action:
>     flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end /
>          actions indirect 0 / jump group 5 / end
> 
>  4. Create a group 5 rule matching on conntrack state:
>     flow create 0 group 5 ingress pattern eth / ipv4 / tcp /
>          conntrack is 1 / end actions queue index 5 / end
> 
>  5. Querying the first rule causes a segmentation fault:
>     flow query 0 1 age
> 
> This patch ensures proper handling of the indirect action with
> conntrack to prevent this crash.

Could you please add the following Fixes tag to the commit message?

	Fixes: 2d084f69aa26 ("net/mlx5: add translation of connection tracking action")

This would allow LTS maintainers to pick up the fix for future LTS
releases.

> 
> Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
> ---
>  .mailmap                        | 1 +
>  drivers/net/mlx5/mlx5_flow.c    | 2 ++
>  drivers/net/mlx5/mlx5_flow_dv.c | 5 +++++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/.mailmap b/.mailmap
> index 8483d96ec5..5c9ea95346 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -812,6 +812,7 @@ Kevin Scott <kevin.c.scott@intel.com>
>  Kevin Traynor <ktraynor@redhat.com>
>  Ke Xu <ke1.xu@intel.com>
>  Ke Zhang <ke1x.zhang@intel.com>
> +Khadem Ullah <14pwcse@uetpeshawar.edu.pk>

Could you please make sure that the mail address in .mailmap and
Signed-off-by tag match?

>  Khoa To <khot@microsoft.com>
>  Kiran KN <kirankn@juniper.net>
>  Kiran Kumar K <kirankumark@marvell.com> <kkokkilagadda@caviumnetworks.com> <kiran.kokkilagadda@caviumnetworks.com>
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 3d49a2d833..5c799ea4ce 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -4550,6 +4550,8 @@ flow_aso_age_get_by_idx(struct rte_eth_dev *dev, uint32_t age_idx)
>  	struct mlx5_aso_age_pool *pool;
>  
>  	rte_rwlock_read_lock(&mng->resize_rwl);
> +	if (mng->pools == NULL)
> +		return NULL;

It is an interesting case.
When DV flow engine is used (current default),
connection tracking and age action cannot be used together.
Because of that, to optimize memory usage, age and CT index
are placed in a union.

The root cause of the crash is not a lack of ageing pools
(they are not initialized since no flow rule uses age action),
but the fact that age and CT index are in the union.
Since flow rule in repro steps use conntrack action,
flow->age value is misinterpreted.

In this case flow_aso_age_get_by_idx() should not be reached at all.

There's a check missing in flow_dv_query() for AGE action:

	flow_dv_query(...) { 
		for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
			switch (actions->type) {

			/* snip */

			case RTE_FLOW_ACTION_TYPE_AGE:
				/* missing check; if true, flow->age should not be read */
				if (flow->indirect_type == MLX5_INDIRECT_ACTION_TYPE_CT)
					return rte_flow_error_set(..., "age not available");
				ret = flow_dv_query_age(dev, flow, data, error);
				break;

			/* snip */
			}
	}

This check should resolve the segfault.

Would you be able to test that approach on your side and if all is good
resend the patch?

>  	pool = mng->pools[pool_idx];
>  	rte_rwlock_read_unlock(&mng->resize_rwl);
>  	return &pool->actions[offset - 1];
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index c217634d9b..f81ce20385 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -18086,6 +18086,11 @@ flow_dv_query_age(struct rte_eth_dev *dev, struct rte_flow *flow,
>  	if (flow->age) {
>  		struct mlx5_aso_age_action *act =
>  				     flow_aso_age_get_by_idx(dev, flow->age);
> +		if (!act)
> +			return rte_flow_error_set
> +					(error, EINVAL,
> +					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +					 NULL, "cannot read age data");
>  
>  		age_param = &act->age_params;
>  	} else if (flow->counter) {
> -- 
> 2.43.0
> 

Best regards,
Dariusz Sosnowski

[PATCH v2] net/mlx5: fix segfault on indirect action age query with conntrack

[Khadem Ullah]

v2:
 - Added missing check for AGE + CT conflict in flow_dv_query().
 - Removed unnecessary null check from flow_aso_age_get_by_idx().
 - Added Fixes tag for LTS tracking.
 - Ensured .mailmap and Signed-off-by addresses match.

This patch fixes a segmentation fault that occurs when querying the
AGE action of a flow rule that uses indirect connection tracking (CT).

Background:
AGE and CT indices share a union in the mlx5 flow struct. When using CT
without age, the age index is invalid. Querying AGE in this case leads
to a crash due to reading an invalid pointer.

Fix:
Add a check in `flow_dv_query()` to prevent AGE queries on indirect CT
actions. This is the correct fix rather than null-checking the pool.

Steps to reproduce:
 1. Create an indirect CT action:
    flow indirect_action 0 create ingress action conntrack / end

 2. Create a root rule with jump:
    flow create 0 ingress pattern eth / ipv4 / tcp / end actions jump group 3 / end

 3. Create a group 3 rule using the indirect action:
    flow create 0 group 3 ingress pattern eth / ipv4 / tcp / end actions indirect 0 / jump group 5 / end

 4. Create a group 5 rule matching CT state:
    flow create 0 group 5 ingress pattern eth / ipv4 / tcp / conntrack is 1 / end actions queue index 5 / end

 5. Querying the first rule causes segfault:
    flow query 0 1 age

Fixes: 2d084f69aa26 ("net/mlx5: add translation of connection tracking action")
Cc: stable@dpdk.org

Signed-off-by: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
---
 .mailmap                        | 1 +
 drivers/net/mlx5/mlx5_flow_dv.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/.mailmap b/.mailmap
index 8483d96ec5..6126f7e472 100644
--- a/.mailmap
+++ b/.mailmap
@@ -812,6 +812,7 @@ Kevin Scott <kevin.c.scott@intel.com>
 Kevin Traynor <ktraynor@redhat.com>
 Ke Xu <ke1.xu@intel.com>
 Ke Zhang <ke1x.zhang@intel.com>
+Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
 Khoa To <khot@microsoft.com>
 Kiran KN <kirankn@juniper.net>
 Kiran Kumar K <kirankumark@marvell.com> <kkokkilagadda@caviumnetworks.com> <kiran.kokkilagadda@caviumnetworks.com>
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c217634d9b..7ce093e075 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -18134,6 +18134,11 @@ flow_dv_query(struct rte_eth_dev *dev,
 						  error);
 			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
+			if (flow->indirect_type == MLX5_INDIRECT_ACTION_TYPE_CT)
+				return rte_flow_error_set(error, ENOTSUP,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "age not available");
 			ret = flow_dv_query_age(dev, flow, data, error);
 			break;
 		default:
-- 
2.43.0