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 <[email protected]>
---
drivers/gpu/drm/v3d/v3d_submit.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 8ed95a203810..2cca9c49f30c 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -127,20 +127,28 @@ 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);
+ job->timestamp_query.queries = NULL;
+ job->timestamp_query.count = 0;
v3d_performance_query_info_free(&job->performance_query,
job->performance_query.count);
+ job->performance_query.queries = NULL;
+ job->performance_query.count = 0;
- if (job->indirect_csd.indirect)
+ if (job->indirect_csd.indirect) {
drm_gem_object_put(job->indirect_csd.indirect);
+ job->indirect_csd.indirect = NULL;
+ }
+}
+static void
+v3d_cpu_job_free(struct kref *ref)
+{
+ v3d_cpu_job_free_resources(job);
v3d_job_free(ref);
}
@@ -1318,10 +1326,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
ret = v3d_get_extensions(file_priv, args->extensions, &se,
cpu_job);
if (ret) {
drm_dbg(dev, "Failed to get extensions.\n");
- if (cpu_job->indirect_csd.indirect) {
-
drm_gem_object_put(cpu_job->indirect_csd.indirect);
- cpu_job->indirect_csd.indirect = NULL;
- }
+ v3d_cpu_job_free_resources(cpu_job);
goto fail;
}
}
@@ -1329,12 +1334,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;
}
@@ -1344,10 +1351,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) {
- if (cpu_job->indirect_csd.indirect) {
- drm_gem_object_put(cpu_job->indirect_csd.indirect);
- cpu_job->indirect_csd.indirect = NULL;
- }
+ v3d_cpu_job_free_resources(cpu_job);
v3d_job_deallocate((void *)&cpu_job);
goto fail;
}
--
2.43.0
