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

[sashiko-bot@kernel.org]

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