[PATCH v1 1/1] net/ice: fix memory leak in raw pattern parse

[PATCH v1 1/1] net/ice: fix memory leak in raw pattern parse

[Anatoly Burakov]

Currently, when parsing rte_flow raw type items, we allocate a bunch of
internal structures, but if errors happen down the line we just quit and
never free the memory we just allocated. Fix this by adding an error
cleanup section.

Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/ice_fdir_filter.c | 40 ++++++++++++++++++-------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c
index d41d223d52..7aa8fc3994 100644
--- a/drivers/net/intel/ice/ice_fdir_filter.c
+++ b/drivers/net/intel/ice/ice_fdir_filter.c
@@ -1867,7 +1867,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			uint16_t tmp_val = 0;
 			uint16_t pkt_len = 0;
 			uint8_t tmp = 0;
-			int i, j;
+			int i, j, ret_val;
 
 			pkt_len = strlen((char *)(uintptr_t)raw_spec->pattern);
 			if (strlen((char *)(uintptr_t)raw_mask->pattern) !=
@@ -1922,24 +1922,34 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 
 			pkt_len /= 2;
 
-			if (ice_parser_run(ad->psr, tmp_spec, pkt_len, &rslt))
-				return -rte_errno;
+			if (ice_parser_run(ad->psr, tmp_spec, pkt_len, &rslt)) {
+				ret_val = -rte_errno;
+				goto raw_error;
+			}
 
-			if (!tmp_mask)
-				return -rte_errno;
+			if (!tmp_mask) {
+				ret_val = -rte_errno;
+				goto raw_error;
+			}
 
 			filter->prof = (struct ice_parser_profile *)
 				ice_malloc(&ad->hw, sizeof(*filter->prof));
-			if (!filter->prof)
-				return -ENOMEM;
+			if (!filter->prof) {
+				ret_val = -ENOMEM;
+				goto raw_error;
+			}
 
 			if (ice_parser_profile_init(&rslt, tmp_spec, tmp_mask,
-				pkt_len, ICE_BLK_FD, true, filter->prof))
-				return -rte_errno;
+				pkt_len, ICE_BLK_FD, true, filter->prof)) {
+				ret_val = -rte_errno;
+				goto raw_error_prof;
+			}
 
 			u8 *pkt_buf = (u8 *)ice_malloc(&ad->hw, pkt_len + 1);
-			if (!pkt_buf)
-				return -ENOMEM;
+			if (!pkt_buf) {
+				ret_val = -ENOMEM;
+				goto raw_error_prof;
+			}
 			rte_memcpy(pkt_buf, tmp_spec, pkt_len);
 			filter->pkt_buf = pkt_buf;
 
@@ -1950,6 +1960,14 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			rte_free(tmp_spec);
 			rte_free(tmp_mask);
 			break;
+
+raw_error_prof:
+			rte_free(filter->prof);
+			filter->prof = NULL;
+raw_error:
+			rte_free(tmp_spec);
+			rte_free(tmp_mask);
+			return ret_val;
 		}
 
 		case RTE_FLOW_ITEM_TYPE_ETH:
-- 
2.47.3

[PATCH v2 1/2] net/ice: remove indirection for FDIR filters

[Anatoly Burakov]

Currently, when filters are created for FDIR, they are allocated
dynamically using `ice_malloc()`. Not only this is inconsistent with hash
filter code paths (hash uses embedded structures), this is also creating
unnecessary indirection and complexity, and creates a memory leak where,
if something fails during raw pattern parse, the profile memory isn't
deallocated.

Since there is no actual reason for why FDIR filter profile must use
indirection, instead of fixing the memory leak just avoid it altogether
by making the filter profile an embedded struct. When parsing begins, the
entire scratch filter structure is zeroed out anyway, so there is no need
to add any additional zero-initialization code.

Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2:
    - Added this patch

 drivers/net/intel/ice/ice_ethdev.h      |  2 +-
 drivers/net/intel/ice/ice_fdir_filter.c | 30 +++++++++----------------
 2 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/net/intel/ice/ice_ethdev.h b/drivers/net/intel/ice/ice_ethdev.h
