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 <[email protected]> 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/[email protected]?part=13
