Re: [PATCH] drm/v3d: Fix indirect GEM object reference leak in v3d_get_cpu_indirect_csd_params()

[Maíra Canal]

Hi Wentao,

Your patch doesn't apply neither to drm-misc-next nor to drm-misc-fixes.
I removed v3d_setup_csd_jobs_and_bos() from
v3d_get_cpu_indirect_csd_params() a couple of months ago.

Please, use drm-misc-next or drm-misc-fixes as the base of your next
series.

Best regards,
- Maíra

On 16/09/26 12:36, Wentao Liang wrote:
> If v3d_setup_csd_jobs_and_bos() fails, the reference taken by
> drm_gem_object_lookup() on info->indirect is never dropped: the caller
> only cleans up via v3d_job_cleanup(), but the CPU job refcount is not
> initialized until v3d_job_init() runs after the extensions succeed, so
> v3d_cpu_job_free() never fires.
>
> Release the GEM reference and clear the pointer when setting up the
> CSD jobs fails.
>
> Fixes: 18b8413b25b7 ("drm/v3d: Create a CPU job extension for a indirect CSD job")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> ---
>   drivers/gpu/drm/v3d/v3d_submit.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index 3ddd53b6f437..f021ab9a69b1 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
> @@ -465,6 +465,7 @@ v3d_get_cpu_indirect_csd_params(struct drm_file *file_priv,
>   	struct v3d_dev *v3d = v3d_priv->v3d;
>   	struct drm_v3d_indirect_csd indirect_csd;
>   	struct v3d_indirect_csd_info *info = &job->indirect_csd;
> +	int ret;
>   
>   	if (!v3d_validate_cpu_job(file_priv, job))
>   		return -EINVAL;
> @@ -485,9 +486,15 @@ v3d_get_cpu_indirect_csd_params(struct drm_file *file_priv,
>   
>   	info->indirect = drm_gem_object_lookup(file_priv, indirect_csd.indirect);
>   
> -	return v3d_setup_csd_jobs_and_bos(file_priv, v3d, &indirect_csd.submit,
> -					  &info->job, &info->clean_job,
> -					  NULL, &info->acquire_ctx);
> +	ret = v3d_setup_csd_jobs_and_bos(file_priv, v3d, &indirect_csd.submit,
> +					 &info->job, &info->clean_job,
> +					 NULL, &info->acquire_ctx);
> +	if (ret) {
> +		drm_gem_object_put(info->indirect);
> +		info->indirect = NULL;
> +	}
> +
> +	return ret;
>   }
>   
>   /* Get data for the query timestamp job submission. */