Hi Guangshuo, On 13/07/26 13:05, Guangshuo Li wrote:
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]> --- 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 | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c index 7682b24f13ec..2b1dc92d4709 100644 --- a/drivers/gpu/drm/v3d/v3d_submit.c +++ b/drivers/gpu/drm/v3d/v3d_submit.c @@ -127,11 +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 +137,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); }@@ -1313,6 +1318,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"); + v3d_cpu_job_free_resources(cpu_job);
You shouldn't call v3d_cpu_job_free_resources() if v3d_get_extensions() failed, as it handles the cleaning path internally. Best regards, - Maíra
