[PATCH net] net/mlx5e: Hold sync_lock while resetting VXLAN ports

[PATCH net] net/mlx5e: Hold sync_lock while resetting VXLAN ports

[Changyul Lee]

mlx5_vxlan_reset_to_default() walks vxlan->htable without sync_lock
and calls mlx5_vxlan_del_port() for each user port. mlx5e_nic_disable()
and mlx5e_uplink_rep_disable() call it after rtnl_unlock(), so the
udp_tunnel_nic work can run mlx5e_vxlan_unset_port() at the same time.
If that removes and frees the entry saved as the next one in
hash_for_each_safe(), the walk then reads freed memory.

Hold sync_lock over the whole walk, and add __mlx5_vxlan_del_port() for
callers that already hold it.

Fixes: c5eb51adf06b ("net/mlx5e: Fix VXLAN synchronization after function reload")
Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---
Found with a static analysis tool we are developing.

Compile-tested only, with x86_64_defconfig plus MLX5_CORE_EN and W=1.

 .../ethernet/mellanox/mlx5/core/lib/vxlan.c   | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
index c2292cab4592..b953bbc4925c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
@@ -121,25 +121,29 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
 	return 0;
 }
 
-int mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
+static int __mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
 {
 	struct mlx5_vxlan_port *vxlanp;
-	int ret = 0;
 
-	mutex_lock(&vxlan->sync_lock);
+	lockdep_assert_held(&vxlan->sync_lock);
 
 	vxlanp = vxlan_lookup_port(vxlan, port);
-	if (WARN_ON(!vxlanp)) {
-		ret = -ENOENT;
-		goto out_unlock;
-	}
+	if (WARN_ON(!vxlanp))
+		return -ENOENT;
 
 	hash_del_rcu(&vxlanp->hlist);
 	synchronize_rcu();
 	mlx5_vxlan_core_del_port_cmd(vxlan->mdev, port);
 	kfree(vxlanp);
+	return 0;
+}
+
+int mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
+{
+	int ret;
 
-out_unlock:
+	mutex_lock(&vxlan->sync_lock);
+	ret = __mlx5_vxlan_del_port(vxlan, port);
 	mutex_unlock(&vxlan->sync_lock);
 	return ret;
 }
@@ -185,12 +189,14 @@ void mlx5_vxlan_reset_to_default(struct mlx5_vxlan *vxlan)
 	if (!mlx5_vxlan_allowed(vxlan))
 		return;
 
+	mutex_lock(&vxlan->sync_lock);
 	hash_for_each_safe(vxlan->htable, bkt, tmp, vxlanp, hlist) {
 		/* Don't delete default UDP port added by the HW.
 		 * Remove only user configured ports
 		 */
 		if (vxlanp->udp_port == IANA_VXLAN_UDP_PORT)
 			continue;
-		mlx5_vxlan_del_port(vxlan, vxlanp->udp_port);
+		__mlx5_vxlan_del_port(vxlan, vxlanp->udp_port);
 	}
+	mutex_unlock(&vxlan->sync_lock);
 }

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.43.0

Re: [PATCH net] net/mlx5e: Hold sync_lock while resetting VXLAN ports

[sashiko-bot]

> mlx5_vxlan_reset_to_default() walks vxlan->htable without sync_lock
> and calls mlx5_vxlan_del_port() for each user port. mlx5e_nic_disable()
> and mlx5e_uplink_rep_disable() call it after rtnl_unlock(), so the
> udp_tunnel_nic work can run mlx5e_vxlan_unset_port() at the same time.
> If that removes and frees the entry saved as the next one in
> hash_for_each_safe(), the walk then reads freed memory.
> 
> Hold sync_lock over the whole walk, and add __mlx5_vxlan_del_port() for
> callers that already hold it.
> 
> Fixes: c5eb51adf06b ("net/mlx5e: Fix VXLAN synchronization after function reload")
> Signed-off-by: Changyul Lee <lcy8047@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113040323.2090829-1-lcy8047@gmail.com?part=1