* [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues [not found] <cover.1661223500.git.wangyunjian@huawei.com> @ 2022-08-23 6:45 ` Yunjian Wang 2022-09-23 9:31 ` wangyunjian 2022-08-23 6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang 1 sibling, 1 reply; 7+ messages in thread From: Yunjian Wang @ 2022-08-23 6:45 UTC (permalink / raw) To: dev Cc: matan, rasland, viacheslavo, dkozlyuk, huangshaozhang, Yunjian Wang, stable The bonding slave remove function was calling the eth_dev_tx_queue_config function, which frees dev->data->tx_queues, and then tries to free priv->txqs[idx] in mlx5_txq_release function, which causes the heap use after free issue. Add checks whether dev->data->tx_queues is not NULL. Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- drivers/net/mlx5/mlx5_txq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index 0140f8b3b2..cb2c33a060 100644 --- a/drivers/net/mlx5/mlx5_txq.c +++ b/drivers/net/mlx5/mlx5_txq.c @@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev, uint16_t idx) struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_txq_ctrl *txq_ctrl; - if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL) + if (dev->data->tx_queues == NULL || priv->txqs == NULL || + (*priv->txqs)[idx] == NULL) return 0; txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, txq); if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1) -- 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues 2022-08-23 6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang @ 2022-09-23 9:31 ` wangyunjian 2022-09-26 12:30 ` Slava Ovsiienko 0 siblings, 1 reply; 7+ messages in thread From: wangyunjian @ 2022-09-23 9:31 UTC (permalink / raw) To: dev; +Cc: matan, rasland, viacheslavo, dkozlyuk, Huangshaozhang, stable Friendly ping. > -----Original Message----- > From: wangyunjian > Sent: Tuesday, August 23, 2022 2:46 PM > To: dev@dpdk.org > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com; > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>; > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing > tx queues > > The bonding slave remove function was calling the eth_dev_tx_queue_config > function, which frees dev->data->tx_queues, and then tries to free > priv->txqs[idx] in mlx5_txq_release function, which causes the heap use > after free issue. Add checks whether dev->data->tx_queues is not NULL. > > Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > drivers/net/mlx5/mlx5_txq.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index > 0140f8b3b2..cb2c33a060 100644 > --- a/drivers/net/mlx5/mlx5_txq.c > +++ b/drivers/net/mlx5/mlx5_txq.c > @@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev, uint16_t > idx) > struct mlx5_priv *priv = dev->data->dev_private; > struct mlx5_txq_ctrl *txq_ctrl; > > - if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL) > + if (dev->data->tx_queues == NULL || priv->txqs == NULL || > + (*priv->txqs)[idx] == NULL) > return 0; > txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, txq); > if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1) > -- > 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues 2022-09-23 9:31 ` wangyunjian @ 2022-09-26 12:30 ` Slava Ovsiienko 0 siblings, 0 replies; 7+ messages in thread From: Slava Ovsiienko @ 2022-09-26 12:30 UTC (permalink / raw) To: wangyunjian, dev Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable Hi, Yunjian Could you, please, tell more details about problematic scenario? In bonding slave? It is not fully clean for me how mlx5_txq_release frees priv->txqs[idx] (BTW NULL is OK to free, it is safe). We have check for NULL here: > > - if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL) priv->txq is internal objects managed by PMD, dev->data->tx_queues are DPDK-wide ones. Theoretically it might happen when DPDK objects are created and internals are not, and vice versa. So, checking for existence of external objects in the routine that manages internals does not look so reasonable. Internal queue object management is based on the atomic reference counter and, generally speaking, should not depend on externals. With best regards, Slava > -----Original Message----- > From: wangyunjian <wangyunjian@huawei.com> > Sent: Friday, September 23, 2022 12:32 > To: dev@dpdk.org > Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; > Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk > <dkozlyuk@nvidia.com>; Huangshaozhang <huangshaozhang@huawei.com>; > stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when > releasing tx queues > > Friendly ping. > > > -----Original Message----- > > From: wangyunjian > > Sent: Tuesday, August 23, 2022 2:46 PM > > To: dev@dpdk.org > > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com; > > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>; > > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when > > releasing tx queues > > > > The bonding slave remove function was calling the > > eth_dev_tx_queue_config function, which frees dev->data->tx_queues, > > and then tries to free > > priv->txqs[idx] in mlx5_txq_release function, which causes the heap > > priv->use > > after free issue. Add checks whether dev->data->tx_queues is not NULL. > > > > Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > drivers/net/mlx5/mlx5_txq.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c > > index > > 0140f8b3b2..cb2c33a060 100644 > > --- a/drivers/net/mlx5/mlx5_txq.c > > +++ b/drivers/net/mlx5/mlx5_txq.c > > @@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev, > > uint16_t > > idx) > > struct mlx5_priv *priv = dev->data->dev_private; > > struct mlx5_txq_ctrl *txq_ctrl; > > > > - if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL) > > + if (dev->data->tx_queues == NULL || priv->txqs == NULL || > > + (*priv->txqs)[idx] == NULL) > > return 0; > > txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, > txq); > > if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1) > > -- > > 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action [not found] <cover.1661223500.git.wangyunjian@huawei.com> 2022-08-23 6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang @ 2022-08-23 6:46 ` Yunjian Wang 2022-09-23 9:31 ` wangyunjian 1 sibling, 1 reply; 7+ messages in thread From: Yunjian Wang @ 2022-08-23 6:46 UTC (permalink / raw) To: dev Cc: matan, rasland, viacheslavo, dkozlyuk, huangshaozhang, Yunjian Wang, stable Currently, the resources for hrxq->action are allocated in mlx5_devx_hrxq_new(). But it was not being freed when the drop action was released in mlx5_devx_drop_action_destroy(). So, fix is to free the resources in mlx5_devx_tir_destroy(). Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c | 6 ------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/net/mlx5/mlx5_devx.c b/drivers/net/mlx5/mlx5_devx.c index 6886ae1f22..09c8856f05 100644 --- a/drivers/net/mlx5/mlx5_devx.c +++ b/drivers/net/mlx5/mlx5_devx.c @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq, static void mlx5_devx_tir_destroy(struct mlx5_hrxq *hrxq) { +#if defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_INFINIBAND_VERBS_H) + if (hrxq->hws_flags) + mlx5dr_action_destroy(hrxq->action); + else + mlx5_flow_os_destroy_flow_action(hrxq->action); + hrxq->action = NULL; +#endif claim_zero(mlx5_devx_cmd_destroy(hrxq->tir)); } diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index eaf23d0df4..e518fe9bfd 100644 --- a/drivers/net/mlx5/mlx5_rxq.c +++ b/drivers/net/mlx5/mlx5_rxq.c @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq) { struct mlx5_priv *priv = dev->data->dev_private; -#ifdef HAVE_IBV_FLOW_DV_SUPPORT - if (hrxq->hws_flags) - mlx5dr_action_destroy(hrxq->action); - else - mlx5_glue->destroy_flow_action(hrxq->action); -#endif priv->obj_ops.hrxq_destroy(hrxq); if (!hrxq->standalone) { mlx5_ind_table_obj_release(dev, hrxq->ind_table, -- 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action 2022-08-23 6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang @ 2022-09-23 9:31 ` wangyunjian 2022-09-26 19:35 ` Slava Ovsiienko 0 siblings, 1 reply; 7+ messages in thread From: wangyunjian @ 2022-09-23 9:31 UTC (permalink / raw) To: dev; +Cc: matan, rasland, viacheslavo, dkozlyuk, Huangshaozhang, stable Friendly ping. > -----Original Message----- > From: wangyunjian > Sent: Tuesday, August 23, 2022 2:46 PM > To: dev@dpdk.org > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com; > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>; > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a > drop action > > Currently, the resources for hrxq->action are allocated in > mlx5_devx_hrxq_new(). But it was not being freed when the drop action was > released in mlx5_devx_drop_action_destroy(). > So, fix is to free the resources in mlx5_devx_tir_destroy(). > > Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c | > 6 ------ > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5_devx.c b/drivers/net/mlx5/mlx5_devx.c index > 6886ae1f22..09c8856f05 100644 > --- a/drivers/net/mlx5/mlx5_devx.c > +++ b/drivers/net/mlx5/mlx5_devx.c > @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev, > struct mlx5_hrxq *hrxq, static void mlx5_devx_tir_destroy(struct > mlx5_hrxq *hrxq) { > +#if defined(HAVE_IBV_FLOW_DV_SUPPORT) > || !defined(HAVE_INFINIBAND_VERBS_H) > + if (hrxq->hws_flags) > + mlx5dr_action_destroy(hrxq->action); > + else > + mlx5_flow_os_destroy_flow_action(hrxq->action); > + hrxq->action = NULL; > +#endif > claim_zero(mlx5_devx_cmd_destroy(hrxq->tir)); > } > > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index > eaf23d0df4..e518fe9bfd 100644 > --- a/drivers/net/mlx5/mlx5_rxq.c > +++ b/drivers/net/mlx5/mlx5_rxq.c > @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev, > struct mlx5_hrxq *hrxq) { > struct mlx5_priv *priv = dev->data->dev_private; > > -#ifdef HAVE_IBV_FLOW_DV_SUPPORT > - if (hrxq->hws_flags) > - mlx5dr_action_destroy(hrxq->action); > - else > - mlx5_glue->destroy_flow_action(hrxq->action); > -#endif > priv->obj_ops.hrxq_destroy(hrxq); > if (!hrxq->standalone) { > mlx5_ind_table_obj_release(dev, hrxq->ind_table, > -- > 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action 2022-09-23 9:31 ` wangyunjian @ 2022-09-26 19:35 ` Slava Ovsiienko 2022-09-29 1:51 ` wangyunjian 0 siblings, 1 reply; 7+ messages in thread From: Slava Ovsiienko @ 2022-09-26 19:35 UTC (permalink / raw) To: wangyunjian, dev Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable Hi, Yunjian We have drop action in mlx5. To create/destroy drop action we have mlx5_drop_action_create/ mlx5_drop_action_destroy common routines. As PMD supports operation either over FW (Verbs in rdma_core) or SW/HW steering with DevX objects the "virtual" methods are used for objects: priv->obj_ops.drop_action_create(dev) - create drop action a) mlx5_ibv_drop_action_create()- use Verbs (dv-flow_en == 0) b) mlx5_devx_drop_action_create() - use DevX (dv-flow_en != 0) priv->obj_ops.drop_action_destroy(dev) - destroy drop action a) mlx5_ibv_drop_action_destroy()- use Verbs (dv-flow_en == 0) b) mlx5_devx_drop_action_destroy() - use DevX (dv-flow_en != 0) Let's consider DevX case. mlx5_devx_drop_action_create() mlx5_rxq_devx_obj_drop_create() mlx5_devx_ind_table_new() mlx5_devx_hrxq_new() xxx_create_dest_tir() xxx_action_create_dest_tir() mlx5_devx_drop_action_destroy() /* It seems action destroy is missing here */ mlx5_devx_tir_destroy() mlx5_devx_ind_table_destroy() mlx5_rxq_devx_obj_drop_release() So, yes, I agree. There is missing action destroy, it seems we have leakage. Let's look at __mlx5_hrxq_remove() destroy_action() priv->obj_ops.hrxq_destroy(); And there we have a problem. obj_ops.hrxq_destroy() - not always mlx5_devx_tir_destroy() is called. It might be mlx5_ibv_qp_destroy() (for Verbas) and patch would not work. So, instead of fixing __mlx5_hrxq_remove() and mlx5_devx_tir_destroy() I would consider patching: - mlx5_devx_drop_action_destroy() - mlx5_ibv_drop_action_destroy by adding action desatroying. With best regards, Slava > -----Original Message----- > From: wangyunjian <wangyunjian@huawei.com> > Sent: Friday, September 23, 2022 12:32 > To: dev@dpdk.org > Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; > Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk > <dkozlyuk@nvidia.com>; Huangshaozhang <huangshaozhang@huawei.com>; > stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when > releasing a drop action > > Friendly ping. > > > -----Original Message----- > > From: wangyunjian > > Sent: Tuesday, August 23, 2022 2:46 PM > > To: dev@dpdk.org > > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com; > > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>; > > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when > > releasing a drop action > > > > Currently, the resources for hrxq->action are allocated in > > mlx5_devx_hrxq_new(). But it was not being freed when the drop action > > was released in mlx5_devx_drop_action_destroy(). > > So, fix is to free the resources in mlx5_devx_tir_destroy(). > > > > Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c > > | > > 6 ------ > > 2 files changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5_devx.c > > b/drivers/net/mlx5/mlx5_devx.c index > > 6886ae1f22..09c8856f05 100644 > > --- a/drivers/net/mlx5/mlx5_devx.c > > +++ b/drivers/net/mlx5/mlx5_devx.c > > @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev, > > struct mlx5_hrxq *hrxq, static void mlx5_devx_tir_destroy(struct > > mlx5_hrxq *hrxq) { > > +#if defined(HAVE_IBV_FLOW_DV_SUPPORT) > > || !defined(HAVE_INFINIBAND_VERBS_H) > > + if (hrxq->hws_flags) > > + mlx5dr_action_destroy(hrxq->action); > > + else > > + mlx5_flow_os_destroy_flow_action(hrxq->action); > > + hrxq->action = NULL; > > +#endif > > claim_zero(mlx5_devx_cmd_destroy(hrxq->tir)); > > } > > > > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c > > index eaf23d0df4..e518fe9bfd 100644 > > --- a/drivers/net/mlx5/mlx5_rxq.c > > +++ b/drivers/net/mlx5/mlx5_rxq.c > > @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev, > > struct mlx5_hrxq *hrxq) { > > struct mlx5_priv *priv = dev->data->dev_private; > > > > -#ifdef HAVE_IBV_FLOW_DV_SUPPORT > > - if (hrxq->hws_flags) > > - mlx5dr_action_destroy(hrxq->action); > > - else > > - mlx5_glue->destroy_flow_action(hrxq->action); > > -#endif > > priv->obj_ops.hrxq_destroy(hrxq); > > if (!hrxq->standalone) { > > mlx5_ind_table_obj_release(dev, hrxq->ind_table, > > -- > > 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action 2022-09-26 19:35 ` Slava Ovsiienko @ 2022-09-29 1:51 ` wangyunjian 0 siblings, 0 replies; 7+ messages in thread From: wangyunjian @ 2022-09-29 1:51 UTC (permalink / raw) To: Slava Ovsiienko, dev Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable I agree with you. Can you fix it? Thanks Yunjian > -----Original Message----- > From: Slava Ovsiienko [mailto:viacheslavo@nvidia.com] > Sent: Tuesday, September 27, 2022 3:36 AM > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org > Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh > <rasland@nvidia.com>; Dmitry Kozlyuk <dkozlyuk@nvidia.com>; > Huangshaozhang <huangshaozhang@huawei.com>; stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when > releasing a drop action > > Hi, Yunjian > > We have drop action in mlx5. > To create/destroy drop action we have > mlx5_drop_action_create/ mlx5_drop_action_destroy common routines. > > As PMD supports operation either over FW (Verbs in rdma_core) or SW/HW > steering with DevX objects the "virtual" methods are used for objects: > > priv->obj_ops.drop_action_create(dev) - create drop action > a) mlx5_ibv_drop_action_create()- use Verbs (dv-flow_en == 0) > b) mlx5_devx_drop_action_create() - use DevX (dv-flow_en != 0) > > priv->obj_ops.drop_action_destroy(dev) - destroy drop action > a) mlx5_ibv_drop_action_destroy()- use Verbs (dv-flow_en == 0) > b) mlx5_devx_drop_action_destroy() - use DevX (dv-flow_en != 0) > > > Let's consider DevX case. > mlx5_devx_drop_action_create() > mlx5_rxq_devx_obj_drop_create() > mlx5_devx_ind_table_new() > mlx5_devx_hrxq_new() > xxx_create_dest_tir() > xxx_action_create_dest_tir() > > > mlx5_devx_drop_action_destroy() > /* It seems action destroy is missing here */ > mlx5_devx_tir_destroy() > mlx5_devx_ind_table_destroy() > mlx5_rxq_devx_obj_drop_release() > > So, yes, I agree. There is missing action destroy, it seems we have leakage. > > Let's look at > __mlx5_hrxq_remove() > destroy_action() > priv->obj_ops.hrxq_destroy(); > > > And there we have a problem. > obj_ops.hrxq_destroy() - not always mlx5_devx_tir_destroy() is called. > It might be mlx5_ibv_qp_destroy() (for Verbas) and patch would not work. > > So, instead of fixing __mlx5_hrxq_remove() and mlx5_devx_tir_destroy() > I would consider patching: > - mlx5_devx_drop_action_destroy() > - mlx5_ibv_drop_action_destroy > by adding action desatroying. > > With best regards, > Slava > > > > -----Original Message----- > > From: wangyunjian <wangyunjian@huawei.com> > > Sent: Friday, September 23, 2022 12:32 > > To: dev@dpdk.org > > Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh > <rasland@nvidia.com>; > > Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk > > <dkozlyuk@nvidia.com>; Huangshaozhang > <huangshaozhang@huawei.com>; > > stable@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when > > releasing a drop action > > > > Friendly ping. > > > > > -----Original Message----- > > > From: wangyunjian > > > Sent: Tuesday, August 23, 2022 2:46 PM > > > To: dev@dpdk.org > > > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com; > > > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>; > > > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org > > > Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when > > > releasing a drop action > > > > > > Currently, the resources for hrxq->action are allocated in > > > mlx5_devx_hrxq_new(). But it was not being freed when the drop action > > > was released in mlx5_devx_drop_action_destroy(). > > > So, fix is to free the resources in mlx5_devx_tir_destroy(). > > > > > > Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > > --- > > > drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c > > > | > > > 6 ------ > > > 2 files changed, 7 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/net/mlx5/mlx5_devx.c > > > b/drivers/net/mlx5/mlx5_devx.c index > > > 6886ae1f22..09c8856f05 100644 > > > --- a/drivers/net/mlx5/mlx5_devx.c > > > +++ b/drivers/net/mlx5/mlx5_devx.c > > > @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev, > > > struct mlx5_hrxq *hrxq, static void mlx5_devx_tir_destroy(struct > > > mlx5_hrxq *hrxq) { > > > +#if defined(HAVE_IBV_FLOW_DV_SUPPORT) > > > || !defined(HAVE_INFINIBAND_VERBS_H) > > > + if (hrxq->hws_flags) > > > + mlx5dr_action_destroy(hrxq->action); > > > + else > > > + mlx5_flow_os_destroy_flow_action(hrxq->action); > > > + hrxq->action = NULL; > > > +#endif > > > claim_zero(mlx5_devx_cmd_destroy(hrxq->tir)); > > > } > > > > > > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c > > > index eaf23d0df4..e518fe9bfd 100644 > > > --- a/drivers/net/mlx5/mlx5_rxq.c > > > +++ b/drivers/net/mlx5/mlx5_rxq.c > > > @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev, > > > struct mlx5_hrxq *hrxq) { > > > struct mlx5_priv *priv = dev->data->dev_private; > > > > > > -#ifdef HAVE_IBV_FLOW_DV_SUPPORT > > > - if (hrxq->hws_flags) > > > - mlx5dr_action_destroy(hrxq->action); > > > - else > > > - mlx5_glue->destroy_flow_action(hrxq->action); > > > -#endif > > > priv->obj_ops.hrxq_destroy(hrxq); > > > if (!hrxq->standalone) { > > > mlx5_ind_table_obj_release(dev, hrxq->ind_table, > > > -- > > > 2.27.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-29 1:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1661223500.git.wangyunjian@huawei.com> 2022-08-23 6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang 2022-09-23 9:31 ` wangyunjian 2022-09-26 12:30 ` Slava Ovsiienko 2022-08-23 6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang 2022-09-23 9:31 ` wangyunjian 2022-09-26 19:35 ` Slava Ovsiienko 2022-09-29 1:51 ` wangyunjian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).