Re: [PATCH] drm/msm/dsi: avoid pinning the command buffer on every transfer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] drm/msm/dsi: double free in msm_dsi_tx_buf_free() [1]
--

commit da2da22a4a147766651afbdf0137d05a0c119858
Author: Michael McAllister <me@michaelmcallister.org>

drm/msm/dsi: avoid pinning the command buffer on every transfer

This commit replaces msm_gem_get_and_pin_iova() with msm_gem_get_iova() in
dsi_dma_base_get_6g() so that fetching the DSI command buffer's DMA address
does not take an additional pin on every transfer.

> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d92659d80f172..64fc54dff4839 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1428,7 +1428,8 @@ int dsi_dma_base_get_6g(struct msm_dsi_host *msm_host, uint64_t *dma_base)
>  	if (!dma_base)
>  		return -EINVAL;
>  
> -	return msm_gem_get_and_pin_iova(msm_host->tx_gem_obj,
> +	/* The command buffer stays pinned from allocation until teardown. */
> +	return msm_gem_get_iova(msm_host->tx_gem_obj,
>  				priv->kms->vm, dma_base);
>  }

[Severity: Critical]
This problem wasn't introduced by this patch, but can msm_dsi_tx_buf_free()
double free msm_host->tx_buf across component unbind cycles [1]?

drivers/gpu/drm/msm/dsi/dsi_host.c:msm_dsi_tx_buf_free() {
	...
	if (msm_host->tx_gem_obj) {
		msm_gem_kernel_put(msm_host->tx_gem_obj, msm_host->vm);
		drm_gpuvm_put(msm_host->vm);
		msm_host->tx_gem_obj = NULL;
		msm_host->vm = NULL;
	}

	if (msm_host->tx_buf)
		dma_free_coherent(dev->dev, msm_host->tx_size, msm_host->tx_buf,
			msm_host->tx_buf_paddr);
}

msm_host persists across component bind and unbind cycles, and unlike
msm_host->tx_gem_obj, msm_host->tx_buf is not cleared after
dma_free_coherent().  If a later bind fails before reallocating the buffer,
the resulting unbind calls dma_free_coherent() on the stale msm_host->tx_buf
pointer a second time.

[1] https://sashiko.dev/#/bug/linux-6f5085d3-ef49-482c-86e4-a2438bc1b91f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179422752570.88190.13452031230666113805.msm-dsi-pin-leak@michaelmcallister.org?part=1