[PATCH] drm/panthor: Fix unlocked pool->vm access in panthor_heap_create()

[PATCH] drm/panthor: Fix unlocked pool->vm access in panthor_heap_create()

[Jiwoo Kim]

panthor_heap_create() takes a reference on pool->vm under pool->lock,
because the pool can be destroyed concurrently, and the initial heap
chunks used to be allocated in that VM. Since commit c63c3bfdde26
("drm/panthor: Avoid sleep locking in the internal BO size path"),
panthor_alloc_heap_chunk() takes the pool instead and reads pool->vm
itself. panthor_heap_grow() calls it with pool->lock held, but
panthor_heap_create() does not, so pool->vm is read without the lock
for every initial chunk.

If TILER_HEAP_CREATE races with VM_DESTROY on the same VM,
panthor_vm_destroy() calls panthor_heap_pool_destroy(), which sets
pool->vm to NULL, and panthor_kernel_bo_create() can then be called
with a NULL VM, which hits its drm_WARN_ON(!vm) and fails with
-EINVAL.

Pass the VM to panthor_alloc_heap_chunk() and
panthor_alloc_heap_chunks() again: panthor_heap_create() passes the VM
it holds a reference on, and panthor_heap_grow() passes pool->vm, which
it reads under pool->lock. In the race above, the initial chunks are
then created in the destroyed but still referenced VM, and
panthor_heap_create() frees them and fails with -EINVAL when it finds
the pool destroyed under pool->lock, as it did before that commit.

The problem was found through AI-assisted source code review. The fix
is compile-tested only and there is no reproducer, as I don't have
Mali CSF hardware.

Fixes: c63c3bfdde26 ("drm/panthor: Avoid sleep locking in the internal BO size path")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Jiwoo Kim <arpe1618@gmail.com>
---

Notes:
    This is the follow-up mentioned in "[PATCH] drm/panthor: Don't access
    gpu_contexts of a destroyed heap pool":
    https://lore.kernel.org/all/20261113162648.110226253-1-arpe1618@gmail.com/
    The two patches are independent and apply in either order.
    
    It conflicts with Osama Abdelkader's pending "drm/panthor: Fix race
    when retrieving the new chunk in panthor_heap_grow()", which also
    changes the panthor_alloc_heap_chunk() arguments. I can rebase on top
    of it if it lands first.

 drivers/gpu/drm/panthor/panthor_heap.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
index 99311abdf1..46f53699a2 100644
--- a/drivers/gpu/drm/panthor/panthor_heap.c
+++ b/drivers/gpu/drm/panthor/panthor_heap.c
@@ -137,7 +137,12 @@ static void panthor_free_heap_chunk(struct panthor_heap_pool *pool,
 	kfree(chunk);
 }
 
