[PATCH] net/bnxt: fix flow create segfault

[PATCH] net/bnxt: fix flow create segfault

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

When creating a flow that requires a destination queue but none is
specified in the flow create command, find_matching_vnic() returns
NULL and the code falls through to insert the new filter into
vnic->filter, dereferencing the NULL vnic and causing a segfault.

Check for a NULL vnic and reject the flow create with a clear error
instead of crashing.

Fixes: 59119d49529e ("net/bnxt: fix flow create when RSS is disabled")
Cc: stable@dpdk.org

Signed-off-by: Dakota Sicher <dakota.sicher@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_flow.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index a2e590540b..0a2899fbbd 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -2148,6 +2148,13 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	}
 
 	vnic = find_matching_vnic(bp, filter);
+	if (!vnic) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Missing destination action for flow.");
+		bnxt_clear_one_vnic_filter(bp, filter);
+		goto free_filter;
+	}
 done:
 	if (!ret || update_flow) {
 		flow->filter = filter;
-- 
2.47.3

[PATCH v2] net/bnxt: fix flow create segfault

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

When creating a flow that requires a destination queue but none is
specified in the flow create command, find_matching_vnic() returns
NULL and the code falls through to insert the new filter into
vnic->filter, dereferencing the NULL vnic and causing a segfault.

Check for a NULL vnic and reject the flow create with a clear error
instead of crashing.

Fixes: 59119d49529e ("net/bnxt: fix flow create when RSS is disabled")
Cc: stable@dpdk.org

Signed-off-by: Dakota Sicher <dakota.sicher@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v2:
* Changed the new NULL check from "if (!vnic)" to "if (vnic == NULL)"
  to match this file's established convention for pointer NULL checks
  (e.g. the existing "if (vnic == NULL)" a few hundred lines above).
* Kept RTE_FLOW_ERROR_TYPE_HANDLE rather than RTE_FLOW_ERROR_TYPE_ACTION
  as one reviewer suggested: every other rte_flow_error_set() call in
  this same function (bnxt_flow_create()) uses
  RTE_FLOW_ERROR_TYPE_HANDLE regardless of the underlying cause, so
  switching just this one would be inconsistent with its neighbors.
* Real bug found during v2 review: the new branch didn't reset "ret"
  before "goto free_filter". If bnxt_match_filter() had already set
  update_flow (ret == -EXDEV) and the filter is a plain L2 filter, the
  stale -EXDEV survived to the free_flow label and reported the
  misleading "pattern exists, updating destination queue" success-like
  error instead of the real failure. Now sets ret = -EINVAL on this
  path, same as every other error branch in this function.

 drivers/net/bnxt/bnxt_flow.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index a2e590540b..0a2899fbbd 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -2148,6 +2148,14 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	}

 	vnic = find_matching_vnic(bp, filter);
