v3d_submit_process_post_deps() looks up the out_sync syncobj via drm_syncobj_find(), and if userspace passes a non-zero handle that doesn't refer to a valid syncobj, the lookup silently returns NULL and the post-deps step skips publishing the submission's last fence to it. The ioctl still returns success, leaving userspace to wait on a invalid syncobj.
Instead of silently ignoring an invalid non-zero out_sync, move the syncobj lookup to the submission and make it fail with -ENOENT up front, mirroring the syncobj validation already done for in_sync. Now, v3d_submit_process_post_deps() only does the fence replacement. Note that the lookup is skipped when the multi-sync extension is in use, since args->out_sync is unused in that case. To keep cleanup symmetric on error paths, convert the function v3d_put_multisync_post_deps() into a single function that releases the references that were acquired but never published for both single-sync and multi-sync. Suggested-by: Tvrtko Ursulin <[email protected]> Signed-off-by: Maíra Canal <[email protected]> --- drivers/gpu/drm/v3d/v3d_submit.c | 46 +++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c index 08a7fa9bc366..afe54e73a461 100644 --- a/drivers/gpu/drm/v3d/v3d_submit.c +++ b/drivers/gpu/drm/v3d/v3d_submit.c @@ -353,17 +353,15 @@ v3d_submit_cleanup_jobs(struct v3d_submit *submit) } static void -v3d_submit_process_post_deps(struct v3d_submit *submit, u32 out_sync, +v3d_submit_process_post_deps(struct v3d_submit *submit, struct drm_syncobj *sync_out, struct v3d_submit_ext *se) { bool has_multisync = se && (se->flags & DRM_V3D_EXT_ID_MULTI_SYNC); struct v3d_job *last_job = submit->jobs[submit->job_count - 1]; - struct drm_syncobj *sync_out; /* Update the return sync object for the job */ /* If it only supports a single signal semaphore*/ if (!has_multisync) { - sync_out = drm_syncobj_find(submit->file_priv, out_sync); if (sync_out) { drm_syncobj_replace_fence(sync_out, last_job->done_fence); drm_syncobj_put(sync_out); @@ -416,10 +414,13 @@ v3d_setup_csd_jobs_and_bos(struct v3d_submit *submit, } static void -v3d_put_multisync_post_deps(struct v3d_submit_ext *se) +v3d_submit_put_post_deps(struct drm_syncobj *sync_out, struct v3d_submit_ext *se) { unsigned int i; + if (sync_out) + drm_syncobj_put(sync_out); + if (!(se && se->out_sync_count)) return; @@ -978,6 +979,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, { struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv }; struct drm_v3d_submit_cl *args = data; + struct drm_syncobj *sync_out = NULL; struct v3d_submit_ext se = {0}; struct v3d_bin_job *bin = NULL; struct v3d_render_job *render = NULL; @@ -1004,6 +1006,12 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, } } + if (args->out_sync && !(se.flags & DRM_V3D_EXT_ID_MULTI_SYNC)) { + sync_out = drm_syncobj_find(file_priv, args->out_sync); + if (!sync_out) + return -ENOENT; + } + if (args->bcl_start != args->bcl_end) { ret = v3d_submit_add_job(&submit, (void **)&bin, sizeof(*bin), v3d_job_free, V3D_BIN); @@ -1063,7 +1071,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, if (ret) goto fail_unreserve; - v3d_submit_process_post_deps(&submit, args->out_sync, &se); + v3d_submit_process_post_deps(&submit, sync_out, &se); v3d_submit_put_jobs(&submit); return 0; @@ -1072,7 +1080,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, v3d_submit_unlock_reservations(&submit); fail: v3d_submit_cleanup_jobs(&submit); - v3d_put_multisync_post_deps(&se); + v3d_submit_put_post_deps(sync_out, &se); return ret; } @@ -1092,6 +1100,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, { struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv }; struct drm_v3d_submit_tfu *args = data; + struct drm_syncobj *sync_out = NULL; struct v3d_submit_ext se = {0}; struct v3d_tfu_job *job = NULL; int ret = 0; @@ -1111,6 +1120,12 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, } } + if (args->out_sync && !(se.flags & DRM_V3D_EXT_ID_MULTI_SYNC)) { + sync_out = drm_syncobj_find(file_priv, args->out_sync); + if (!sync_out) + return -ENOENT; + } + ret = v3d_submit_add_job(&submit, (void **)&job, sizeof(*job), v3d_job_free, V3D_TFU); if (ret) @@ -1155,7 +1170,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, if (ret) goto fail_unreserve; - v3d_submit_process_post_deps(&submit, args->out_sync, &se); + v3d_submit_process_post_deps(&submit, sync_out, &se); v3d_submit_put_jobs(&submit); return 0; @@ -1164,7 +1179,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data, v3d_submit_unlock_reservations(&submit); fail: v3d_submit_cleanup_jobs(&submit); - v3d_put_multisync_post_deps(&se); + v3d_submit_put_post_deps(sync_out, &se); return ret; } @@ -1184,6 +1199,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, { struct v3d_submit submit = { .v3d = to_v3d_dev(dev), .file_priv = file_priv }; struct drm_v3d_submit_csd *args = data; + struct drm_syncobj *sync_out = NULL; struct v3d_submit_ext se = {0}; int ret; @@ -1210,6 +1226,12 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, } } + if (args->out_sync && !(se.flags & DRM_V3D_EXT_ID_MULTI_SYNC)) { + sync_out = drm_syncobj_find(file_priv, args->out_sync); + if (!sync_out) + return -ENOENT; + } + ret = v3d_setup_csd_jobs_and_bos(&submit, args, &se); if (ret) goto fail; @@ -1222,7 +1244,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, if (ret) goto fail_unreserve; - v3d_submit_process_post_deps(&submit, args->out_sync, &se); + v3d_submit_process_post_deps(&submit, sync_out, &se); v3d_submit_put_jobs(&submit); return 0; @@ -1231,7 +1253,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data, v3d_submit_unlock_reservations(&submit); fail: v3d_submit_cleanup_jobs(&submit); - v3d_put_multisync_post_deps(&se); + v3d_submit_put_post_deps(sync_out, &se); return ret; } @@ -1330,7 +1352,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data, if (ret) goto fail_unreserve; - v3d_submit_process_post_deps(&submit, 0, &se); + v3d_submit_process_post_deps(&submit, NULL, &se); v3d_submit_put_jobs(&submit); return 0; @@ -1339,7 +1361,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data, v3d_submit_unlock_reservations(&submit); fail: v3d_submit_cleanup_jobs(&submit); - v3d_put_multisync_post_deps(&se); + v3d_submit_put_post_deps(NULL, &se); kvfree(cpu_job->timestamp_query.queries); kvfree(cpu_job->performance_query.queries); -- 2.54.0
