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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Resource leak (memory and drm_syncobj references) when parsing chained user-space extensions.
--

commit 42106e7ee90a922dbfb05df35206e0f77fc80751
Author: Maíra Canal <mcanal@igalia.com>

drm/v3d: Move out_sync syncobj lookup to submission

The commit moves the out_sync sync object lookup to the submission phase,
failing with -ENOENT if userspace passes a non-zero handle that doesn't
refer to a valid syncobj. It also consolidates the cleanup routines on
error paths for both single-sync and multi-sync implementations.

> 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
[ ... ]
> @@ -1032,6 +1034,12 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
>  		}
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but can the direct returns when
parsing extensions cause a resource leak?

In v3d_submit_cl_ioctl(), if userspace provides a valid
DRM_V3D_EXT_ID_MULTI_SYNC extension followed by an invalid one (e.g., an
unknown ID), v3d_get_extensions() will successfully allocate se.out_syncs
and acquire drm_syncobj references for the first extension, but then fail
on the second extension and return an error code.

Because the code directly returns here instead of jumping to a cleanup label,
it bypasses v3d_submit_put_post_deps(), leaking both the memory and the
drm_syncobj references.

Could this be triggered repeatedly by userspace chaining a valid multi-sync
extension with an invalid one, potentially exhausting kernel memory and file
descriptors?

> +	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) {
>  		bin = (struct v3d_bin_job *) v3d_submit_add_job(&submit, V3D_BIN);
>  		if (IS_ERR(bin)) {
[ ... ]
> @@ -1137,6 +1146,12 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
>  		}
>  	}
>  

[Severity: High]
This is a pre-existing issue, but the same resource leak when
v3d_get_extensions() fails appears here in v3d_submit_tfu_ioctl() as well.

> +	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);
[ ... ]
> @@ -1234,6 +1250,12 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
>  		}
>  	}
>  

[Severity: High]
This is a pre-existing issue, but the same resource leak when
v3d_get_extensions() fails also appears here in v3d_submit_csd_ioctl().

> +	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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-v3d-sched-misc-fixes-v3-0-d7114bba55a0@igalia.com?part=13