+/*
+ * @vm must be pool->vm read under pool->lock, or a reference taken from it
+ * under that lock: panthor_heap_pool_destroy() clears pool->vm.
+ */
 static int panthor_alloc_heap_chunk(struct panthor_heap_pool *pool,
+				    struct panthor_vm *vm,
 				    struct panthor_heap *heap,
 				    bool initial_chunk)
 {
@@ -149,7 +154,7 @@ static int panthor_alloc_heap_chunk(struct panthor_heap_pool *pool,
 	if (!chunk)
 		return -ENOMEM;
 
-	chunk->bo = panthor_kernel_bo_create(pool->ptdev, pool->vm, heap->chunk_size,
+	chunk->bo = panthor_kernel_bo_create(pool->ptdev, vm, heap->chunk_size,
 					     DRM_PANTHOR_BO_NO_MMAP,
 					     DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
 					     PANTHOR_VM_KERNEL_AUTO_VA,
@@ -209,6 +214,7 @@ static void panthor_free_heap_chunks(struct panthor_heap_pool *pool,
 }
 
 static int panthor_alloc_heap_chunks(struct panthor_heap_pool *pool,
+				     struct panthor_vm *vm,
 				     struct panthor_heap *heap,
 				     u32 chunk_count)
 {
@@ -216,7 +222,7 @@ static int panthor_alloc_heap_chunks(struct panthor_heap_pool *pool,
 	u32 i;
 
 	for (i = 0; i < chunk_count; i++) {
-		ret = panthor_alloc_heap_chunk(pool, heap, true);
+		ret = panthor_alloc_heap_chunk(pool, vm, heap, true);
 		if (ret)
 			return ret;
 	}
@@ -315,7 +321,7 @@ int panthor_heap_create(struct panthor_heap_pool *pool,
 	heap->max_chunks = max_chunks;
 	heap->target_in_flight = target_in_flight;
 
-	ret = panthor_alloc_heap_chunks(pool, heap, initial_chunk_count);
+	ret = panthor_alloc_heap_chunks(pool, vm, heap, initial_chunk_count);
 	if (ret)
 		goto err_free_heap;
 
@@ -473,7 +479,7 @@ int panthor_heap_grow(struct panthor_heap_pool *pool,
 	 * further jobs in this queue fail immediately instead of having to
 	 * wait for the job timeout.
 	 */
-	ret = panthor_alloc_heap_chunk(pool, heap, false);
+	ret = panthor_alloc_heap_chunk(pool, pool->vm, heap, false);
 	if (ret)
 		goto out_unlock;
 

base-commit: bafda823590ddf7031ecb765d673572fae7ab44a
-- 
2.43.0

Re: [PATCH] drm/panthor: Fix unlocked pool->vm access in panthor_heap_create()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] drm/panthor: use-after-free of pool->gpu_contexts in
  panthor_heap_grow() [1]
--

commit 8965445d49e12381fc8f429998271cbb17e58ff7
Author: Jiwoo Kim <arpe1618@gmail.com>

drm/panthor: Fix unlocked pool->vm access in panthor_heap_create()

This commit passes an explicit panthor_vm pointer to
panthor_alloc_heap_chunk() and panthor_alloc_heap_chunks() so that
panthor_heap_create() uses the VM reference acquired under pool->lock
rather than reading pool->vm unlocked.

> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c
> index 99311abdf1e91..46f53699a2601 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c

[ ... ]

> @@ -473,7 +479,7 @@ int panthor_heap_grow(struct panthor_heap_pool *pool,
>  	 * further jobs in this queue fail immediately instead of having to
>  	 * wait for the job timeout.
>  	 */
> -	ret = panthor_alloc_heap_chunk(pool, heap, false);
> +	ret = panthor_alloc_heap_chunk(pool, pool->vm, heap, false);
>  	if (ret)
>  		goto out_unlock;
>  

[Severity: High]
This problem wasn't introduced by this patch, but can panthor_heap_grow()
(and panthor_heap_return_chunk()) trigger a use-after-free on
pool->gpu_contexts [1] when racing with panthor_heap_pool_destroy()?

drivers/gpu/drm/panthor/panthor_heap.c:panthor_heap_grow() {
	u64 offset = heap_gpu_va - panthor_kernel_bo_gpuva(pool->gpu_contexts);
	u32 heap_id = (u32)offset / panthor_heap_ctx_stride(pool->ptdev);
    ...
	if (offset > U32_MAX || heap_id >= MAX_HEAPS_PER_POOL)
		return -EINVAL;

	down_read(&pool->lock);
    ...
}

group_process_tiler_oom() grabs a reference to the heap pool under
sched->lock and drops the lock before calling panthor_heap_grow().
Because panthor_vm_destroy() does not flush sched->heap_alloc_wq,
panthor_heap_pool_destroy() can destroy pool->gpu_contexts under
pool->lock while the pool stays alive through the worker's reference:

kworker                         CPU0
group_tiler_oom_work()
  group_process_tiler_oom()
    panthor_vm_get_heap_pool()
                                panthor_vm_destroy()
                                  panthor_heap_pool_destroy()
                                    down_write(&pool->lock)
                                    panthor_kernel_bo_destroy(...)
                                    up_write(&pool->lock)
    panthor_heap_grow()
      panthor_kernel_bo_gpuva(...)  <-- use-after-free

Both panthor_heap_grow() and panthor_heap_return_chunk() dereference
pool->gpu_contexts via panthor_kernel_bo_gpuva() before acquiring
pool->lock and checking whether the pool has been destroyed.

[1] https://sashiko.dev/#/bug/linux-7cebf7b5-5dd3-4267-b0af-8d80a7aa727d

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113171211.836427578-1-arpe1618@gmail.com?part=1