Re: [PATCH rdma-next v2] RDMA/irdma: Check MR contiguity before PBLE alloc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irdma: double free of PBLE on MR reregistration HW failure
--

commit 448b999143bf40a6d4795a1e901ceab0ac8c8c2d
Author: Jacob Moroni <jmoroni@google.com>

RDMA/irdma: Check MR contiguity before PBLE alloc

This commit checks if a memory region is contiguous before attempting to
allocate PBLEs. This prevents unnecessary PBLE allocation failures from
causing memory registration to fail when a direct mapping could have been
used instead.

> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 71d7ae7baa478..f89c8516c57c0 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c

[ ... ]

> @@ -3413,24 +3374,33 @@ static int irdma_reg_user_mr_type_mem(struct irdma_mr *iwmr, int access,
>  {
>  	struct irdma_device *iwdev = to_iwdev(iwmr->ibmr.device);
>  	struct irdma_pbl *iwpbl = &iwmr->iwpbl;
> +	struct ib_umem *region = iwmr->region;
>  	u32 stag = 0;
> -	u8 lvl;
>  	int err;
>  
> -	lvl = iwmr->page_cnt != 1 ? PBLE_LEVEL_1 | PBLE_LEVEL_2 : PBLE_LEVEL_0;
>  	iwmr->access = access;
>  
> -	err = irdma_setup_pbles(iwdev->rf, iwmr, lvl);
> -	if (err)
> -		return err;
> -
> -	if (lvl) {
> -		err = irdma_check_mr_contiguous(&iwpbl->pble_alloc,
> +	if (ib_umem_is_contiguous(region)) {
> +		/* HW needs a valid page size even without PBLEs, so use 4K. It
> +		 * behaves like a PBLE-backed region where the PBA must be page
> +		 * aligned. HW takes the page offset from the lower 12 VA bits.
> +		 */
> +		iwmr->page_size = SZ_4K;
> +		iwmr->pgaddrmem[0] = ALIGN_DOWN(ib_umem_start_dma_addr(region),
>  						iwmr->page_size);
> -		if (err) {
> -			irdma_free_pble(iwdev->rf->pble_rsrc, &iwpbl->pble_alloc);
> -			iwpbl->pbl_allocated = false;
> -		}
> +	} else {
> +		iwmr->page_size = ib_umem_find_best_pgsz(region,
> +				iwdev->rf->sc_dev.hw_attrs.page_size_cap,
> +				iwmr->ibmr.iova);
> +		if (unlikely(!iwmr->page_size))
> +			return -EOPNOTSUPP;
> +
> +		iwmr->page_cnt = ib_umem_num_dma_blocks(region,
> +							iwmr->page_size);
> +		err = irdma_setup_pbles(iwdev->rf, iwmr,
> +					PBLE_LEVEL_1 | PBLE_LEVEL_2);
> +		if (err)
> +			return err;
>  	}
>  
>  	if (create_stag) {
>  		stag = irdma_create_stag(iwdev);
>  		if (!stag) {
>  			err = -ENOMEM;
>  			goto free_pble;
>  		}
>  
>  		iwmr->stag = stag;
>  		iwmr->ibmr.rkey = stag;
>  		iwmr->ibmr.lkey = stag;
>  	}
>  
>  	err = irdma_hwreg_mr(iwdev, iwmr, access);
>  	if (err)
>  		goto err_hwreg;
>  
>  	return 0;
>  
>  err_hwreg:
>  	if (stag)
>  		irdma_free_stag(iwdev, stag);
>  
>  free_pble:
>  	if (iwpbl->pble_alloc.level != PBLE_LEVEL_0 && iwpbl->pbl_allocated)
>  		irdma_free_pble(iwdev->rf->pble_rsrc, &iwpbl->pble_alloc);

[Severity: High]
If irdma_hwreg_mr() fails during memory registration, the free_pble error
path frees the PBLE allocator resources but does not clear
iwpbl->pbl_allocated.

When this occurs during MR reregistration in irdma_rereg_mr_trans(), the
error propagates up to the RDMA core, which retains the original MR.

Upon subsequent destruction of the MR in irdma_dereg_mr(), the driver
checks this stale flag:

	if (iwpbl->pbl_allocated)
		irdma_free_pble(iwdev->rf->pble_rsrc, &iwpbl->pble_alloc);

Could this double free corrupt the driver's internal memory management data
structures (irdma_prm_return_pbles), leading to kernel memory corruption?

>  
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014162144.4053279-1-jmoroni@google.com?part=1