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

Wentao Liang posted 1 patch 1 week, 1 day ago
drivers/gpu/drm/v3d/v3d_submit.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
[PATCH] drm/v3d: Fix indirect GEM object reference leak in v3d_get_cpu_indirect_csd_params()
Posted by Wentao Liang 1 week, 1 day ago
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@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 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. */
-- 
2.34.1
Re: [PATCH] drm/v3d: Fix indirect GEM object reference leak in v3d_get_cpu_indirect_csd_params()
Posted by Maíra Canal 1 week ago
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@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   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. */