Re: [PATCH v3 13/14] drm/v3d: Reject invalid out_sync handles in submit ioctls

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 03/06/2026 23:25, Maíra Canal wrote:
> 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 <tvrtko.ursulin@igalia.com>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>   drivers/gpu/drm/v3d/v3d_submit.c | 50 +++++++++++++++++++++++++++++-----------
>   1 file changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index 2beb99a25104..dc27770d85fd 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
> @@ -338,17 +338,15 @@ v3d_submit_attach_object_fences(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);

Is it worth adding an assert that sync_obj and has_multisync are 
mutually exclusive? It caught me up thinking what prevents a ref leak.

> @@ -381,7 +379,7 @@ v3d_push_job(struct v3d_job *job)
>   }
>   
>   static int
> -v3d_submit_jobs(struct v3d_submit *submit, u32 out_sync,
> +v3d_submit_jobs(struct v3d_submit *submit, struct drm_syncobj *sync_out,
>   		struct v3d_submit_ext *se)
>   {
>   	struct v3d_dev *v3d = submit->v3d;
> @@ -406,7 +404,7 @@ v3d_submit_jobs(struct v3d_submit *submit, u32 out_sync,
>   
>   	v3d_submit_attach_object_fences(submit);
>   	v3d_submit_unlock_reservations(submit);
> -	v3d_submit_process_post_deps(submit, out_sync, se);
> +	v3d_submit_process_post_deps(submit, sync_out, se);
>   
>   	v3d_submit_put_jobs(submit);
>   
> @@ -444,10 +442,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;
>   
> @@ -1006,6 +1007,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;
> @@ -1032,6 +1034,12 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>   		}
>   	}
>   
> +	if (args->out_sync && !(se.flags & DRM_V3D_EXT_ID_MULTI_SYNC)) {

Is it feasible to make sure out_sync is zero if multi-sync flag is set? 
Or we have to allow letting garbage in for backwards compatibility?

Either way:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

> +		sync_out = drm_syncobj_find(file_priv, args->out_sync);
> +		if (!sync_out)
> +			return -ENOENT;
> +	}
> +
>   	if (args->bcl_start != args->bcl_end) {
>   		bin = (struct v3d_bin_job *) v3d_submit_add_job(&submit, V3D_BIN);
>   		if (IS_ERR(bin)) {
> @@ -1088,7 +1096,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
> +	ret = v3d_submit_jobs(&submit, sync_out, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> @@ -1098,7 +1106,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;
>   }
> @@ -1118,6 +1126,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;
> @@ -1137,6 +1146,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;
> +	}
> +
>   	job = (struct v3d_tfu_job *) v3d_submit_add_job(&submit, V3D_TFU);
>   	if (IS_ERR(job)) {
>   		ret = PTR_ERR(job);
> @@ -1178,7 +1193,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
> +	ret = v3d_submit_jobs(&submit, sync_out, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> @@ -1188,7 +1203,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;
>   }
> @@ -1208,6 +1223,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;
>   
> @@ -1234,6 +1250,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;
> @@ -1246,7 +1268,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit, args->out_sync, &se);
> +	ret = v3d_submit_jobs(&submit, sync_out, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> @@ -1256,7 +1278,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;
>   }
> @@ -1354,7 +1376,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
>   	if (ret)
>   		goto fail;
>   
> -	ret = v3d_submit_jobs(&submit, 0, &se);
> +	ret = v3d_submit_jobs(&submit, NULL, &se);
>   	if (ret)
>   		goto fail_unreserve;
>   
> @@ -1364,7 +1386,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);
>   
>   	return ret;
>   }
>