Re: [PATCH v3 11/17] drm/panthor: Don't pre-allocate VMAs or page tables when preparing a full VM unmap

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

Pre-existing issues:
- [Critical] panthor_kernel_bo_destroy ignores the return value of panthor_vm_unmap_range, which can fail with -ENOMEM, leaving physical memory mapped in the GPU after it is freed.
--

--- Patch [11]: [PATCH v3 11/17] drm/panthor: Don't pre-allocate VMAs or page tables when preparing a full VM unmap ---
commit e1c4ff0ccf43825a86dfb74945a18fbc59b6d1ca
Author: Boris Brezillon <[email protected]>

drm/panthor: Don't pre-allocate VMAs or page tables when preparing a full VM unmap

In the cleanup path, we unmap the full VA range to make sure things are
clean before the VM is released. I'd rather not fail on memory
allocation in that path, so let's make sure
panthor_vm_pgtable_prepare_unmap_op_ctx() doesn't allocate VMAs or
page tables when the unmap range matches the VM virtual address range.

> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index a48466788f4db..acdc0dd04f8be 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1466,6 +1466,10 @@ static int panthor_as_prepare_unmap_op_ctx(struct panthor_as_op_ctx *op_ctx,
>  	op_ctx->va.addr = va;
>  	op_ctx->flags = DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP;
>  
> +	/* Unmap on the whole VM range don't need new VMAs or page tables. */
> +	if (va == as->base.mm_start && size == as->base.mm_range)
> +		return 0;
> +

[Severity: Critical]
This is a pre-existing issue, but I noticed that memory allocation failures 
during teardown of a kernel BO can lead to a use-after-free condition.

Because this new early return only applies to full VM unmaps, non-full unmaps 
like those in panthor_kernel_bo_destroy() will still attempt allocations via
panthor_as_op_ctx_prealloc_vmas() which can fail with -ENOMEM.

If memory pressure causes this allocation to fail, panthor_vm_unmap_range() 
will abort and return -ENOMEM. However, panthor_kernel_bo_destroy() ignores 
this return value:

drivers/gpu/drm/panthor/panthor_gem.c:panthor_kernel_bo_destroy() {
	...
	panthor_vm_unmap_range(vm, bo->va_node.start, bo->va_node.size);
	panthor_vm_free_va(vm, &bo->va_node);
	if (vm == panthor_fw_vm(ptdev))
		panthor_gem_unpin(to_panthor_bo(bo->obj));
	drm_gem_object_put(bo->obj);
}

Does this leave physical memory mapped in the GPU after the underlying GEM 
object is destroyed and freed back to the system? 

If a local user can trigger this by creating and destroying scheduling groups 
or heap pools under memory pressure, could they exploit the stale GPU mapping 
to access newly allocated kernel or user memory?

>  	/* Pre-allocate L3 page tables to account for the split-2M-block
>  	 * situation on unmap.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11