drm/v3d: Fix potential memory leak in the timestamp extension

If fetching of userspace memory fails during the main loop, all drm sync
objs looked up until that point will be leaked because of the missing
drm_syncobj_put.

Fix it by exporting and using a common cleanup helper.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the timestamp query job")
Cc: Maíra Canal <mcanal@igalia.com>
Cc: Iago Toral Quiroga <itoral@igalia.com>
Cc: stable@vger.kernel.org # v6.8+
Reviewed-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240711135340.84617-3-tursulin@igalia.com

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 099b962bdfde3a878bda56c509456457539c69ca..e208ffdfba32baa6aa214d19dbcb6dcc0d44c83f 100644 (file)
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -563,6 +563,8 @@ void v3d_mmu_insert_ptes(struct v3d_bo *bo);
 void v3d_mmu_remove_ptes(struct v3d_bo *bo);
 
 /* v3d_sched.c */
+void v3d_timestamp_query_info_free(struct v3d_timestamp_query_info *query_info,
+                                  unsigned int count);
 void v3d_job_update_stats(struct v3d_job *job, enum v3d_queue queue);
 int v3d_sched_init(struct v3d_dev *v3d);
 void v3d_sched_fini(struct v3d_dev *v3d);

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 03df37a3acf5c52bba205b186ec87954d5870e15..59dc0287dab9836a81c1e86457987ace6424aaca 100644 (file)
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -73,18 +73,28 @@ v3d_sched_job_free(struct drm_sched_job *sched_job)
        v3d_job_cleanup(job);
 }
 
+void
+v3d_timestamp_query_info_free(struct v3d_timestamp_query_info *query_info,
+                             unsigned int count)
+{
+       if (query_info->queries) {
+               unsigned int i;
+
+               for (i = 0; i < count; i++)
+                       drm_syncobj_put(query_info->queries[i].syncobj);
+
+               kvfree(query_info->queries);
+       }
+}
+
 static void
 v3d_cpu_job_free(struct drm_sched_job *sched_job)
 {
        struct v3d_cpu_job *job = to_cpu_job(sched_job);
-       struct v3d_timestamp_query_info *timestamp_query = &job->timestamp_query;
        struct v3d_performance_query_info *performance_query = &job->performance_query;
 
-       if (timestamp_query->queries) {
-               for (int i = 0; i < timestamp_query->count; i++)
-                       drm_syncobj_put(timestamp_query->queries[i].syncobj);
-               kvfree(timestamp_query->queries);
-       }
+       v3d_timestamp_query_info_free(&job->timestamp_query,
+                                     job->timestamp_query.count);
 
        if (performance_query->queries) {
                for (int i = 0; i < performance_query->count; i++)

diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 263fefc1d04ff7ce0c1de849085d667dc3fc43ad..121bf1314b80515ab4863426cce03d06d72c7065 100644 (file)
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -452,6 +452,8 @@ v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv,
 {
        u32 __user *offsets, *syncs;
        struct drm_v3d_timestamp_query timestamp;
+       unsigned int i;
+       int err;
 
        if (!job) {
                DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
@@ -480,19 +482,19 @@ v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv,
        offsets = u64_to_user_ptr(timestamp.offsets);
        syncs = u64_to_user_ptr(timestamp.syncs);
 
-       for (int i = 0; i < timestamp.count; i++) {
+       for (i = 0; i < timestamp.count; i++) {
                u32 offset, sync;
 
                if (copy_from_user(&offset, offsets++, sizeof(offset))) {
-                       kvfree(job->timestamp_query.queries);
-                       return -EFAULT;
+                       err = -EFAULT;
+                       goto error;
                }
 
                job->timestamp_query.queries[i].offset = offset;
 
                if (copy_from_user(&sync, syncs++, sizeof(sync))) {
-                       kvfree(job->timestamp_query.queries);
-                       return -EFAULT;
+                       err = -EFAULT;
+                       goto error;
                }
 
                job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);
@@ -500,6 +502,10 @@ v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv,
        job->timestamp_query.count = timestamp.count;
 
        return 0;
+
+error:
+       v3d_timestamp_query_info_free(&job->timestamp_query, i);
+       return err;
 }
 
 static int
@@ -509,6 +515,8 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv,
 {
        u32 __user *syncs;
        struct drm_v3d_reset_timestamp_query reset;
+       unsigned int i;
+       int err;
 
        if (!job) {
                DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
@@ -533,14 +541,14 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv,
 
        syncs = u64_to_user_ptr(reset.syncs);
 
-       for (int i = 0; i < reset.count; i++) {
+       for (i = 0; i < reset.count; i++) {
                u32 sync;
 
                job->timestamp_query.queries[i].offset = reset.offset + 8 * i;
 
                if (copy_from_user(&sync, syncs++, sizeof(sync))) {
-                       kvfree(job->timestamp_query.queries);
-                       return -EFAULT;
+                       err = -EFAULT;
+                       goto error;
                }
 
                job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);
@@ -548,6 +556,10 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv,
        job->timestamp_query.count = reset.count;
 
        return 0;
+
+error:
+       v3d_timestamp_query_info_free(&job->timestamp_query, i);
+       return err;
 }
 
 /* Get data for the copy timestamp query results job submission. */
@@ -558,7 +570,8 @@ v3d_get_cpu_copy_query_results_params(struct drm_file *file_priv,
 {
        u32 __user *offsets, *syncs;
        struct drm_v3d_copy_timestamp_query copy;
-       int i;
+       unsigned int i;
+       int err;
 
        if (!job) {
                DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
@@ -591,15 +604,15 @@ v3d_get_cpu_copy_query_results_params(struct drm_file *file_priv,
                u32 offset, sync;
 
                if (copy_from_user(&offset, offsets++, sizeof(offset))) {
-                       kvfree(job->timestamp_query.queries);
-                       return -EFAULT;
+                       err = -EFAULT;
+                       goto error;
                }
 
                job->timestamp_query.queries[i].offset = offset;
 
                if (copy_from_user(&sync, syncs++, sizeof(sync))) {
-                       kvfree(job->timestamp_query.queries);
-                       return -EFAULT;
+                       err = -EFAULT;
+                       goto error;
                }
 
                job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);
@@ -613,6 +626,10 @@ v3d_get_cpu_copy_query_results_params(struct drm_file *file_priv,
        job->copy.stride = copy.stride;
 
        return 0;
+
+error:
+       v3d_timestamp_query_info_free(&job->timestamp_query, i);
+       return err;
 }
 
 static int