[PATCH rdma-next] RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep

[PATCH rdma-next] RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep

[Edward Srouji]

From: Shay Drory <shayd@nvidia.com>

mlx5_ib_set_vport_rep() attaches the representor netdev to the IB
device, which takes a netdev reference, and then adds the LAG demux
rule. If mlx5_lag_demux_rule_add() fails, the error is returned
without detaching the netdev. The eswitch does not call unload() for a
representor whose load() failed, so the reference is never dropped and
a later unregister_netdevice() of the representor waits forever while
holding the eswitch representors lock.

Similarly, if ib_device_set_netdev() fails, the rep pointers assigned at
the start of the function are left dangling.

Undo the netdev attach and clear the rep pointers on failure.

Fixes: d6c9b4de8109 ("{net/RDMA}/mlx5: Add LAG demux table API and vport demux rules")
Fixes: 8d159eb2117b ("RDMA/mlx5: Use IB set_netdev and get_netdev functions")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Chiara Meiohas <cmeiohas@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Edward Srouji <edwards@nvidia.com>
---
 drivers/infiniband/hw/mlx5/ib_rep.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
index 65d8767d1830c630f5aa2a2a693110e2c9bc9e48..9cbd27063ba8ff780446d9be2801012492383734 100644
--- a/drivers/infiniband/hw/mlx5/ib_rep.c
+++ b/drivers/infiniband/hw/mlx5/ib_rep.c
@@ -28,7 +28,7 @@ mlx5_ib_set_vport_rep(struct mlx5_core_dev *dev,
 
 	ret = ib_device_set_netdev(&ibdev->ib_dev, ndev, vport_index + 1);
 	if (ret)
-		return ret;
+		goto err_set_netdev;
 
 	/* Only Vports that are not native to the LAG master eswitch need to add
 	 * demux rule.
@@ -36,7 +36,18 @@ mlx5_ib_set_vport_rep(struct mlx5_core_dev *dev,
 	if (mlx5_eswitch_get_total_vports(dev) > vport_index)
 		return 0;
 
-	return mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
+	ret = mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
+	if (ret)
+		goto err_demux;
+
+	return 0;
+
+err_demux:
+	ib_device_set_netdev(&ibdev->ib_dev, NULL, vport_index + 1);
+err_set_netdev:
+	rep->rep_data[REP_IB].priv = NULL;
+	ibdev->port[vport_index].rep = NULL;
+	return ret;
 }
 
 static void mlx5_ib_register_peer_vport_reps(struct mlx5_core_dev *mdev);

---
base-commit: 978361dd2ccff357d4fe6aac6ce057d1bfae8312
change-id: 20261015-rep-netdev-leak-93431d272bbb

Best regards,
-- 
Edward Srouji <edwards@nvidia.com>

Re: [PATCH rdma-next] RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rdma/mlx5: TOCTOU race in port representor cleanup leading to NULL deref
--

commit 8e906a48ae2be9e11d0c09a6021af6c819f14449
Author: Shay Drory <shayd@nvidia.com>

RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep

mlx5_ib_set_vport_rep() attaches the representor netdev to the IB
device, which takes a netdev reference, and then adds the LAG demux
rule. Undo the netdev attach and clear the rep pointers on failure.

> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
> index 65d8767d1830c..9cbd27063ba8f 100644
> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
> @@ -36,7 +36,18 @@ mlx5_ib_set_vport_rep(struct mlx5_core_dev *dev,
>  	if (mlx5_eswitch_get_total_vports(dev) > vport_index)
>  		return 0;
>  
> -	return mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
> +	ret = mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
> +	if (ret)
> +		goto err_demux;
> +
> +	return 0;
> +
> +err_demux:
> +	ib_device_set_netdev(&ibdev->ib_dev, NULL, vport_index + 1);
> +err_set_netdev:
> +	rep->rep_data[REP_IB].priv = NULL;
> +	ibdev->port[vport_index].rep = NULL;

[Severity: High]
Can this lockless clearing of the pointer cause a NULL pointer dereference
if it races with concurrent readers?

In drivers/infiniband/hw/mlx5/counters.c:mlx5_ib_query_q_counters_vport(),
the rep pointer is checked and then dereferenced:

	if (!dev->port[port_num].rep ||
	    dev->port[port_num].rep->vport == MLX5_VPORT_UPLINK)
		return 0;

	mdev = mlx5_eswitch_get_core_dev(dev->port[port_num].rep->esw);

Since there is no READ_ONCE() or local caching of the pointer on the
read side, and no synchronization with this error path, can the compiler
reload the pointer from memory after the safety check?

If the pointer is cleared by the error path here between the check and
the dereference in mlx5_ib_query_q_counters_vport(), wouldn't this
result in a NULL pointer dereference?

> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rep-netdev-leak-v1-1-5110013806ae@nvidia.com?part=1

Re: [PATCH rdma-next] RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep

[Edward Srouji]

On 10/15/2026 2:55 PM, sashiko-bot@kernel.org wrote:
> External email: Use caution opening links or attachments
> 
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] rdma/mlx5: TOCTOU race in port representor cleanup leading to NULL deref
> --
> 
> commit 8e906a48ae2be9e11d0c09a6021af6c819f14449
> Author: Shay Drory <shayd@nvidia.com>
> 
> RDMA/mlx5: Add missing error flow in mlx5_ib_set_vport_rep
> 
> mlx5_ib_set_vport_rep() attaches the representor netdev to the IB
> device, which takes a netdev reference, and then adds the LAG demux
> rule. Undo the netdev attach and clear the rep pointers on failure.
> 
>> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
>> index 65d8767d1830c..9cbd27063ba8f 100644
>> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
>> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
>> @@ -36,7 +36,18 @@ mlx5_ib_set_vport_rep(struct mlx5_core_dev *dev,
>>        if (mlx5_eswitch_get_total_vports(dev) > vport_index)
>>                return 0;
>>
>> -     return mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
>> +     ret = mlx5_lag_demux_rule_add(rep_dev, rep->vport, vport_index);
>> +     if (ret)
>> +             goto err_demux;
>> +
>> +     return 0;
>> +
>> +err_demux:
>> +     ib_device_set_netdev(&ibdev->ib_dev, NULL, vport_index + 1);
>> +err_set_netdev:
>> +     rep->rep_data[REP_IB].priv = NULL;
>> +     ibdev->port[vport_index].rep = NULL;
> 
> [Severity: High]
> Can this lockless clearing of the pointer cause a NULL pointer dereference
> if it races with concurrent readers?
> 
> In drivers/infiniband/hw/mlx5/counters.c:mlx5_ib_query_q_counters_vport(),
> the rep pointer is checked and then dereferenced:
> 
>          if (!dev->port[port_num].rep ||
>              dev->port[port_num].rep->vport == MLX5_VPORT_UPLINK)
>                  return 0;
> 
>          mdev = mlx5_eswitch_get_core_dev(dev->port[port_num].rep->esw);
> 
> Since there is no READ_ONCE() or local caching of the pointer on the
> read side, and no synchronization with this error path, can the compiler
> reload the pointer from memory after the safety check?
> 
> If the pointer is cleared by the error path here between the check and
> the dereference in mlx5_ib_query_q_counters_vport(), wouldn't this
> result in a NULL pointer dereference?
> 
>> +     return ret;
>>   }
Essentially, Sashiko is correct.
But the same pattern already exist, with a wider racy window, in 
mlx5_ib_vport_rep_unload().
I let the AI to look for readers that have similar patterns, and it 
found in multiple different places.
Specifically here, the function can be reordered to publish the reps at 
the end of it, but it's a behavioral change and need to consider what it 
affects.>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rep-netdev-leak-v1-1-5110013806ae@nvidia.com?part=1