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

[sashiko-bot@kernel.org]

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