[PATCH v1] net/mlx5e: IPoIB: Fix race in QPN to netdev mapping

[PATCH v1] net/mlx5e: IPoIB: Fix race in QPN to netdev mapping

[Binbin Deng]

The QPN-to-netdev hash table is protected by ht_lock, but the NAPI lookup
path traverses and dereferences nodes without taking that lock.  A child
interface teardown can therefore remove and free the node while the parent
RQ is still processing completions for that child:

CPU0                                    CPU1
mlx5i_pkey_get_netdev()
node = mlx5i_find_qpn_to_netdev_node()
                                        mlx5i_pkey_del_qpn()
                                        node = mlx5i_find_qpn_to_netdev_node()
                                        spin_lock_bh(&ht->ht_lock)
                                        hlist_del_init(&node->hlist)
                                        spin_unlock_bh(&ht->ht_lock)
                                        kfree(node)
read node->netdev or node->hlist.next

At that point CPU0 is reading freed memory.  The race is reachable when a
PKEY child interface goes down while its completions are still queued on
the shared parent RQ; this is the same lifetime case already described by
the RX path comment.  It can lead to a KASAN use-after-free, corruption of
the RX path, or a panic.

Take ht_lock around hash lookups, and keep the deletion lookup and unlink
in one critical section.  The NAPI path copies the netdev pointer while
the lock prevents the mapping node from being freed.

Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by deep code inspection. Due to a lack of hardware
infrastructure, the issue was not observed at runtime.

Fixes: 7e7f4780c340 ("net/mlx5e: IPoIB, Use hash-table to map between QPN to child netdev")
Assisted-by: Coccinelle
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
index 4120afd097c3..91b203c6d603 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
@@ -109,13 +109,14 @@ int mlx5i_pkey_del_qpn(struct net_device *netdev, u32 qpn)
 	struct mlx5i_pkey_qpn_ht *ht = ipriv->qpn_htbl;
 	struct qpn_to_netdev *node;
 
+	spin_lock_bh(&ht->ht_lock);
 	node = mlx5i_find_qpn_to_netdev_node(ht->buckets, qpn);
 	if (!node) {
 		mlx5_core_warn(epriv->mdev, "QPN to netdev delete from HT failed\n");
+		spin_unlock_bh(&ht->ht_lock);
 		return -EINVAL;
 	}
 
-	spin_lock_bh(&ht->ht_lock);
 	hlist_del_init(&node->hlist);
 	spin_unlock_bh(&ht->ht_lock);
 	kfree(node);
@@ -128,10 +129,12 @@ struct net_device *mlx5i_pkey_get_netdev(struct net_device *netdev, u32 qpn)
 	struct mlx5i_priv *ipriv = netdev_priv(netdev);
 	struct qpn_to_netdev *node;
 
+	spin_lock_bh(&ipriv->qpn_htbl->ht_lock);
 	node = mlx5i_find_qpn_to_netdev_node(ipriv->qpn_htbl->buckets, qpn);
 	if (!node)
 		return NULL;
-
+	spin_unlock_bh(&ipriv->qpn_htbl->ht_lock);
+
 	return node->netdev;
 }
 
-- 
2.43.0

Re: [PATCH v1] net/mlx5e: IPoIB: Fix race in QPN to netdev mapping

[Dragos Tatulea]

On 02.11.26 08:41, Binbin Deng wrote:
> The QPN-to-netdev hash table is protected by ht_lock, but the NAPI lookup
> path traverses and dereferences nodes without taking that lock.  A child
> interface teardown can therefore remove and free the node while the parent
> RQ is still processing completions for that child:
> 
> CPU0                                    CPU1
> mlx5i_pkey_get_netdev()
> node = mlx5i_find_qpn_to_netdev_node()
>                                         mlx5i_pkey_del_qpn()
>                                         node = mlx5i_find_qpn_to_netdev_node()
>                                         spin_lock_bh(&ht->ht_lock)
>                                         hlist_del_init(&node->hlist)
>                                         spin_unlock_bh(&ht->ht_lock)
>                                         kfree(node)
> read node->netdev or node->hlist.next
> 
> At that point CPU0 is reading freed memory.  The race is reachable when a
> PKEY child interface goes down while its completions are still queued on
> the shared parent RQ; this is the same lifetime case already described by
> the RX path comment.  It can lead to a KASAN use-after-free, corruption of
> the RX path, or a panic.
> 
> Take ht_lock around hash lookups, and keep the deletion lookup and unlink
> in one critical section.  The NAPI path copies the netdev pointer while
> the lock prevents the mapping node from being freed.
> 
> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by deep code inspection. Due to a lack of hardware
> infrastructure, the issue was not observed at runtime.
> 
> Fixes: 7e7f4780c340 ("net/mlx5e: IPoIB, Use hash-table to map between QPN to child netdev")
> Assisted-by: Coccinelle
> Signed-off-by: Binbin Deng <18983559317@163.com>
Already WIP by Prathamesh:
https://lore.kernel.org/all/20261031225749.14214-1-prathameshdeshpande7@gmail.com/

Thanks,
Dragos