Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues: - [Critical] Double-free vulnerability of query arrays introduced by new error handling. Pre-existing issues: - [Critical] Deterministic NULL pointer dereference in `drm_sched_job_cleanup()` when pre-init validation fails. - [High] Memory and syncobj leak in CL, CSD, and TFU ioctls on partial extension parsing failure. - [High] Memory and WW-Mutex lock leak of nested jobs for INDIRECT_CSD on early failures. -- --- Patch [1]: [PATCH v2] drm/v3d: release CPU job resources on pre-init failures --- commit e05d6340b909c6cf82de569c1c19fd20b04f9a1f Author: Guangshuo Li <[email protected]> Date: Mon Jul 13 16:05:31 2026 +0000 drm/v3d: release CPU job resources on pre-init failures The changes referenced by the Fixes tags moved CPU job resource cleanup to v3d_cpu_job_free(), the CPU job kref destructor. However, CPU job extensions are parsed before v3d_job_init() has successfully installed that destructor. Timestamp and performance query arrays, their syncobj references, and the indirect CSD GEM reference can therefore be acquired before the CPU job is managed by the kref cleanup path. If a later pre-init validation fails, or if v3d_job_init() itself fails, the CPU job can be cleaned up or deallocated without running v3d_cpu_job_free(). Those extension resources are then leaked. Move the CPU-job-owned resource release into a helper and call it both from the normal CPU job destructor and from the pre-init error paths. Fixes: b0fe80c0b925 ("drm/v3d: Fix use-after-free of CPU job query arrays on error path") Fixes: 6eb6e5acafa4 ("drm/v3d: Release indirect CSD GEM reference on CPU job free") Signed-off-by: Guangshuo Li <[email protected]> > diff --git a/drivers/gpu/drm/v3d/v3d_submit.c > b/drivers/gpu/drm/v3d/v3d_submit.c > index 1db43c6a078d5..7775fd4a9417b 100644 > --- a/drivers/gpu/drm/v3d/v3d_submit.c > +++ b/drivers/gpu/drm/v3d/v3d_submit.c [ ... ] > @@ -1311,6 +1316,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data, > ret = v3d_get_extensions(file_priv, args->extensions, &se, > cpu_job); > if (ret) { > drm_dbg(dev, "Failed to get extensions.\n"); > + v3d_cpu_job_free_resources(cpu_job); [Severity: Critical] Does this new error handling introduce a double-free? If v3d_get_cpu_timestamp_query_params() fails midway (for instance, if drm_syncobj_find() returns NULL), it internally cleans up by calling v3d_timestamp_query_info_free(). This frees the query array via kvfree() but leaves a dangling pointer in job->timestamp_query.queries since it isn't set to NULL. Because v3d_cpu_job_free_resources() is now called on the failure path here, it unconditionally calls v3d_timestamp_query_info_free() again. The dangling pointer evaluates to true, resulting in a second kvfree() on the same address. [Severity: High] This is a pre-existing issue, but does a failure in v3d_get_extensions() leak syncobjs in other ioctls? While v3d_submit_cpu_ioctl() uses goto fail to clean up the multisync dependencies via v3d_put_multisync_post_deps(&se), v3d_submit_cl_ioctl() and v3d_submit_csd_ioctl() use a direct return: if (args->flags & DRM_V3D_SUBMIT_EXTENSION) { ret = v3d_get_extensions(file_priv, args->extensions, &se, NULL); if (ret) { drm_dbg(dev, "Failed to get extensions.\n"); return ret; } } If a valid MULTI_SYNC extension is processed before an invalid extension, this direct return bypasses the cleanup, permanently leaking the allocated se.out_syncs array and the acquired syncobj references. > goto fail; [Severity: Critical] This is a pre-existing issue, but can this goto fail result in a NULL pointer dereference? If we jump to fail before v3d_job_init() is called, cpu_job has only been allocated (and zero-initialized). At the fail label, v3d_job_cleanup(cpu_job) is called, which directly calls drm_sched_job_cleanup(&job->base). Since the scheduler job wasn't initialized, job->s_fence is NULL, and drm_sched_job_cleanup() will crash when attempting to read the fence refcount via kref_read(&job->s_fence->finished.refcount). > } > } > > @@ -1318,12 +1324,14 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void > *data, > /* Every CPU job must have a CPU job user extension */ > if (!cpu_job->job_type) { > drm_dbg(dev, "CPU job must have a CPU job user extension.\n"); > + v3d_cpu_job_free_resources(cpu_job); > ret = -EINVAL; > goto fail; [Severity: High] This is a pre-existing issue, but does jumping to fail here leak memory and WW-Mutex locks for nested INDIRECT_CSD jobs? If v3d_get_extensions() successfully parsed an INDIRECT_CSD extension, it allocated cpu_job->indirect_csd.job and clean_job, and locked their BO reservations. Because the local variables csd_job and clean_job are only assigned later in this function, they remain NULL during this early goto fail. When the fail block calls v3d_job_cleanup() on them, it does nothing, bypassing fail_unreserve and permanently abandoning both the allocations and the locks. > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