+	if (vnic == NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Missing destination action for flow.");
+		bnxt_clear_one_vnic_filter(bp, filter);
+		ret = -EINVAL;
+		goto free_filter;
+	}
 done:
 	if (!ret || update_flow) {
 		flow->filter = filter;
-- 
2.47.3

Re: [PATCH v2] net/bnxt: fix flow create segfault

[Stephen Hemminger]

On Sun, 20 Sep 2026 20:18:17 -0600
Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com> wrote:

> From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> 
> When creating a flow that requires a destination queue but none is
> specified in the flow create command, find_matching_vnic() returns
> NULL and the code falls through to insert the new filter into
> vnic->filter, dereferencing the NULL vnic and causing a segfault.
> 
> Check for a NULL vnic and reject the flow create with a clear error
> instead of crashing.
> 
> Fixes: 59119d49529e ("net/bnxt: fix flow create when RSS is disabled")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dakota Sicher <dakota.sicher@broadcom.com>
> Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> ---

Better AI review flagged an error here:

[PATCH v2] net/bnxt: fix flow create segfault

Error: new vnic == NULL branch corrupts filter lists when
update_flow is set.

When bnxt_match_filter() returns -EXDEV it has already done
STAILQ_INSERT_TAIL(&vnic->filter, nf, next) on the existing vnic and
set flow->filter = nf for the existing flow. If find_matching_vnic()
then returns NULL, the new branch calls bnxt_clear_one_vnic_filter()
and bnxt_free_filter() on that same nf. bnxt_free_filter() memsets
it and links it onto bp->free_filter_list while it is still on
vnic->filter. The existing flow is left pointing at a recycled filter
whose HW filter was just deleted. Before this patch the update_flow
path went to free_flow without touching vnic. Gate the new check on
!update_flow, or handle update_flow first.

[PATCH v3] net/bnxt: fix flow create segfault

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

When creating a flow that requires a destination queue but none is
specified in the flow create command, find_matching_vnic() returns
NULL and the code falls through to insert the new filter into
vnic->filter, dereferencing the NULL vnic and causing a segfault.

Reject a flow create with no destination up front, right after
bnxt_validate_and_parse_flow() and before bnxt_match_filter() is
called (skipped for HWRM_CFA_TUNNEL_REDIRECT_FILTER, which has no
destination VNIC by design). Checking this only after
bnxt_match_filter() is not enough: when the new filter's pattern
matches an already-installed flow with a different destination,
bnxt_match_filter() unlinks that flow's old filter, links the new
(about-to-be-rejected) one in its place, frees the old filter, and
repoints the existing flow's flow->filter at the new one, all before
returning -EXDEV. Rejecting the missing-destination case afterward
then frees that same new filter while it is still linked into
vnic->filter and still referenced by the pre-existing flow, leaving
that unrelated flow's HW filter deleted and flow->filter pointing at
freed memory. Checking before bnxt_match_filter() ever runs avoids
triggering that mutation in the first place.

Fixes: 59119d49529e ("net/bnxt: fix flow create when RSS is disabled")
Cc: stable@dpdk.org

Signed-off-by: Dakota Sicher <dakota.sicher@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v3:
* Moved the NULL-vnic check from right before the done: label (after
  the tunnel-redirect/EM/NTUPLE filter setup, where v2 had it) to
  right after bnxt_validate_and_parse_flow(), before
  bnxt_match_filter() runs. Stephen Hemminger found that checking
  only after bnxt_match_filter() has already run is not enough: for
  a flow that turns out to be an update (matches an existing flow's
  pattern with a different destination), bnxt_match_filter() has
  already unlinked the old filter, linked the new one in its place,
  freed the old one, and repointed the existing flow's flow->filter
  at the new one before returning -EXDEV -- rejecting afterward then
  frees that same new filter while it is still linked into
  vnic->filter and referenced by the pre-existing flow, corrupting
  that unrelated flow. Kishore Padmanabha separately flagged the old
  check's location as dead code once this move landed, since the
  earlier check already guarantees a match by the time execution
  reaches that point.
v2:
* Changed the NULL check from "if (!vnic)" to "if (vnic == NULL)" to
  match this file's established convention for pointer NULL checks.
* Kept RTE_FLOW_ERROR_TYPE_HANDLE rather than RTE_FLOW_ERROR_TYPE_ACTION
  as one reviewer suggested: every other rte_flow_error_set() call in
  this function uses RTE_FLOW_ERROR_TYPE_HANDLE regardless of the
  underlying cause.
* Fixed the new branch to reset ret = -EINVAL before goto free_filter,
  instead of leaving a stale update_flow-path -EXDEV to reach
  free_flow and report a misleading success-like error.

 drivers/net/bnxt/bnxt_flow.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 4bf38043b5..62dbb028f0 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -2047,6 +2047,16 @@ bnxt_flow_create(struct rte_eth_dev *dev,
 	if (ret != 0)
 		goto free_filter;
 
+	if (filter->filter_type != HWRM_CFA_TUNNEL_REDIRECT_FILTER &&
+	    find_matching_vnic(bp, filter) == NULL) {
+		rte_flow_error_set(error, EINVAL,
+				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
+				   "Missing destination action for flow.");
+		bnxt_hwrm_clear_l2_filter(bp, filter);
+		ret = -EINVAL;
+		goto free_filter;
+	}
+
 	ret = bnxt_match_filter(bp, filter);
 	if (ret == -EEXIST) {
 		PMD_DRV_LOG_LINE(DEBUG, "Flow already exists.");
-- 
2.47.3