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

[Edward Srouji <edwards@nvidia.com>]

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