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

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

Pre-existing issues:
- [High] mlx5_st_dealloc_index leaks the idx_data struct when erasing it from the xarray.
--

--- Patch [5]: [PATCH v13 5/5] RDMA/mlx5: get tph for p2p access when registering dma-buf mr ---
commit cb1e1bfb29faeb4a7f3d550176e3590d73486a3e
Author: Zhiping Zhang <[email protected]>

RDMA/mlx5: get tph for p2p access when registering dma-buf mr

This commit wires up mlx5_ib to query the exporter's chosen PCIe TLP
Processing Hint (TPH) Steering Tag at MR registration time for peer-to-peer
DMA access. It translates the exporter-provided steering tags into local ST
entries or uses default routing if no hint is available.

> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 00e13028762ae..fdd7d94bd4aa4 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -167,6 +168,61 @@ static int get_unchangeable_access_flags(struct mlx5_ib_dev *dev,
>  #define MLX5_FRMR_POOLS_KERNEL_KEY_PH_MASK GENMASK_ULL(23, 16)
>  #define MLX5_FRMR_POOLS_KERNEL_KEY_ST_INDEX_MASK GENMASK_ULL(15, 0)
>  
> +static u8 mlx5_ib_tph_key_ph(u8 ph)
> +{
> +	if (ph == MLX5_IB_NO_PH || ph == 0)
> +		ph ^= MLX5_IB_NO_PH;
> +
> +	return ph;
> +}
> +
> +static int mlx5_ib_get_st_handle_ref(struct mlx5_ib_dev *dev, u16 st_index)
> +{
> +	if (st_index == MLX5_MKC_PCIE_TPH_NO_STEERING_TAG_INDEX)
> +		return 0;
> +
> +	return mlx5_st_get_index(dev->mdev, st_index);
> +}
> +
> +static void mlx5_ib_put_st_index_ref(struct mlx5_ib_dev *dev, u16 st_index)
> +{
> +	if (st_index == MLX5_MKC_PCIE_TPH_NO_STEERING_TAG_INDEX)
> +		return;
> +
> +	mlx5_st_dealloc_index(dev->mdev, st_index);

[Severity: High]
This is a pre-existing issue, but this patch introduces a new, heavily-exercised
reachable path for it.

Does mlx5_st_dealloc_index() leak the mlx5_st_idx_data struct when erasing it
from the xarray?

When a steering tag's reference count drops to zero, the entry is removed via
xa_erase():

drivers/net/ethernet/mellanox/mlx5/core/lib/st.c:mlx5_st_dealloc_index() {
    ...
	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 */
	}
    ...
}

However, the underlying idx_data pointer is not freed with kfree(idx_data).

Because the next time the same tag is requested, a new struct is allocated via
kzalloc_obj() in mlx5_st_alloc_index_by_tag(), won't repeated allocations and
deallocations lead to an unbounded kernel memory leak?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5