Re: [PATCH v2 24/30] drm/panfrost: switch to iova_to_phys_length

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-after-free of panfrost_gem_mapping caused by missing kref_get_unless_zero() during page fault handling.
- [High] IOVA aliasing and page table corruption due to the page fault handler resurrecting purged heap BOs.
--

commit 2c49ad58ab72225d5654336ec80f9da1ce85ee4e
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

drm/panfrost: switch to iova_to_phys_length

Migrate panfrost_mmu to use ops->iova_to_phys_length(ops, iova, NULL)
instead of the deprecated ops->iova_to_phys.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 4a3162c3b659f..aa0bc82deaf68 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -514,7 +514,7 @@ void panfrost_mmu_unmap(struct panfrost_gem_mapping *mapping)
>  
>  		if (bo->is_heap)
>  			pgcount = 1;
> -		if (!bo->is_heap || ops->iova_to_phys(ops, iova)) {
> +		if (!bo->is_heap || ops->iova_to_phys_length(ops, iova, NULL) != PHYS_ADDR_MAX) {
>  			unmapped_page = ops->unmap_pages(ops, iova, pgsize, pgcount, NULL);
>  			WARN_ON(unmapped_page != pgsize * pgcount);
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but could this long-running
hardware unmap operation race with page fault handling and cause a
use-after-free?

When a GEM mapping's refcount drops to 0, panfrost_gem_mapping_release()
calls panfrost_gem_teardown_mapping(), which drops the mm_lock during hardware
unmapping:

drivers/gpu/drm/panfrost/panfrost_gem.c:panfrost_gem_teardown_mapping() {
    ...
    if (mapping->active)
        panfrost_mmu_unmap(mapping);

    spin_lock(&mapping->mmu->mm_lock);
    if (drm_mm_node_allocated(&mapping->mmnode))
        drm_mm_remove_node(&mapping->mmnode);
    spin_unlock(&mapping->mmu->mm_lock);
    ...
}

While the mm_lock is dropped, the mmnode remains discoverable in the drm_mm
tree. A concurrent page fault can then find this soon-to-be-freed mapping
and increment its refcount from 0 without checking:

drivers/gpu/drm/panfrost/panfrost_mmu.c:addr_to_mapping() {
    ...
    mapping = drm_mm_node_to_panfrost_mapping(node);

    kref_get(&mapping->refcount);
    break;
    ...
}

Once the release function finishes and frees the mapping, the page fault handler
would dereference the freed mapping pointer. Does addr_to_mapping() need to
use kref_get_unless_zero() to prevent this?

[Severity: High]
This is a pre-existing issue, but can the page fault handler resurrect a
purged heap BO and cause IOVA aliasing?

When the shrinker purges a heap BO, it frees the IOVA range and drops the
pages before releasing the BO's dma_resv lock:

drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c:panfrost_gem_purge() {
    ...
    panfrost_gem_teardown_mappings_locked(bo);
    drm_gem_shmem_purge_locked(&bo->base);
    ...
}

Concurrently, a page fault handler might have found the mapping via
addr_to_mapping() and blocked waiting for the same dma_resv lock.

Once the shrinker completes and drops the lock, the fault handler wakes up,
acquires the lock, and sees !bo->base.pages. It then blindly allocates new
pages and maps them:

drivers/gpu/drm/panfrost/panfrost_mmu.c:panfrost_mmu_map_fault_addr() {
    ...
    if (!bo->base.pages) {
        bo->sgts = kvmalloc_objs(struct sg_table,
                                 bo->base.base.size / SZ_2M,
                                 GFP_KERNEL | __GFP_ZERO);
    ...
}

Since the IOVA range was already freed in the drm_mm allocator by the shrinker,
it could have been assigned to a different BO, leading to hardware page table
corruption. Should panfrost_mmu_map_fault_addr() verify if the BO was purged
(e.g., by checking if the mmnode is still allocated) before allocating new
pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602104637.1219810-1-guanghuifeng@linux.alibaba.com?part=24