[PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()

[PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()

[Brajesh Gupta]

Call sequence of double call:
pvr_context_destroy
  pvr_context_kill_queues
    pvr_queue_kill
      drm_sched_entity_destroy
        drm_sched_entity_fini // here
  pvr_context_put
    kref_put(..., pvr_context_release)
      pvr_context_destroy_queues
        pvr_queue_destroy
          drm_sched_entity_fini // here

Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
drm_sched_entity_flush() + drm_sched_entity_fini().
drm_sched_entity_flush() ensures all pending jobs are completed and
drm_sched_entity_fini() ensures no further submission is allowed as
per expectation from pvr_context_kill_queues(). Double call to
drm_sched_entity_fini() is misuse of the API so keep call only in
pvr_context_create() failure path.

Stack trace for issue with addition of refcounting for DRM entity
stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):
[  789.490527] ------------[ cut here ]------------
[  789.490559] refcount_t: underflow; use-after-free.
[  789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
[  789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
[  789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
[  789.490809] Hardware name: Texas Instruments AM625 SK (DT)
[  789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
[  789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  789.490876] pc : refcount_warn_saturate+0xf4/0x144
[  789.490884] lr : refcount_warn_saturate+0xf4/0x144
[  789.490892] sp : ffff8000822cbcc0
[  789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
[  789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
[  789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
[  789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
[  789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
[  789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
[  789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
[  789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
[  789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
[  789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
[  789.491027] Call trace:
[  789.491032]  refcount_warn_saturate+0xf4/0x144 (P)
[  789.491043]  drm_sched_entity_fini+0x164/0x18c [gpu_sched]
[  789.491081]  pvr_queue_destroy+0x64/0x134 [powervr]
[  789.491110]  pvr_context_destroy_queues+0x34/0x64 [powervr]
[  789.491138]  pvr_context_release+0x70/0xac [powervr]
[  789.491166]  pvr_context_put.part.0+0x5c/0x7c [powervr]
[  789.491193]  pvr_context_put+0x14/0x24 [powervr]
[  789.491221]  pvr_queue_fence_release_work+0x20/0x38 [powervr]
[  789.491249]  process_one_work+0x160/0x4c4
[  789.491264]  worker_thread+0x188/0x310
[  789.491276]  kthread+0x130/0x13c
[  789.491287]  ret_from_fork+0x10/0x20
[  789.491300] ---[ end trace 0000000000000000 ]---

Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
Cc: stable@vger.kernel.org
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
Changes in v6:
- Fix variable name in pvr_queue.h as per v5.
- Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com

Changes in v5:
- Update description of the issue and added stable tag.
- Modified variable name to align with behaviour.
- Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com

Changes in v4:
- Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
- Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com

Changes in v3:
- Fixed a typo.
- Handled missing memory leak for RENDER_CONTEXT.
- Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com

Changes in v2:
- Fixed memory leak identified in following error path handling of pvr_context_create():
- pvr_context_create()
-   ...
-   err_destroy_queues:
-     pvr_context_destroy_queues()
-       pvr_queue_destroy()
- Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
---
 drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
 drivers/gpu/drm/imagination/pvr_queue.c   |  6 ++++--
 drivers/gpu/drm/imagination/pvr_queue.h   |  2 +-
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
index eba4694400b5..b6f9e078315d 100644
--- a/drivers/gpu/drm/imagination/pvr_context.c
+++ b/drivers/gpu/drm/imagination/pvr_context.c
@@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
 /**
  * pvr_context_destroy_queues() - Destroy all queues attached to a context.
  * @ctx: Context to destroy queues on.
+ * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
+ *                      creation failure path.
  *
  * Should be called when the last reference to a context object is dropped.
  * It releases all resources attached to the queues bound to this context.
  */
-static void pvr_context_destroy_queues(struct pvr_context *ctx)
+static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
 {
 	switch (ctx->type) {
 	case DRM_PVR_CTX_TYPE_RENDER:
-		pvr_queue_destroy(ctx->queues.fragment);
-		pvr_queue_destroy(ctx->queues.geometry);
+		pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
+		pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
 		break;
 	case DRM_PVR_CTX_TYPE_COMPUTE:
-		pvr_queue_destroy(ctx->queues.compute);
+		pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
 		break;
 	case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
-		pvr_queue_destroy(ctx->queues.transfer);
+		pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
 		break;
 	}
 }
@@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
 	return -EINVAL;
 
 err_destroy_queues:
-	pvr_context_destroy_queues(ctx);
+	pvr_context_destroy_queues(ctx, true);
 	return err;
 }
 
@@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
 	pvr_fw_object_destroy(ctx->fw_obj);
 
 err_destroy_queues:
-	pvr_context_destroy_queues(ctx);
+	pvr_context_destroy_queues(ctx, true);
 
 err_free_ctx_id:
 	/*
@@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
 	spin_unlock(&pvr_dev->ctx_list_lock);
 
 	xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
-	pvr_context_destroy_queues(ctx);
+	pvr_context_destroy_queues(ctx, false);
 	pvr_fw_object_destroy(ctx->fw_obj);
 	kfree(ctx->data);
 	pvr_vm_context_put(ctx->vm_ctx);
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 7ed60e1c1a86..941c017399fc 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
 /**
  * pvr_queue_destroy() - Destroy a queue.
  * @queue: The queue to destroy.
+ * @cleanup_queue_entity: Whether to cleanup the queue entity.
  *
  * Cleanup the queue and free the resources attached to it. Should be
  * called from the context release function.
  */
-void pvr_queue_destroy(struct pvr_queue *queue)
+void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
 {
 	if (!queue)
 		return;
@@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
 	mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
 
 	drm_sched_fini(&queue->scheduler);
-	drm_sched_entity_fini(&queue->entity);
+	if (cleanup_queue_entity)
+		drm_sched_entity_fini(&queue->entity);
 
 	if (WARN_ON(queue->last_queued_job_scheduled_fence))
 		dma_fence_put(queue->last_queued_job_scheduled_fence);
diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
index 4aa72665ce25..149cc6d124bf 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.h
+++ b/drivers/gpu/drm/imagination/pvr_queue.h
@@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
 
 void pvr_queue_kill(struct pvr_queue *queue);
 
-void pvr_queue_destroy(struct pvr_queue *queue);
+void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
 
 void pvr_queue_process(struct pvr_queue *queue);
 

---
base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
change-id: 20260610-b4-sched_fix-ac3b920f475b

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>

Re: [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A TOCTOU race condition between pvr_submit_jobs() and pvr_context_destroy() allows jobs to be pushed to a destroyed scheduler entity, permanently leaking the job, the context, and any associated BOs/fences.
--

commit e2ffb23e3856239fee51854e5b7240ef10f40010
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>

drm/imagination: Fix double call to drm_sched_entity_fini()

This commit addresses a use-after-free underflow caused by a double call to
drm_sched_entity_fini(). It updates the teardown paths to only clean up the
queue entity during a context creation failure. This ensures tearing down
contexts does not erroneously finalize the scheduler entity multiple times.

> diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> --- a/drivers/gpu/drm/imagination/pvr_context.c
> +++ b/drivers/gpu/drm/imagination/pvr_context.c
[ ... ]
> @@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
>  	spin_unlock(&pvr_dev->ctx_list_lock);
>  
>  	xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> -	pvr_context_destroy_queues(ctx);
> +	pvr_context_destroy_queues(ctx, false);

[Severity: High]
This isn't a bug introduced by this patch, but is there a pre-existing race
condition between pvr_submit_jobs() and context destruction?

If unprivileged userspace calls DRM_IOCTL_PVR_SUBMIT_JOBS and
DRM_IOCTL_PVR_DESTROY_CONTEXT concurrently on the same context:

1. pvr_submit_jobs() in drivers/gpu/drm/imagination/pvr_job.c acquires the
context reference via pvr_context_lookup() but no synchronization locks are
held across the job submission process.

2. Concurrently, pvr_context_destroy() in
drivers/gpu/drm/imagination/pvr_context.c removes the context handle and
calls pvr_context_kill_queues(), stopping the scheduler entity.

3. pvr_submit_jobs() proceeds and calls drm_sched_entity_push_job().

Since the entity is already stopped, drm_sched_rq_add_entity() returns NULL
and does not wake up the scheduler. Does this mean the job and the context
(along with associated BOs and fences) are permanently leaked since the job
never executes and never drops its context reference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-b4-sched_fix-v6-1-afd66a9cabf5@imgtec.com?part=1

Re: [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()

[Alessio Belle]

Hi Brajesh,

On Tue, 2026-06-30 at 15:33 +0530, Brajesh Gupta wrote:
> Call sequence of double call:
> pvr_context_destroy
>   pvr_context_kill_queues
>     pvr_queue_kill
>       drm_sched_entity_destroy
>         drm_sched_entity_fini // here
>   pvr_context_put
>     kref_put(..., pvr_context_release)
>       pvr_context_destroy_queues
>         pvr_queue_destroy
>           drm_sched_entity_fini // here
> 
> Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
> drm_sched_entity_flush() + drm_sched_entity_fini().
> drm_sched_entity_flush() ensures all pending jobs are completed and
> drm_sched_entity_fini() ensures no further submission is allowed as
> per expectation from pvr_context_kill_queues(). Double call to
> drm_sched_entity_fini() is misuse of the API so keep call only in
> pvr_context_create() failure path.
> 
> Stack trace for issue with addition of refcounting for DRM entity
> stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):

Sorry late nit: looking at other kernel dumps in commit descriptions, there
should be an empty line here.

> [  789.490527] ------------[ cut here ]------------
> [  789.490559] refcount_t: underflow; use-after-free.
> [  789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
> [  789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
> [  789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
> [  789.490809] Hardware name: Texas Instruments AM625 SK (DT)
> [  789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
> [  789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [  789.490876] pc : refcount_warn_saturate+0xf4/0x144
> [  789.490884] lr : refcount_warn_saturate+0xf4/0x144
> [  789.490892] sp : ffff8000822cbcc0
> [  789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
> [  789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
> [  789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
> [  789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
> [  789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
> [  789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
> [  789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
> [  789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
> [  789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
> [  789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
> [  789.491027] Call trace:
> [  789.491032]  refcount_warn_saturate+0xf4/0x144 (P)
> [  789.491043]  drm_sched_entity_fini+0x164/0x18c [gpu_sched]
> [  789.491081]  pvr_queue_destroy+0x64/0x134 [powervr]
> [  789.491110]  pvr_context_destroy_queues+0x34/0x64 [powervr]
> [  789.491138]  pvr_context_release+0x70/0xac [powervr]
> [  789.491166]  pvr_context_put.part.0+0x5c/0x7c [powervr]
> [  789.491193]  pvr_context_put+0x14/0x24 [powervr]
> [  789.491221]  pvr_queue_fence_release_work+0x20/0x38 [powervr]
> [  789.491249]  process_one_work+0x160/0x4c4
> [  789.491264]  worker_thread+0x188/0x310
> [  789.491276]  kthread+0x130/0x13c
> [  789.491287]  ret_from_fork+0x10/0x20
> [  789.491300] ---[ end trace 0000000000000000 ]---
> 
> Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> ---
> Changes in v6:
> - Fix variable name in pvr_queue.h as per v5.
> - Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com
> 
> Changes in v5:
> - Update description of the issue and added stable tag.
> - Modified variable name to align with behaviour.
> - Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com
> 
> Changes in v4:
> - Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
> - Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com
> 
> Changes in v3:
> - Fixed a typo.
> - Handled missing memory leak for RENDER_CONTEXT.
> - Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com
> 
> Changes in v2:
> - Fixed memory leak identified in following error path handling of pvr_context_create():
> - pvr_context_create()
> -   ...
> -   err_destroy_queues:
> -     pvr_context_destroy_queues()
> -       pvr_queue_destroy()
> - Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
> ---
>  drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
>  drivers/gpu/drm/imagination/pvr_queue.c   |  6 ++++--
>  drivers/gpu/drm/imagination/pvr_queue.h   |  2 +-
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> index eba4694400b5..b6f9e078315d 100644
> --- a/drivers/gpu/drm/imagination/pvr_context.c
> +++ b/drivers/gpu/drm/imagination/pvr_context.c
> @@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
>  /**
>   * pvr_context_destroy_queues() - Destroy all queues attached to a context.
>   * @ctx: Context to destroy queues on.
> + * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
> + *                      creation failure path.

nit: Could you align the second line of this comment?

With these sorted, in v7 you can add to the end of the trailers:

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

>   *
>   * Should be called when the last reference to a context object is dropped.
>   * It releases all resources attached to the queues bound to this context.
>   */
> -static void pvr_context_destroy_queues(struct pvr_context *ctx)
> +static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
>  {
>  	switch (ctx->type) {
>  	case DRM_PVR_CTX_TYPE_RENDER:
> -		pvr_queue_destroy(ctx->queues.fragment);
> -		pvr_queue_destroy(ctx->queues.geometry);
> +		pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
> +		pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
>  		break;
>  	case DRM_PVR_CTX_TYPE_COMPUTE:
> -		pvr_queue_destroy(ctx->queues.compute);
> +		pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
>  		break;
>  	case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
> -		pvr_queue_destroy(ctx->queues.transfer);
> +		pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
>  		break;
>  	}
>  }
> @@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
>  	return -EINVAL;
>  
>  err_destroy_queues:
> -	pvr_context_destroy_queues(ctx);
> +	pvr_context_destroy_queues(ctx, true);
>  	return err;
>  }
>  
> @@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
>  	pvr_fw_object_destroy(ctx->fw_obj);
>  
>  err_destroy_queues:
> -	pvr_context_destroy_queues(ctx);
> +	pvr_context_destroy_queues(ctx, true);
>  
>  err_free_ctx_id:
>  	/*
> @@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
>  	spin_unlock(&pvr_dev->ctx_list_lock);
>  
>  	xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> -	pvr_context_destroy_queues(ctx);
> +	pvr_context_destroy_queues(ctx, false);
>  	pvr_fw_object_destroy(ctx->fw_obj);
>  	kfree(ctx->data);
>  	pvr_vm_context_put(ctx->vm_ctx);
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index 7ed60e1c1a86..941c017399fc 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> @@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
>  /**
>   * pvr_queue_destroy() - Destroy a queue.
>   * @queue: The queue to destroy.
> + * @cleanup_queue_entity: Whether to cleanup the queue entity.
>   *
>   * Cleanup the queue and free the resources attached to it. Should be
>   * called from the context release function.
>   */
> -void pvr_queue_destroy(struct pvr_queue *queue)
> +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
>  {
>  	if (!queue)
>  		return;
> @@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
>  	mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
>  
>  	drm_sched_fini(&queue->scheduler);
> -	drm_sched_entity_fini(&queue->entity);
> +	if (cleanup_queue_entity)
> +		drm_sched_entity_fini(&queue->entity);
>  
>  	if (WARN_ON(queue->last_queued_job_scheduled_fence))
>  		dma_fence_put(queue->last_queued_job_scheduled_fence);
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
> index 4aa72665ce25..149cc6d124bf 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.h
> +++ b/drivers/gpu/drm/imagination/pvr_queue.h
> @@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
>  
>  void pvr_queue_kill(struct pvr_queue *queue);
>  
> -void pvr_queue_destroy(struct pvr_queue *queue);
> +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
>  
>  void pvr_queue_process(struct pvr_queue *queue);
>  
> 
> ---
> base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
> change-id: 20260610-b4-sched_fix-ac3b920f475b
> 
> Best regards,

Re: [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()

[Brajesh Gupta]

On Tue, 2026-06-30 at 13:53 +0000, Alessio Belle wrote:
Hi Alessio,
> *** NOTE: This is an internal email from Imagination Technologies ***
> 
> 
> 
> 
> Hi Brajesh,
> 
> On Tue, 2026-06-30 at 15:33 +0530, Brajesh Gupta wrote:
> > Call sequence of double call:
> > pvr_context_destroy
> >   pvr_context_kill_queues
> >     pvr_queue_kill
> >       drm_sched_entity_destroy
> >         drm_sched_entity_fini // here
> >   pvr_context_put
> >     kref_put(..., pvr_context_release)
> >       pvr_context_destroy_queues
> >         pvr_queue_destroy
> >           drm_sched_entity_fini // here
> > 
> > Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
> > drm_sched_entity_flush() + drm_sched_entity_fini().
> > drm_sched_entity_flush() ensures all pending jobs are completed and
> > drm_sched_entity_fini() ensures no further submission is allowed as
> > per expectation from pvr_context_kill_queues(). Double call to
> > drm_sched_entity_fini() is misuse of the API so keep call only in
> > pvr_context_create() failure path.
> > 
> > Stack trace for issue with addition of refcounting for DRM entity
> > stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):
> 
> Sorry late nit: looking at other kernel dumps in commit descriptions, there
> should be an empty line here.
Fixed.

> 
> > [  789.490527] ------------[ cut here ]------------
> > [  789.490559] refcount_t: underflow; use-after-free.
> > [  789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
> > [  789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
> > [  789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
> > [  789.490809] Hardware name: Texas Instruments AM625 SK (DT)
> > [  789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
> > [  789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [  789.490876] pc : refcount_warn_saturate+0xf4/0x144
> > [  789.490884] lr : refcount_warn_saturate+0xf4/0x144
> > [  789.490892] sp : ffff8000822cbcc0
> > [  789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
> > [  789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
> > [  789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
> > [  789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
> > [  789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
> > [  789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
> > [  789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
> > [  789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
> > [  789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
> > [  789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
> > [  789.491027] Call trace:
> > [  789.491032]  refcount_warn_saturate+0xf4/0x144 (P)
> > [  789.491043]  drm_sched_entity_fini+0x164/0x18c [gpu_sched]
> > [  789.491081]  pvr_queue_destroy+0x64/0x134 [powervr]
> > [  789.491110]  pvr_context_destroy_queues+0x34/0x64 [powervr]
> > [  789.491138]  pvr_context_release+0x70/0xac [powervr]
> > [  789.491166]  pvr_context_put.part.0+0x5c/0x7c [powervr]
> > [  789.491193]  pvr_context_put+0x14/0x24 [powervr]
> > [  789.491221]  pvr_queue_fence_release_work+0x20/0x38 [powervr]
> > [  789.491249]  process_one_work+0x160/0x4c4
> > [  789.491264]  worker_thread+0x188/0x310
> > [  789.491276]  kthread+0x130/0x13c
> > [  789.491287]  ret_from_fork+0x10/0x20
> > [  789.491300] ---[ end trace 0000000000000000 ]---
> > 
> > Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> > ---
> > Changes in v6:
> > - Fix variable name in pvr_queue.h as per v5.
> > - Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com
> > 
> > Changes in v5:
> > - Update description of the issue and added stable tag.
> > - Modified variable name to align with behaviour.
> > - Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com
> > 
> > Changes in v4:
> > - Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
> > - Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com
> > 
> > Changes in v3:
> > - Fixed a typo.
> > - Handled missing memory leak for RENDER_CONTEXT.
> > - Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com
> > 
> > Changes in v2:
> > - Fixed memory leak identified in following error path handling of pvr_context_create():
> > - pvr_context_create()
> > -   ...
> > -   err_destroy_queues:
> > -     pvr_context_destroy_queues()
> > -       pvr_queue_destroy()
> > - Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
> > ---
> >  drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
> >  drivers/gpu/drm/imagination/pvr_queue.c   |  6 ++++--
> >  drivers/gpu/drm/imagination/pvr_queue.h   |  2 +-
> >  3 files changed, 15 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> > index eba4694400b5..b6f9e078315d 100644
> > --- a/drivers/gpu/drm/imagination/pvr_context.c
> > +++ b/drivers/gpu/drm/imagination/pvr_context.c
> > @@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
> >  /**
> >   * pvr_context_destroy_queues() - Destroy all queues attached to a context.
> >   * @ctx: Context to destroy queues on.
> > + * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
> > + *                      creation failure path.
> 
> nit: Could you align the second line of this comment?
Fixed

> 
> With these sorted, in v7 you can add to the end of the trailers:
> 
> Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>
Done

Thanks,
Brajesh
> 
> Thanks,
> Alessio
> 
> >   *
> >   * Should be called when the last reference to a context object is dropped.
> >   * It releases all resources attached to the queues bound to this context.
> >   */
> > -static void pvr_context_destroy_queues(struct pvr_context *ctx)
> > +static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
> >  {
> >       switch (ctx->type) {
> >       case DRM_PVR_CTX_TYPE_RENDER:
> > -             pvr_queue_destroy(ctx->queues.fragment);
> > -             pvr_queue_destroy(ctx->queues.geometry);
> > +             pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
> > +             pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
> >               break;
> >       case DRM_PVR_CTX_TYPE_COMPUTE:
> > -             pvr_queue_destroy(ctx->queues.compute);
> > +             pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
> >               break;
> >       case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
> > -             pvr_queue_destroy(ctx->queues.transfer);
> > +             pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
> >               break;
> >       }
> >  }
> > @@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
> >       return -EINVAL;
> > 
> >  err_destroy_queues:
> > -     pvr_context_destroy_queues(ctx);
> > +     pvr_context_destroy_queues(ctx, true);
> >       return err;
> >  }
> > 
> > @@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
> >       pvr_fw_object_destroy(ctx->fw_obj);
> > 
> >  err_destroy_queues:
> > -     pvr_context_destroy_queues(ctx);
> > +     pvr_context_destroy_queues(ctx, true);
> > 
> >  err_free_ctx_id:
> >       /*
> > @@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
> >       spin_unlock(&pvr_dev->ctx_list_lock);
> > 
> >       xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> > -     pvr_context_destroy_queues(ctx);
> > +     pvr_context_destroy_queues(ctx, false);
> >       pvr_fw_object_destroy(ctx->fw_obj);
> >       kfree(ctx->data);
> >       pvr_vm_context_put(ctx->vm_ctx);
> > diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> > index 7ed60e1c1a86..941c017399fc 100644
> > --- a/drivers/gpu/drm/imagination/pvr_queue.c
> > +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> > @@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
> >  /**
> >   * pvr_queue_destroy() - Destroy a queue.
> >   * @queue: The queue to destroy.
> > + * @cleanup_queue_entity: Whether to cleanup the queue entity.
> >   *
> >   * Cleanup the queue and free the resources attached to it. Should be
> >   * called from the context release function.
> >   */
> > -void pvr_queue_destroy(struct pvr_queue *queue)
> > +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
> >  {
> >       if (!queue)
> >               return;
> > @@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
> >       mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
> > 
> >       drm_sched_fini(&queue->scheduler);
> > -     drm_sched_entity_fini(&queue->entity);
> > +     if (cleanup_queue_entity)
> > +             drm_sched_entity_fini(&queue->entity);
> > 
> >       if (WARN_ON(queue->last_queued_job_scheduled_fence))
> >               dma_fence_put(queue->last_queued_job_scheduled_fence);
> > diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
> > index 4aa72665ce25..149cc6d124bf 100644
> > --- a/drivers/gpu/drm/imagination/pvr_queue.h
> > +++ b/drivers/gpu/drm/imagination/pvr_queue.h
> > @@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
> > 
> >  void pvr_queue_kill(struct pvr_queue *queue);
> > 
> > -void pvr_queue_destroy(struct pvr_queue *queue);
> > +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
> > 
> >  void pvr_queue_process(struct pvr_queue *queue);
> > 
> > 
> > ---
> > base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
> > change-id: 20260610-b4-sched_fix-ac3b920f475b
> > 
> > Best regards,
>