Re: [PATCH mlx5-next 07/10] RDMA/mlx5: Extract IB specific lock out of data direct

[Leon Romanovsky <leon@kernel.org>]

On Fri, Sep 18, 2026 at 11:16:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] core: missing smp_store_release() in lockless pointer publication
> --
> 
> commit 70f41e46e61426ca24948348bb11ea96b2dc863f
> Author: Dragos Tatulea <dtatulea@nvidia.com>
> 
> RDMA/mlx5: Extract IB specific lock out of data direct
> 
> The data_direct_lock from struct mlx5_ib_dev is an IB specific lock.
> It protects from unbind while mlx5_data_direct_dev is being accessed.
> So move data_direct_lock out of the generic bind/unbind functions
> and into the IB specific notifier callback.
> 
> > diff --git a/drivers/infiniband/hw/mlx5/data_direct.c b/drivers/infiniband/hw/mlx5/data_direct.c
> > index f871e3ce9f73d..16afacdd36018 100644
> > --- a/drivers/infiniband/hw/mlx5/data_direct.c
> > +++ b/drivers/infiniband/hw/mlx5/data_direct.c
> > @@ -197,9 +197,7 @@ mlx5_data_direct_get_reg(struct mlx5_ib_dev *ibdev)
> >  static void mlx5_data_direct_bind(struct mlx5_ib_dev *ibdev,
> >  				  struct mlx5_data_direct_dev *dev)
> >  {
> > -	mutex_lock(&ibdev->data_direct_lock);
> > -	ibdev->data_direct_dev = dev;
> > -	mutex_unlock(&ibdev->data_direct_lock);
> > +	WRITE_ONCE(ibdev->data_direct_dev, dev);
> 
> [Severity: High]
> Is a release barrier needed here?
> 
> By removing the shared lock, this becomes a lockless publication.
> mutex_lock() only provides ACQUIRE semantics, so the writer lacks a RELEASE
> barrier like smp_store_release().
> 
> Could weakly-ordered CPUs reorder the initialization of dev to become
> visible after the pointer publication?  This might allow a concurrent
> lockless reader to observe the newly assigned pointer but read uninitialized
> structure fields (e.g., dev->pdev being NULL), leading to a kernel panic
> when dereferenced.
> 
> [ ... ]
> > diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> > index e6b74955d95dc..1fc61a680f8af 100644
> > --- a/drivers/infiniband/hw/mlx5/mr.c
> > +++ b/drivers/infiniband/hw/mlx5/mr.c
> > @@ -997,7 +997,7 @@ reg_user_mr_dmabuf_by_data_direct(struct ib_pd *pd, u64 offset,
> >  		return ERR_PTR(-EOPNOTSUPP);
> >  
> >  	mutex_lock(&dev->data_direct_lock);
> > -	data_direct_dev = dev->data_direct_dev;
> > +	data_direct_dev = READ_ONCE(dev->data_direct_dev);
> 
> [Severity: High]
> Is an acquire barrier like smp_load_acquire() needed here instead of
> READ_ONCE()?
> 
> Since the reader in reg_user_mr_dmabuf_by_data_direct() takes a different
> lock (dev->data_direct_lock) than the writer (which takes
> mlx5_data_direct_mutex), it loads the pointer locklessly with respect to the
> writer.
> 
> Could an unprivileged userspace issuing an RDMA memory registration
> concurrently with a data direct device hotplug or bind observe uninitialized
> memory fields because of this missing barrier?

At least for bind, no. The ibdev device shouldn't be operable at that
stage yet.

Thanks

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260917111422.4054862-1-tariqt@nvidia.com?part=7
>