[PATCH net] net/mlx5: IPOIB, Fix QPN lookup race

[PATCH net] net/mlx5: IPOIB, Fix QPN lookup race

[Prathamesh Deshpande]

mlx5i_pkey_get_netdev() reads the QPN hash table without ht_lock,
while mlx5i_pkey_del_qpn() removes entries under that lock and then
frees them.

PKEY child interfaces share the parent's RX resources, so RX lookups
can overlap child removal and access a freed entry. A lookup for
another QPN in the same bucket can also be affected.

Hold ht_lock during lookup and save node->netdev before unlocking.

Fixes: 7e7f4780c340 ("net/mlx5e: IPoIB, Use hash-table to map between QPN to child netdev")
Signed-off-by: Prathamesh Deshpande <prathameshdeshpande7@gmail.com>
---
 .../ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c   | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 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..6c24ae8e7662 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib_vlan.c
@@ -126,13 +126,17 @@ int mlx5i_pkey_del_qpn(struct net_device *netdev, u32 qpn)
 struct net_device *mlx5i_pkey_get_netdev(struct net_device *netdev, u32 qpn)
 {
 	struct mlx5i_priv *ipriv = netdev_priv(netdev);
+	struct mlx5i_pkey_qpn_ht *ht = ipriv->qpn_htbl;
+	struct net_device *result = NULL;
 	struct qpn_to_netdev *node;
 
-	node = mlx5i_find_qpn_to_netdev_node(ipriv->qpn_htbl->buckets, qpn);
-	if (!node)
-		return NULL;
+	spin_lock_bh(&ht->ht_lock);
+	node = mlx5i_find_qpn_to_netdev_node(ht->buckets, qpn);
+	if (node)
+		result = node->netdev;
+	spin_unlock_bh(&ht->ht_lock);
 
-	return node->netdev;
+	return result;
 }
 
 static int mlx5i_pkey_open(struct net_device *netdev);
-- 
2.43.0

Re: [PATCH net] net/mlx5: IPOIB, Fix QPN lookup race

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net/mlx5: IPOIB, Fix QPN lookup race

[Dragos Tatulea]

On 30.10.26 01:22, Prathamesh Deshpande wrote:
> mlx5i_pkey_get_netdev() reads the QPN hash table without ht_lock,
> while mlx5i_pkey_del_qpn() removes entries under that lock and then
> frees them.
> 
> PKEY child interfaces share the parent's RX resources, so RX lookups
> can overlap child removal and access a freed entry. A lookup for
> another QPN in the same bucket can also be affected.
>
I don't think that all these cases can happen. But I can see how a
CQE completion on the parent can read a pointer from the hash
bucket that just got deleted.

Please improve the commit message.

> Hold ht_lock during lookup and save node->netdev before unlocking.
> 
Holding the spinlock in the datapath is not a good idea. AFAIU
using hlist RCU variants will be enough.

Thanks,
Dragos