Skip to content

Commit

Permalink
drm/v3d: Fix potential memory leak in the performance extension
Browse files Browse the repository at this point in the history
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: bae7cb5 ("drm/v3d: Create a CPU job extension for the reset performance query job")
Cc: Maíra Canal <mcanal@igalia.com>
Cc: Iago Toral Quiroga <itoral@igalia.com>
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240711135340.84617-4-tursulin@igalia.com
(cherry picked from commit 484de39)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
  • Loading branch information
Tvrtko Ursulin authored and Thomas Zimmermann committed Jul 18, 2024
1 parent 0e50fcc commit 32df4ab
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 26 deletions.
2 changes: 2 additions & 0 deletions drivers/gpu/drm/v3d/v3d_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ 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_performance_query_info_free(struct v3d_performance_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);
Expand Down
22 changes: 16 additions & 6 deletions drivers/gpu/drm/v3d/v3d_sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,30 @@ v3d_timestamp_query_info_free(struct v3d_timestamp_query_info *query_info,
}
}

void
v3d_performance_query_info_free(struct v3d_performance_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_performance_query_info *performance_query = &job->performance_query;

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++)
drm_syncobj_put(performance_query->queries[i].syncobj);
kvfree(performance_query->queries);
}
v3d_performance_query_info_free(&job->performance_query,
job->performance_query.count);

v3d_job_cleanup(&job->base);
}
Expand Down
52 changes: 32 additions & 20 deletions drivers/gpu/drm/v3d/v3d_submit.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ v3d_get_cpu_reset_performance_params(struct drm_file *file_priv,
u32 __user *syncs;
u64 __user *kperfmon_ids;
struct drm_v3d_reset_performance_query reset;
unsigned int i, j;
int err;

if (!job) {
DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
Expand Down Expand Up @@ -668,39 +670,43 @@ v3d_get_cpu_reset_performance_params(struct drm_file *file_priv,
syncs = u64_to_user_ptr(reset.syncs);
kperfmon_ids = u64_to_user_ptr(reset.kperfmon_ids);

for (int i = 0; i < reset.count; i++) {
for (i = 0; i < reset.count; i++) {
u32 sync;
u64 ids;
u32 __user *ids_pointer;
u32 id;

if (copy_from_user(&sync, syncs++, sizeof(sync))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

job->performance_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);

if (copy_from_user(&ids, kperfmon_ids++, sizeof(ids))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

ids_pointer = u64_to_user_ptr(ids);

for (int j = 0; j < reset.nperfmons; j++) {
for (j = 0; j < reset.nperfmons; j++) {
if (copy_from_user(&id, ids_pointer++, sizeof(id))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

job->performance_query.queries[i].kperfmon_ids[j] = id;
}

job->performance_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);
}
job->performance_query.count = reset.count;
job->performance_query.nperfmons = reset.nperfmons;

return 0;

error:
v3d_performance_query_info_free(&job->performance_query, i);
return err;
}

static int
Expand All @@ -711,6 +717,8 @@ v3d_get_cpu_copy_performance_query_params(struct drm_file *file_priv,
u32 __user *syncs;
u64 __user *kperfmon_ids;
struct drm_v3d_copy_performance_query copy;
unsigned int i, j;
int err;

if (!job) {
DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
Expand Down Expand Up @@ -742,34 +750,34 @@ v3d_get_cpu_copy_performance_query_params(struct drm_file *file_priv,
syncs = u64_to_user_ptr(copy.syncs);
kperfmon_ids = u64_to_user_ptr(copy.kperfmon_ids);

for (int i = 0; i < copy.count; i++) {
for (i = 0; i < copy.count; i++) {
u32 sync;
u64 ids;
u32 __user *ids_pointer;
u32 id;

if (copy_from_user(&sync, syncs++, sizeof(sync))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

job->performance_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);

if (copy_from_user(&ids, kperfmon_ids++, sizeof(ids))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

ids_pointer = u64_to_user_ptr(ids);

for (int j = 0; j < copy.nperfmons; j++) {
for (j = 0; j < copy.nperfmons; j++) {
if (copy_from_user(&id, ids_pointer++, sizeof(id))) {
kvfree(job->performance_query.queries);
return -EFAULT;
err = -EFAULT;
goto error;
}

job->performance_query.queries[i].kperfmon_ids[j] = id;
}

job->performance_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync);
}
job->performance_query.count = copy.count;
job->performance_query.nperfmons = copy.nperfmons;
Expand All @@ -782,6 +790,10 @@ v3d_get_cpu_copy_performance_query_params(struct drm_file *file_priv,
job->copy.stride = copy.stride;

return 0;

error:
v3d_performance_query_info_free(&job->performance_query, i);
return err;
}

/* Whenever userspace sets ioctl extensions, v3d_get_extensions parses data
Expand Down

0 comments on commit 32df4ab

Please sign in to comment.