Re: [PATCH v6 5/5] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `mlx5r_create_mkeys` error rollback loop contains an off-by-one error. It leaks the successfully created hardware mkey at `handles[0]` and improperly attempts to destroy the uninitialized failed handle at `handles[i]`, which translates to destroying the reserved hardware mkey index 0.
--

commit 37ae7395b661b5e14306aec55bedd4ae96943e49
Author: Zhiping Zhang <zhipingz@meta.com>

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

Query dma-buf TPH metadata when registering a dma-buf MR for peer-to-
peer access and translate the returned steering tag into an mlx5 ST
index. Keep the DMAH path as the first priority and only fall back to
DMA-buf metadata when no DMAH is supplied.

[ ... ]

> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 254e6aa4ccafb..7b0a883e04048 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c

[ ... ]

> @@ -299,7 +331,8 @@ static int mlx5r_create_mkeys(struct ib_device *device, struct ib_frmr_key *key,
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes
in mlx5r_create_mkeys(), I noticed an off-by-one error in the error
rollback loop right above this return statement.

The error rollback loop is written as:

	if (err)
		for (; i > 0; i--)
			mlx5_core_destroy_mkey(dev->mdev, handles[i]);

Since the loop body operates on handles[i] and decrements afterward, doesn't
this terminate before i == 0, thereby skipping handles[0] and leaking the
first successfully created hardware mkey?

Additionally, is it safe to call mlx5_core_destroy_mkey() on the failed
handles[i] where the error occurred?

Because handles[i] only received an 8-bit variant via assign_mkey_variant()
and the firmware failed to populate its hardware index, its upper 24 bits
are zeroed. When mlx5_mkey_to_idx() extracts the upper 24 bits, it yields 0.

Could this cause the driver to inadvertently destroy mkey index 0, which is
a reserved hardware resource?

>  
> -static void mlx5r_destroy_mkeys(struct ib_device *device, u32 *handles,
> +static void mlx5r_destroy_mkeys(struct ib_device *device,
> +				const struct ib_frmr_key *key, u32 *handles,
>  				unsigned int count)
>  {

[ ... ]

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