index a0d2082206..6478d6dfbd 100644
--- a/drivers/net/intel/ice/ice_ethdev.h
+++ b/drivers/net/intel/ice/ice_ethdev.h
@@ -379,7 +379,7 @@ struct ice_fdir_filter_conf {
 	uint64_t input_set_i; /* only for tunnel inner fields */
 	uint32_t mark_flag;
 
-	struct ice_parser_profile *prof;
+	struct ice_parser_profile prof;
 	bool parser_ena;
 	u8 *pkt_buf;
 	u8 pkt_len;
diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c
index d41d223d52..d593624792 100644
--- a/drivers/net/intel/ice/ice_fdir_filter.c
+++ b/drivers/net/intel/ice/ice_fdir_filter.c
@@ -1314,7 +1314,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 	if (filter->parser_ena) {
 		struct ice_hw *hw = ICE_PF_TO_HW(pf);
 
-		int id = ice_find_first_bit(filter->prof->ptypes, UINT16_MAX);
+		int id = ice_find_first_bit(filter->prof.ptypes, UINT16_MAX);
 		int ptg = hw->blk[ICE_BLK_FD].xlt1.t[id];
 		u16 ctrl_vsi = pf->fdir.fdir_vsi->idx;
 		u16 main_vsi = pf->main_vsi->idx;
@@ -1324,11 +1324,11 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 		if (pi->fdir_actived_cnt != 0) {
 			for (i = 0; i < ICE_MAX_FV_WORDS; i++)
 				if (pi->prof.fv[i].proto_id !=
-				    filter->prof->fv[i].proto_id ||
+				    filter->prof.fv[i].proto_id ||
 				    pi->prof.fv[i].offset !=
-				    filter->prof->fv[i].offset ||
+				    filter->prof.fv[i].offset ||
 				    pi->prof.fv[i].msk !=
-				    filter->prof->fv[i].msk)
+				    filter->prof.fv[i].msk)
 					break;
 			if (i == ICE_MAX_FV_WORDS) {
 				fv_found = true;
@@ -1338,7 +1338,7 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 
 		if (!fv_found) {
 			ret = ice_flow_set_hw_prof(hw, main_vsi, ctrl_vsi,
-						   filter->prof, ICE_BLK_FD);
+						   &filter->prof, ICE_BLK_FD);
 			if (ret)
 				goto error;
 		}
@@ -1348,12 +1348,12 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 			goto error;
 
 		if (!fv_found) {
-			for (i = 0; i < filter->prof->fv_num; i++) {
+			for (i = 0; i < filter->prof.fv_num; i++) {
 				pi->prof.fv[i].proto_id =
-					filter->prof->fv[i].proto_id;
+					filter->prof.fv[i].proto_id;
 				pi->prof.fv[i].offset =
-					filter->prof->fv[i].offset;
-				pi->prof.fv[i].msk = filter->prof->fv[i].msk;
+					filter->prof.fv[i].offset;
+				pi->prof.fv[i].msk = filter->prof.fv[i].msk;
 			}
 			pi->fdir_actived_cnt = 1;
 		}
@@ -1451,7 +1451,6 @@ ice_fdir_create_filter(struct ice_adapter *ad,
 	return -rte_errno;
 
 error:
-	rte_free(filter->prof);
 	rte_free(filter->pkt_buf);
 	return -rte_errno;
 }
@@ -1473,7 +1472,7 @@ ice_fdir_destroy_filter(struct ice_adapter *ad,
 	if (filter->parser_ena) {
 		struct ice_hw *hw = ICE_PF_TO_HW(pf);
 
-		int id = ice_find_first_bit(filter->prof->ptypes, UINT16_MAX);
+		int id = ice_find_first_bit(filter->prof.ptypes, UINT16_MAX);
 		int ptg = hw->blk[ICE_BLK_FD].xlt1.t[id];
 		u16 ctrl_vsi = pf->fdir.fdir_vsi->idx;
 		u16 main_vsi = pf->main_vsi->idx;
@@ -1501,7 +1500,6 @@ ice_fdir_destroy_filter(struct ice_adapter *ad,
 
 		flow->rule = NULL;
 
-		rte_free(filter->prof);
 		rte_free(filter->pkt_buf);
 		rte_free(filter);
 
@@ -1928,13 +1926,8 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			if (!tmp_mask)
 				return -rte_errno;
 
-			filter->prof = (struct ice_parser_profile *)
-				ice_malloc(&ad->hw, sizeof(*filter->prof));
-			if (!filter->prof)
-				return -ENOMEM;
-
 			if (ice_parser_profile_init(&rslt, tmp_spec, tmp_mask,
-				pkt_len, ICE_BLK_FD, true, filter->prof))
+				pkt_len, ICE_BLK_FD, true, &filter->prof))
 				return -rte_errno;
 
 			u8 *pkt_buf = (u8 *)ice_malloc(&ad->hw, pkt_len + 1);
@@ -2495,7 +2488,6 @@ ice_fdir_parse(struct ice_adapter *ad,
 	rte_free(item);
 	return ret;
 error:
-	rte_free(filter->prof);
 	rte_free(filter->pkt_buf);
 	rte_free(item);
 	return ret;
-- 
2.47.3

[PATCH v2 2/2] net/ice: fix memory leak in raw pattern parse

[Anatoly Burakov]

Currently, when parsing rte_flow raw type items, we allocate a bunch of
internal structures, but if errors happen down the line we just quit and
never free the memory we just allocated. Fix this by adding an error
cleanup section.

Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2:
    - Removed the filter profile allocation error
      handling as it's now directly embedded in the
      filter structure and not allocated dynamically.

 drivers/net/intel/ice/ice_fdir_filter.c | 31 ++++++++++++++++++-------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c
index d593624792..3090a5d13c 100644
--- a/drivers/net/intel/ice/ice_fdir_filter.c
+++ b/drivers/net/intel/ice/ice_fdir_filter.c
@@ -1865,7 +1865,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			uint16_t tmp_val = 0;
 			uint16_t pkt_len = 0;
 			uint8_t tmp = 0;
-			int i, j;
+			int i, j, ret_val;
 
 			pkt_len = strlen((char *)(uintptr_t)raw_spec->pattern);
 			if (strlen((char *)(uintptr_t)raw_mask->pattern) !=
@@ -1920,19 +1920,27 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 
 			pkt_len /= 2;
 
-			if (ice_parser_run(ad->psr, tmp_spec, pkt_len, &rslt))
-				return -rte_errno;
+			if (ice_parser_run(ad->psr, tmp_spec, pkt_len, &rslt)) {
+				ret_val = -rte_errno;
+				goto raw_error;
+			}
 
-			if (!tmp_mask)
-				return -rte_errno;
+			if (!tmp_mask) {
+				ret_val = -rte_errno;
+				goto raw_error;
+			}
 
 			if (ice_parser_profile_init(&rslt, tmp_spec, tmp_mask,
-				pkt_len, ICE_BLK_FD, true, &filter->prof))
-				return -rte_errno;
+				pkt_len, ICE_BLK_FD, true, &filter->prof)) {
+				ret_val = -rte_errno;
+				goto raw_error;
+			}
 
 			u8 *pkt_buf = (u8 *)ice_malloc(&ad->hw, pkt_len + 1);
-			if (!pkt_buf)
-				return -ENOMEM;
+			if (!pkt_buf) {
+				ret_val = -ENOMEM;
+				goto raw_error;
+			}
 			rte_memcpy(pkt_buf, tmp_spec, pkt_len);
 			filter->pkt_buf = pkt_buf;
 
@@ -1943,6 +1951,11 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 			rte_free(tmp_spec);
 			rte_free(tmp_mask);
 			break;
+
+raw_error:
+			rte_free(tmp_spec);
+			rte_free(tmp_mask);
+			return ret_val;
 		}
 
 		case RTE_FLOW_ITEM_TYPE_ETH:
-- 
2.47.3

Re: [PATCH v2 2/2] net/ice: fix memory leak in raw pattern parse

[Burakov, Anatoly]

On 10/7/2025 2:42 PM, Anatoly Burakov wrote:
> Currently, when parsing rte_flow raw type items, we allocate a bunch of
> internal structures, but if errors happen down the line we just quit and
> never free the memory we just allocated. Fix this by adding an error
> cleanup section.
> 
> Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 

Fixline might be incorrect, i need to dig in a little more, as 
rte_malloc calls weren't there in that specific commit.

-- 
Thanks,
Anatoly