Re: [PATCH v11 4/4] RDMA/mlx5: get tph for p2p access when registering dma-buf mr

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Failure to re-query PCIe TPH metadata upon DMA-buf invalidation

Pre-existing issues:
- [High] Memory leak of struct mlx5_st_idx_data in mlx5_st_dealloc_index()
--

commit e39795c3bd224c81bae70b87815075c4489e3f9e
Author: Zhiping Zhang <zhipingz@meta.com>
Subject: RDMA/mlx5: get tph for p2p access when registering dma-buf mr

This commit allows the mlx5 importer to query a dma-buf exporter's chosen
PCIe TLP Processing Hint (TPH) Steering Tag. It updates MR registration to
use this metadata for p2p access and program requester-side TPH on the
outbound mkey, falling back to no-TPH routing if unavailable.

> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index e6b74955d95dc..55216ae637614 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
[ ... ]
> @@ -903,6 +947,52 @@ static struct dma_buf_attach_ops mlx5_ib_dmabuf_attach_ops = {
>  	.invalidate_mappings = mlx5_ib_dmabuf_invalidate_cb,
>  };
>  
> +static void get_pci_tph_mr_dmabuf(struct mlx5_ib_dev *dev, struct dma_buf *dmabuf,
> +				  u16 *st_index, u8 *ph)

[Severity: Medium]
When an unpinned DMA-buf MR is invalidated and then subsequently faulted back
in via pagefault_dmabuf_mr() in drivers/infiniband/hw/mlx5/odp.c, the driver
calls ib_umem_dmabuf_map_pages() to map the pages, but it doesn't seem to
re-query the TPH metadata using get_pci_tph_mr_dmabuf() or
dma_buf_get_pci_tph().

Since the exporter might move the memory and change the preferred TPH steering
tag, does the driver need to re-evaluate the TPH metadata upon invalidation
and re-mapping to ensure DMA traffic isn't sent to a stale or suboptimal
NUMA node?

> +{
> +	u16 local_st_index;
> +	u16 steering_tag;
[ ... ]
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> index 997be91f0a135..bf14a7a6df15e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
[ ... ]
> @@ -152,8 +147,46 @@ int mlx5_st_alloc_index(struct mlx5_core_dev *dev, enum tph_mem_type mem_type,
>  	mutex_unlock(&st->lock);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(mlx5_st_alloc_index_by_tag);
[ ... ]
> +EXPORT_SYMBOL_GPL(mlx5_st_get_index);
> +
>  int mlx5_st_dealloc_index(struct mlx5_core_dev *dev, u16 st_index)
>  {
>  	struct mlx5_st_idx_data *idx_data;

[Severity: High]
This is a pre-existing issue, but I noticed it while reviewing this patch
since this patch broadens its exposure.

In mlx5_st_dealloc_index(), when the use count reaches zero:

	if (refcount_dec_and_test(&idx_data->usecount)) {
		xa_erase(&st->idx_xa, st_index);
		/* We leave PCI config space as was before, no mkey will refer to it */
	}

Is there a memory leak here? The dynamically allocated idx_data is erased from
the xarray but does not appear to be passed to kfree().

Since unprivileged users can repeatedly create and destroy MRs, could this
lead to kernel memory exhaustion over time?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702181025.2694961-1-zhipingz@meta.com?part=4