Re: [PATCH v2] drm/v3d: release CPU job resources on pre-init failures

[Guangshuo Li]

Hi Maíra,

On Wed, 15 Jul 2026 at 21:37, Maíra Canal <mcanal@xxxxxxxxxx> 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 <lgs201920130244@xxxxxxxxx>
> > ---
> > 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