[PATCH v2 5/8] net/ice: fix DCF QoS bandwidth replay config leak

[Anurag Mandal]

ice_dcf_hierarchy_commit() keeps a copy of the bandwidth config
of each VF, and of the TC nodes, in qos_bw_cfg[] to replay it
after a VF reset.
Each DCF hierarchy commit allocates commit allocates new copies
of the VF and TC bandwidth config for VF reset replay, without
freeing the previous ones,
They cannot simply be freed there, as the VF reset thread may be
replaying them at the same time.
A new copy is also visible to that thread before it is filled in.
Also, ice_dcf_uninit_hw() frees the array but not the copies, so
they also leak on close and reset.

Fill each new copy first, then swap it in under the lock the VF
reset thread holds while replaying, and free the old copy after
that and free all copies in ice_dcf_uninit_hw().

Fixes: 931ee54072b1 ("net/ice: support QoS bandwidth config after VF reset in DCF")
Fixes: 775a25a372f5 ("net/ice: clear QoS bandwidth on DCF close")
Cc: stable at dpdk.org

Signed-off-by: Anurag Mandal <anurag.mandal at intel.com>
---
 drivers/net/intel/ice/ice_dcf.c        |  6 ++++++
 drivers/net/intel/ice/ice_dcf_ethdev.h |  2 ++
 drivers/net/intel/ice/ice_dcf_parent.c | 15 +++++++++++++++
 drivers/net/intel/ice/ice_dcf_sched.c  | 17 +++++++++--------
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/net/intel/ice/ice_dcf.c b/drivers/net/intel/ice/ice_dcf.c
index f94856ad9c..4bc3c07d81 100644
--- a/drivers/net/intel/ice/ice_dcf.c
+++ b/drivers/net/intel/ice/ice_dcf.c
@@ -884,6 +884,7 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 {
 	struct rte_pci_device *pci_dev = RTE_CLASS_TO_BUS_DEVICE(eth_dev, *pci_dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	uint16_t i;
 
 	if (hw->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
 		if (hw->tm_conf.committed) {
@@ -919,6 +920,11 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw)
 	rte_free(hw->rss_key);
 	hw->rss_key = NULL;
 
+	if (hw->qos_bw_cfg != NULL) {
+		for (i = 0; i < hw->num_vfs; i++)
+			rte_free(hw->qos_bw_cfg[i]);
+	}
+
 	rte_free(hw->qos_bw_cfg);
 	hw->qos_bw_cfg = NULL;
 
diff --git a/drivers/net/intel/ice/ice_dcf_ethdev.h b/drivers/net/intel/ice/ice_dcf_ethdev.h
index 6dcbaac5eb..4560be6330 100644
--- a/drivers/net/intel/ice/ice_dcf_ethdev.h
+++ b/drivers/net/intel/ice/ice_dcf_ethdev.h
@@ -75,6 +75,8 @@ void ice_dcf_handle_pf_event_msg(struct ice_dcf_hw *dcf_hw,
 				 uint8_t *msg, uint16_t msglen);
 int ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev);
 void ice_dcf_uninit_parent_adapter(struct rte_eth_dev *eth_dev);
+void ice_dcf_replace_qos_bw_cfg(struct ice_dcf_hw *hw, uint16_t idx,
+				struct virtchnl_dcf_bw_cfg_list *bw_cfg);
 
 int ice_devargs_check(struct rte_devargs *devargs, enum ice_dcf_devrarg devarg_type);
 int ice_dcf_vf_repr_init(struct rte_eth_dev *vf_rep_eth_dev, void *init_param);
diff --git a/drivers/net/intel/ice/ice_dcf_parent.c b/drivers/net/intel/ice/ice_dcf_parent.c
index d5d37f8810..4d620fe8f5 100644
--- a/drivers/net/intel/ice/ice_dcf_parent.c
+++ b/drivers/net/intel/ice/ice_dcf_parent.c
@@ -162,6 +162,21 @@ ice_dcf_vsi_update_service_handler(void *param)
 	return 0;
 }
 
+void
+ice_dcf_replace_qos_bw_cfg(struct ice_dcf_hw *hw, uint16_t idx,
+			   struct virtchnl_dcf_bw_cfg_list *bw_cfg)
+{
+	struct virtchnl_dcf_bw_cfg_list *old;
+
+	/* the VF reset thread replays this config while holding the lock */
+	rte_spinlock_lock(&vsi_update_lock);
+	old = hw->qos_bw_cfg[idx];
+	hw->qos_bw_cfg[idx] = bw_cfg;
+	rte_spinlock_unlock(&vsi_update_lock);
+
+	rte_free(old);
+}
+
 static void
 start_vsi_reset_thread(struct ice_dcf_hw *dcf_hw, bool vfr, uint16_t vf_id)
 {
diff --git a/drivers/net/intel/ice/ice_dcf_sched.c b/drivers/net/intel/ice/ice_dcf_sched.c
index de28019c5b..a524d9c777 100644
--- a/drivers/net/intel/ice/ice_dcf_sched.c
+++ b/drivers/net/intel/ice/ice_dcf_sched.c
@@ -725,6 +725,7 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 	struct ice_dcf_hw *hw = &adapter->real_hw;
 	struct virtchnl_dcf_bw_cfg_list *vf_bw = NULL;
 	struct virtchnl_dcf_bw_cfg_list *tc_bw = NULL;
+	struct virtchnl_dcf_bw_cfg_list *bw_cfg;
 	struct ice_dcf_tm_node_list *vsi_list = &hw->tm_conf.vsi_list;
 	struct rte_tm_shaper_params *profile;
 	struct ice_dcf_tm_node *tm_node;
@@ -813,14 +814,14 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 		if (ret_val)
 			goto fail_clear;
 
-		hw->qos_bw_cfg[vf_id] = rte_zmalloc("vf_bw_cfg", size, 0);
-		if (!hw->qos_bw_cfg[vf_id]) {
+		bw_cfg = rte_zmalloc("vf_bw_cfg", size, 0);
+		if (!bw_cfg) {
 			ret_val = ICE_ERR_NO_MEMORY;
 			goto fail_clear;
 		}
 		/* store the bandwidth information for replay */
-		ice_memcpy(hw->qos_bw_cfg[vf_id], vf_bw, size,
-			   ICE_NONDMA_TO_NONDMA);
+		ice_memcpy(bw_cfg, vf_bw, size, ICE_NONDMA_TO_NONDMA);
+		ice_dcf_replace_qos_bw_cfg(hw, vf_id, bw_cfg);
 		ice_memset(vf_bw, 0, size, ICE_NONDMA_MEM);
 	}
 
@@ -840,13 +841,13 @@ static int ice_dcf_hierarchy_commit(struct rte_eth_dev *dev,
 		goto fail_clear;
 
 	/* store TC node bw configuration */
-	hw->qos_bw_cfg[ICE_DCF_VFID] = rte_zmalloc("tc_bw_cfg", size, 0);
-	if (!hw->qos_bw_cfg[ICE_DCF_VFID]) {
+	bw_cfg = rte_zmalloc("tc_bw_cfg", size, 0);
+	if (!bw_cfg) {
 		ret_val = ICE_ERR_NO_MEMORY;
 		goto fail_clear;
 	}
-	ice_memcpy(hw->qos_bw_cfg[ICE_DCF_VFID], tc_bw, size,
-		   ICE_NONDMA_TO_NONDMA);
+	ice_memcpy(bw_cfg, tc_bw, size, ICE_NONDMA_TO_NONDMA);
+	ice_dcf_replace_qos_bw_cfg(hw, ICE_DCF_VFID, bw_cfg);
 
 	hw->tm_conf.committed = true;
 	free(vf_bw);
-- 
2.25.1