[PATCH v3] drm/v3d: release CPU job resources on pre-init failures

[Guangshuo Li]

The changes referenced by the Fixes tags moved CPU job resource cleanup
to v3d_cpu_job_free(), the CPU job kref destructor.

However, CPU job extensions are parsed before v3d_job_init() has
successfully installed that destructor. Timestamp and performance query
arrays, their syncobj references, and the indirect CSD GEM reference can
therefore be acquired before the CPU job is managed by the kref cleanup
path.

If a later pre-init validation fails, or if v3d_job_init() itself fails,
the CPU job can be cleaned up or deallocated without running
v3d_cpu_job_free(). Those extension resources are then leaked.

Move the CPU-job-owned resource release into a helper and call it both
from the normal CPU job destructor and from the pre-init error paths.

Fixes: b0fe80c0b925 ("drm/v3d: Fix use-after-free of CPU job query arrays on error path")
Fixes: 6eb6e5acafa4 ("drm/v3d: Release indirect CSD GEM reference on CPU job free")
Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
---
v3:
  - Do not call v3d_cpu_job_free_resources() after v3d_get_extensions()
    fails, since v3d_get_extensions() already handles the cleanup
    internally, as pointed out by Maíra Canal.

v2:
  - Rebase onto drm-misc-fixes.
  - Fold the indirect CSD GEM pre-init failure fix into this patch.
  - Drop unnecessary pointer and count resets after releasing resources,
    as suggested by Maíra Canal.

 drivers/gpu/drm/v3d/v3d_submit.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 7682b24f13ec..77e94c25b2af 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -127,10 +127,8 @@ v3d_render_job_free(struct kref *ref)
 }
 
 static void
-v3d_cpu_job_free(struct kref *ref)
+v3d_cpu_job_free_resources(struct v3d_cpu_job *job)
 {
-	struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job,
-					       base.refcount);
 
 	v3d_timestamp_query_info_free(&job->timestamp_query,
 				      job->timestamp_query.count);
@@ -140,7 +138,15 @@ v3d_cpu_job_free(struct kref *ref)
 
 	if (job->indirect_csd.indirect)
 		drm_gem_object_put(job->indirect_csd.indirect);
+}
+
+static void
+v3d_cpu_job_free(struct kref *ref)
+{
+	struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job,
+					       base.refcount);
 
+	v3d_cpu_job_free_resources(job);
 	v3d_job_free(ref);
 }
 
@@ -1320,12 +1326,14 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	/* Every CPU job must have a CPU job user extension */
 	if (!cpu_job->job_type) {
 		drm_dbg(dev, "CPU job must have a CPU job user extension.\n");
+		v3d_cpu_job_free_resources(cpu_job);
 		ret = -EINVAL;
 		goto fail;
 	}
 
 	if (args->bo_handle_count != cpu_job_bo_handle_count[cpu_job->job_type]) {
 		drm_dbg(dev, "This CPU job was not submitted with the proper number of BOs.\n");
+		v3d_cpu_job_free_resources(cpu_job);
 		ret = -EINVAL;
 		goto fail;
 	}
@@ -1335,6 +1343,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
 			   v3d_cpu_job_free, 0, &se, V3D_CPU);
 	if (ret) {
+		v3d_cpu_job_free_resources(cpu_job);
 		v3d_job_deallocate((void *)&cpu_job);
 		goto fail;
 	}
-- 
2.43.0