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@gmail.com>
---
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);
 			goto fail;
 		}
 	}
@@ -1320,12 +1326,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;
 	}
 
 	if (args->bo_handle_count != cpu_job_bo_handle_count[cpu_job->job_type]) {
 		drm_dbg(dev, "This CPU job was not submitted with the proper number of BOs.\n");
+		v3d_cpu_job_free_resources(cpu_job);
 		ret = -EINVAL;
 		goto fail;
 	}
@@ -1335,6 +1343,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
 			   v3d_cpu_job_free, 0, &se, V3D_CPU);
 	if (ret) {
+		v3d_cpu_job_free_resources(cpu_job);
 		v3d_job_deallocate((void *)&cpu_job);
 		goto fail;
 	}
-- 
2.43.0

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@gmail.com>
> ---
> 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

Hi Maíra,

On Wed, 15 Jul 2026 at 21:37, Maíra Canal <mcanal@igalia.com> 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@gmail.com>
> > ---
> > 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