[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

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

[Khadem Ullah]

[-- Attachment #1: Type: text/plain, Size: 2937 bytes --]

Thank you very much for the review and Ack.

I’ll make sure to include version change notes under a notes section in
future patch versions, as per the contribution guide.

Also noted about maintainers, I had added them in v1 but missed them in v2.
I’ll make sure to always use `get-maintainer.sh` going forward.

Thanks again for the guidance and support.

Best regards,
Khadem Ullah


On Thu, Jun 26, 2025, 18:33 Dariusz Sosnowski <dsosnowski@nvidia.com> wrote:

> Thank you very much for changes and detailed descriptions.
> It helped a lot during review.
>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
>
> On Thu, Jun 26, 2025 at 09:07:02AM -0400, Khadem Ullah wrote:
> > 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.
>
> In case of any future contribution would you be able to put the changes
> between versions in notes section of the patch?
> You can find the details here:
> https://doc.dpdk.org/guides/contributing/patches.html#creating-patches
>
> Also, in the future would you be able to send patches to all relevant
> maintainers? We have a script, ./devtools/get-maintainer.sh,
> which extracts the info from MAINTAINERS file.
> You can find more info here:
> https://doc.dpdk.org/guides/contributing/patches.html#sending-patches
>
> >
> > 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>
>

[-- Attachment #2: Type: text/html, Size: 4190 bytes --]

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

[Dariusz Sosnowski]

Thank you very much for changes and detailed descriptions.
It helped a lot during review.

Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

On Thu, Jun 26, 2025 at 09:07:02AM -0400, Khadem Ullah wrote:
> 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.

In case of any future contribution would you be able to put the changes
between versions in notes section of the patch?
You can find the details here: https://doc.dpdk.org/guides/contributing/patches.html#creating-patches

Also, in the future would you be able to send patches to all relevant
maintainers? We have a script, ./devtools/get-maintainer.sh,
which extracts the info from MAINTAINERS file.
You can find more info here: https://doc.dpdk.org/guides/contributing/patches.html#sending-patches

> 
> 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>

[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