[PATCH 19.11] net/mlx5: fix invalid memory access in port closing

[PATCH 19.11] net/mlx5: fix invalid memory access in port closing

[Michael Baum]

The shared IB device (sh) has per port data updated in port creation.
In port closing this port data is updated even when the SH still exist.

However, this updating is happened after SH has been released and for
last port it actually accesses to freed memory.

This patch updates the port data before SH releasing.

Fixes: 08c0b56cb304 ("net/mlx5: fix port event cleaning order")
Cc: michaelba@nvidia.com

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index ece8c5e3e1..efeeb8443b 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1405,6 +1405,12 @@ mlx5_dev_close(struct rte_eth_dev *dev)
 		close(priv->nl_socket_rdma);
 	if (priv->vmwa_context)
 		mlx5_vlan_vmwa_exit(priv->vmwa_context);
+	priv->sh->port[priv->ibv_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
+	/*
+	 * The interrupt handler port id must be reset before priv is reset
+	 * since 'mlx5_dev_interrupt_nl_cb' uses priv.
+	 */
+	rte_io_wmb();
 	/*
 	 * Free the shared context in last turn, because the cleanup
 	 * routines above may use some shared fields, like
@@ -1458,12 +1464,6 @@ mlx5_dev_close(struct rte_eth_dev *dev)
 		if (!c)
 			claim_zero(rte_eth_switch_domain_free(priv->domain_id));
 	}
-	priv->sh->port[priv->ibv_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
-	/*
-	 * The interrupt handler port id must be reset before priv is reset
-	 * since 'mlx5_dev_interrupt_nl_cb' uses priv.
-	 */
-	rte_io_wmb();
 	memset(priv, 0, sizeof(*priv));
 	priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
 	/*
-- 
2.25.1

Re: [PATCH 19.11] net/mlx5: fix invalid memory access in port closing

[Christian Ehrhardt]

On Thu, Nov 17, 2022 at 4:48 PM Michael Baum <michaelba@nvidia.com> wrote:
>
> The shared IB device (sh) has per port data updated in port creation.
> In port closing this port data is updated even when the SH still exist.
>
> However, this updating is happened after SH has been released and for
> last port it actually accesses to freed memory.
>
> This patch updates the port data before SH releasing.
>
> Fixes: 08c0b56cb304 ("net/mlx5: fix port event cleaning order")
> Cc: michaelba@nvidia.com

Applied, thanks for the ping in regard to this follow on fix, I'd have
missed it since it has no 19.11 in the subject.

> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index ece8c5e3e1..efeeb8443b 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -1405,6 +1405,12 @@ mlx5_dev_close(struct rte_eth_dev *dev)
>                 close(priv->nl_socket_rdma);
>         if (priv->vmwa_context)
>                 mlx5_vlan_vmwa_exit(priv->vmwa_context);
> +       priv->sh->port[priv->ibv_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
> +       /*
> +        * The interrupt handler port id must be reset before priv is reset
> +        * since 'mlx5_dev_interrupt_nl_cb' uses priv.
> +        */
> +       rte_io_wmb();
>         /*
>          * Free the shared context in last turn, because the cleanup
>          * routines above may use some shared fields, like
> @@ -1458,12 +1464,6 @@ mlx5_dev_close(struct rte_eth_dev *dev)
>                 if (!c)
>                         claim_zero(rte_eth_switch_domain_free(priv->domain_id));
>         }
> -       priv->sh->port[priv->ibv_port - 1].nl_ih_port_id = RTE_MAX_ETHPORTS;
> -       /*
> -        * The interrupt handler port id must be reset before priv is reset
> -        * since 'mlx5_dev_interrupt_nl_cb' uses priv.
> -        */
> -       rte_io_wmb();
>         memset(priv, 0, sizeof(*priv));
>         priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
>         /*
> --
> 2.25.1
>


-- 
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd