Hi Maíra, On Wed, 15 Jul 2026 at 21:37, Maíra Canal <[email protected]> wrote: > > Hi Guangshuo, > > On 13/07/26 13:05, Guangshuo Li wrote: > > 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]> > > --- > > v2: > > - Rebase onto drm-misc-fixes. > > - Fold the indirect CSD GEM pre-init failure fix into this patch. > > - Drop unnecessary pointer and count resets after releasing resources, > > as suggested by Maíra Canal. > > > > drivers/gpu/drm/v3d/v3d_submit.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/v3d/v3d_submit.c > > b/drivers/gpu/drm/v3d/v3d_submit.c > > index 7682b24f13ec..2b1dc92d4709 100644 > > --- a/drivers/gpu/drm/v3d/v3d_submit.c > > +++ b/drivers/gpu/drm/v3d/v3d_submit.c > > @@ -127,11 +127,8 @@ v3d_render_job_free(struct kref *ref) > > } > > > > static void > > -v3d_cpu_job_free(struct kref *ref) > > +v3d_cpu_job_free_resources(struct v3d_cpu_job *job) > > { > > - struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job, > > - base.refcount); > > - > > v3d_timestamp_query_info_free(&job->timestamp_query, > > job->timestamp_query.count); > > > > @@ -140,7 +137,15 @@ v3d_cpu_job_free(struct kref *ref) > > > > if (job->indirect_csd.indirect) > > drm_gem_object_put(job->indirect_csd.indirect); > > +} > > + > > +static void > > +v3d_cpu_job_free(struct kref *ref) > > +{ > > + struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job, > > + base.refcount); > > > > + v3d_cpu_job_free_resources(job); > > v3d_job_free(ref); > > } > > > > @@ -1313,6 +1318,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); > > You shouldn't call v3d_cpu_job_free_resources() if v3d_get_extensions() > failed, as it handles the cleaning path internally. > > Best regards, > - Maíra
Thank you for the review. You are right that v3d_get_extensions() already handles the cleanup internally. I will remove the extra v3d_cpu_job_free_resources() call and send a v3. Best regards, Guangshuo
