diff mbox series

net/ice: fix double free ACL flow entry

Message ID 20210903100411.1693789-1-dapengx.yu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series net/ice: fix double free ACL flow entry |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing fail Testing issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Yu, DapengX Sept. 3, 2021, 10:04 a.m. UTC 
  From: Dapeng Yu <dapengx.yu@intel.com>

If call ice_flow_rem_entry() directly without checking entry_id, may
cause an ACL flow entry to be freed more than once.

This patch tries to find entry_id first, then call ice_flow_rem_entry()
to avoid the defect.

Fixes: 40d466fa9f76 ("net/ice: support ACL filter in DCF")
Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
 drivers/net/ice/ice_acl_filter.c | 33 +++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Simei Su Sept. 24, 2021, 5:40 a.m. UTC | #1 
> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Friday, September 3, 2021 6:04 PM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Su, Simei <simei.su@intel.com>; Yu, DapengX
> <dapengx.yu@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/ice: fix double free ACL flow entry
> 
> From: Dapeng Yu <dapengx.yu@intel.com>
> 
> If call ice_flow_rem_entry() directly without checking entry_id, may cause an
> ACL flow entry to be freed more than once.
> 
> This patch tries to find entry_id first, then call ice_flow_rem_entry() to avoid
> the defect.
> 
> Fixes: 40d466fa9f76 ("net/ice: support ACL filter in DCF")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> ---
>  drivers/net/ice/ice_acl_filter.c | 33 +++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_acl_filter.c b/drivers/net/ice/ice_acl_filter.c
> index 0c15a7036c..f44ce5d77e 100644
> --- a/drivers/net/ice/ice_acl_filter.c
> +++ b/drivers/net/ice/ice_acl_filter.c
> @@ -45,7 +45,7 @@ static struct ice_flow_parser ice_acl_parser;
> 
>  struct acl_rule {
>  	enum ice_fltr_ptype flow_type;
> -	uint32_t entry_id[4];
> +	uint64_t entry_id[4];
>  };
> 
>  static struct
> @@ -440,7 +440,7 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct
> ice_fdir_fltr *input,
>  			PMD_DRV_LOG(ERR, "Fail to add entry.");
>  			return ret;
>  		}
> -		rule->entry_id[entry_idx] = slot_id;
> +		rule->entry_id[entry_idx] = entry_id;
>  		pf->acl.hw_entry_id[slot_id] = hw_entry;
>  	} else {
>  		PMD_DRV_LOG(ERR, "Exceed the maximum entry number(%d)"
> @@ -451,18 +451,28 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct
> ice_fdir_fltr *input,
>  	return 0;
>  }
> 
> +static inline void
> +ice_acl_del_entry(struct ice_hw *hw, uint64_t entry_id) {
> +	uint64_t hw_entry;
> +
> +	hw_entry = ice_flow_find_entry(hw, ICE_BLK_ACL, entry_id);
> +	ice_flow_rem_entry(hw, ICE_BLK_ACL, hw_entry); }
> +
>  static inline void
>  ice_acl_hw_rem_conf(struct ice_pf *pf, struct acl_rule *rule, int32_t entry_idx)
> {
>  	uint32_t slot_id;
>  	int32_t i;
> +	uint64_t entry_id;
>  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
> 
>  	for (i = 0; i < entry_idx; i++) {
> -		slot_id = rule->entry_id[i];
> +		entry_id = rule->entry_id[i];
> +		slot_id = ICE_LO_DWORD(entry_id);
>  		rte_bitmap_set(pf->acl.slots, slot_id);
> -		ice_flow_rem_entry(hw, ICE_BLK_ACL,
> -				   pf->acl.hw_entry_id[slot_id]);
> +		ice_acl_del_entry(hw, entry_id);
>  	}
>  }
> 
> @@ -562,6 +572,7 @@ ice_acl_destroy_filter(struct ice_adapter *ad,  {
>  	struct acl_rule *rule = (struct acl_rule *)flow->rule;
>  	uint32_t slot_id, i;
> +	uint64_t entry_id;
>  	struct ice_pf *pf = &ad->pf;
>  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
>  	int ret = 0;
> @@ -569,19 +580,19 @@ ice_acl_destroy_filter(struct ice_adapter *ad,
>  	switch (rule->flow_type) {
>  	case ICE_FLTR_PTYPE_NONF_IPV4_OTHER:
>  		for (i = 0; i < 4; i++) {
> -			slot_id = rule->entry_id[i];
> +			entry_id = rule->entry_id[i];
> +			slot_id = ICE_LO_DWORD(entry_id);
>  			rte_bitmap_set(pf->acl.slots, slot_id);
> -			ice_flow_rem_entry(hw, ICE_BLK_ACL,
> -					   pf->acl.hw_entry_id[slot_id]);
> +			ice_acl_del_entry(hw, entry_id);
>  		}
>  		break;
>  	case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
>  	case ICE_FLTR_PTYPE_NONF_IPV4_TCP:
>  	case ICE_FLTR_PTYPE_NONF_IPV4_SCTP:
> -		slot_id = rule->entry_id[0];
> +		entry_id = rule->entry_id[0];
> +		slot_id = ICE_LO_DWORD(entry_id);
>  		rte_bitmap_set(pf->acl.slots, slot_id);
> -		ice_flow_rem_entry(hw, ICE_BLK_ACL,
> -				   pf->acl.hw_entry_id[slot_id]);
> +		ice_acl_del_entry(hw, entry_id);
>  		break;
>  	default:
>  		rte_flow_error_set(error, EINVAL,
> --
> 2.27.0

Reviewed-by: Simei Su <simei.su@intel.com>
Qi Zhang Sept. 24, 2021, 5:46 a.m. UTC | #2 
> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Friday, September 24, 2021 1:41 PM
> To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/ice: fix double free ACL flow entry
> 
> 
> 
> > -----Original Message-----
> > From: Yu, DapengX <dapengx.yu@intel.com>
> > Sent: Friday, September 3, 2021 6:04 PM
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Su, Simei <simei.su@intel.com>; Yu, DapengX
> > <dapengx.yu@intel.com>; stable@dpdk.org
> > Subject: [PATCH] net/ice: fix double free ACL flow entry
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>
> >
> > If call ice_flow_rem_entry() directly without checking entry_id, may
> > cause an ACL flow entry to be freed more than once.
> >
> > This patch tries to find entry_id first, then call
> > ice_flow_rem_entry() to avoid the defect.
> >
> > Fixes: 40d466fa9f76 ("net/ice: support ACL filter in DCF")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> > ---
> >  drivers/net/ice/ice_acl_filter.c | 33
> > +++++++++++++++++++++-----------
> >  1 file changed, 22 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_acl_filter.c
> > b/drivers/net/ice/ice_acl_filter.c
> > index 0c15a7036c..f44ce5d77e 100644
> > --- a/drivers/net/ice/ice_acl_filter.c
> > +++ b/drivers/net/ice/ice_acl_filter.c
> > @@ -45,7 +45,7 @@ static struct ice_flow_parser ice_acl_parser;
> >
> >  struct acl_rule {
> >  	enum ice_fltr_ptype flow_type;
> > -	uint32_t entry_id[4];
> > +	uint64_t entry_id[4];
> >  };
> >
> >  static struct
> > @@ -440,7 +440,7 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct
> > ice_fdir_fltr *input,
> >  			PMD_DRV_LOG(ERR, "Fail to add entry.");
> >  			return ret;
> >  		}
> > -		rule->entry_id[entry_idx] = slot_id;
> > +		rule->entry_id[entry_idx] = entry_id;
> >  		pf->acl.hw_entry_id[slot_id] = hw_entry;
> >  	} else {
> >  		PMD_DRV_LOG(ERR, "Exceed the maximum entry number(%d)"
> > @@ -451,18 +451,28 @@ ice_acl_hw_set_conf(struct ice_pf *pf, struct
> > ice_fdir_fltr *input,
> >  	return 0;
> >  }
> >
> > +static inline void
> > +ice_acl_del_entry(struct ice_hw *hw, uint64_t entry_id) {
> > +	uint64_t hw_entry;
> > +
> > +	hw_entry = ice_flow_find_entry(hw, ICE_BLK_ACL, entry_id);
> > +	ice_flow_rem_entry(hw, ICE_BLK_ACL, hw_entry); }
> > +
> >  static inline void
> >  ice_acl_hw_rem_conf(struct ice_pf *pf, struct acl_rule *rule, int32_t
> > entry_idx) {
> >  	uint32_t slot_id;
> >  	int32_t i;
> > +	uint64_t entry_id;
> >  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
> >
> >  	for (i = 0; i < entry_idx; i++) {
> > -		slot_id = rule->entry_id[i];
> > +		entry_id = rule->entry_id[i];
> > +		slot_id = ICE_LO_DWORD(entry_id);
> >  		rte_bitmap_set(pf->acl.slots, slot_id);
> > -		ice_flow_rem_entry(hw, ICE_BLK_ACL,
> > -				   pf->acl.hw_entry_id[slot_id]);
> > +		ice_acl_del_entry(hw, entry_id);
> >  	}
> >  }
> >
> > @@ -562,6 +572,7 @@ ice_acl_destroy_filter(struct ice_adapter *ad,  {
> >  	struct acl_rule *rule = (struct acl_rule *)flow->rule;
> >  	uint32_t slot_id, i;
> > +	uint64_t entry_id;
> >  	struct ice_pf *pf = &ad->pf;
> >  	struct ice_hw *hw = ICE_PF_TO_HW(pf);
> >  	int ret = 0;
> > @@ -569,19 +580,19 @@ ice_acl_destroy_filter(struct ice_adapter *ad,
> >  	switch (rule->flow_type) {
> >  	case ICE_FLTR_PTYPE_NONF_IPV4_OTHER:
> >  		for (i = 0; i < 4; i++) {
> > -			slot_id = rule->entry_id[i];
> > +			entry_id = rule->entry_id[i];
> > +			slot_id = ICE_LO_DWORD(entry_id);
> >  			rte_bitmap_set(pf->acl.slots, slot_id);
> > -			ice_flow_rem_entry(hw, ICE_BLK_ACL,
> > -					   pf->acl.hw_entry_id[slot_id]);
> > +			ice_acl_del_entry(hw, entry_id);
> >  		}
> >  		break;
> >  	case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
> >  	case ICE_FLTR_PTYPE_NONF_IPV4_TCP:
> >  	case ICE_FLTR_PTYPE_NONF_IPV4_SCTP:
> > -		slot_id = rule->entry_id[0];
> > +		entry_id = rule->entry_id[0];
> > +		slot_id = ICE_LO_DWORD(entry_id);
> >  		rte_bitmap_set(pf->acl.slots, slot_id);
> > -		ice_flow_rem_entry(hw, ICE_BLK_ACL,
> > -				   pf->acl.hw_entry_id[slot_id]);
> > +		ice_acl_del_entry(hw, entry_id);
> >  		break;
> >  	default:
> >  		rte_flow_error_set(error, EINVAL,
> > --
> > 2.27.0
> 
> Reviewed-by: Simei Su <simei.su@intel.com>
> 
Applied to dpdk-next-net-intel.

Thanks
Qi
>
diff mbox series

Patch

diff --git a/drivers/net/ice/ice_acl_filter.c b/drivers/net/ice/ice_acl_filter.c
index 0c15a7036c..f44ce5d77e 100644
--- a/drivers/net/ice/ice_acl_filter.c
+++ b/drivers/net/ice/ice_acl_filter.c
@@ -45,7 +45,7 @@  static struct ice_flow_parser ice_acl_parser;
 
 struct acl_rule {
 	enum ice_fltr_ptype flow_type;
-	uint32_t entry_id[4];
+	uint64_t entry_id[4];
 };
 
 static struct
@@ -440,7 +440,7 @@  ice_acl_hw_set_conf(struct ice_pf *pf, struct ice_fdir_fltr *input,
 			PMD_DRV_LOG(ERR, "Fail to add entry.");
 			return ret;
 		}
-		rule->entry_id[entry_idx] = slot_id;
+		rule->entry_id[entry_idx] = entry_id;
 		pf->acl.hw_entry_id[slot_id] = hw_entry;
 	} else {
 		PMD_DRV_LOG(ERR, "Exceed the maximum entry number(%d)"
@@ -451,18 +451,28 @@  ice_acl_hw_set_conf(struct ice_pf *pf, struct ice_fdir_fltr *input,
 	return 0;
 }
 
+static inline void
+ice_acl_del_entry(struct ice_hw *hw, uint64_t entry_id)
+{
+	uint64_t hw_entry;
+
+	hw_entry = ice_flow_find_entry(hw, ICE_BLK_ACL, entry_id);
+	ice_flow_rem_entry(hw, ICE_BLK_ACL, hw_entry);
+}
+
 static inline void
 ice_acl_hw_rem_conf(struct ice_pf *pf, struct acl_rule *rule, int32_t entry_idx)
 {
 	uint32_t slot_id;
 	int32_t i;
+	uint64_t entry_id;
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
 
 	for (i = 0; i < entry_idx; i++) {
-		slot_id = rule->entry_id[i];
+		entry_id = rule->entry_id[i];
+		slot_id = ICE_LO_DWORD(entry_id);
 		rte_bitmap_set(pf->acl.slots, slot_id);
-		ice_flow_rem_entry(hw, ICE_BLK_ACL,
-				   pf->acl.hw_entry_id[slot_id]);
+		ice_acl_del_entry(hw, entry_id);
 	}
 }
 
@@ -562,6 +572,7 @@  ice_acl_destroy_filter(struct ice_adapter *ad,
 {
 	struct acl_rule *rule = (struct acl_rule *)flow->rule;
 	uint32_t slot_id, i;
+	uint64_t entry_id;
 	struct ice_pf *pf = &ad->pf;
 	struct ice_hw *hw = ICE_PF_TO_HW(pf);
 	int ret = 0;
@@ -569,19 +580,19 @@  ice_acl_destroy_filter(struct ice_adapter *ad,
 	switch (rule->flow_type) {
 	case ICE_FLTR_PTYPE_NONF_IPV4_OTHER:
 		for (i = 0; i < 4; i++) {
-			slot_id = rule->entry_id[i];
+			entry_id = rule->entry_id[i];
+			slot_id = ICE_LO_DWORD(entry_id);
 			rte_bitmap_set(pf->acl.slots, slot_id);
-			ice_flow_rem_entry(hw, ICE_BLK_ACL,
-					   pf->acl.hw_entry_id[slot_id]);
+			ice_acl_del_entry(hw, entry_id);
 		}
 		break;
 	case ICE_FLTR_PTYPE_NONF_IPV4_UDP:
 	case ICE_FLTR_PTYPE_NONF_IPV4_TCP:
 	case ICE_FLTR_PTYPE_NONF_IPV4_SCTP:
-		slot_id = rule->entry_id[0];
+		entry_id = rule->entry_id[0];
+		slot_id = ICE_LO_DWORD(entry_id);
 		rte_bitmap_set(pf->acl.slots, slot_id);
-		ice_flow_rem_entry(hw, ICE_BLK_ACL,
-				   pf->acl.hw_entry_id[slot_id]);
+		ice_acl_del_entry(hw, entry_id);
 		break;
 	default:
 		rte_flow_error_set(error, EINVAL,