Re: [PATCH v3 4/4] drm/panthor: Check for sparse binding range overflow

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The newly added overflow check in `panthor_vm_prepare_unmap_op_ctx` is mathematically dead code because the `MAP_SPARSE` flag is explicitly rejected for UNMAP operations.
- [Low] The overflow check in `panthor_vm_prepare_map_op_ctx` is artificially restricted to sparse mappings, bypassing early protection for non-sparse mappings.

Pre-existing issues:
- [High] Unbounded sparse mappings allow unprivileged users to instantly consume massive amounts of un-accounted kernel memory, causing system-wide OOM (Denial of Service).
--

commit 5e16f4b93a1093aa035900789a27182078dc6a8d
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panthor: Check for sparse binding range overflow

This commit adds checks to prevent sparse binding range overflows early in
the driver before GPUVM functions are invoked. This avoids wasting time
allocating vm_bind context resources.

Closes: https://sashiko.dev/#/message/20260623204220.CDB1B1F000E9%40smtp.kernel.org
Fixes: 12cf826bf1dd ("drm/panthor: Support sparse mappings")

> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 77fdad4e51665..8bd9b975e5ce2 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c

[ ... ]

> @@ -1353,6 +1354,10 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  	if (is_sparse && (op->bo_handle || op->bo_offset))
>  		return -EINVAL;
>  
> +	/* Protect against sparse VA range overflow */
> +	if (is_sparse && check_add_overflow(op->va, op->size, &end))
> +		return -EINVAL;
> +

[Severity: Low]
Is this overflow check artificially restricted to sparse mappings?

If a non-sparse mapping request overflows the 64-bit bounds, the wrapped sum
bypasses the vm->user_va_range check and misses this new early check. The
driver will needlessly pin the BO and pre-allocate context resources before
safely failing downstream in drm_gpuvm_sm_map().

Would dropping the is_sparse condition here correctly fail early for both
sparse and non-sparse mappings?

[Severity: High]
This is a pre-existing issue, but does the lack of accounting on pt_cache
allow untrusted users to cause a system-wide OOM?

When a user submits a sparse mapping request with a massive size (e.g., 512GB)
and va = 0, it skips the backing GEM size validations. In
panthor_vm_op_ctx_prealloc_pts(), pt_count is calculated assuming the entire
range needs full L1-L3 page tables:

drivers/gpu/drm/panthor/panthor_mmu.c:panthor_vm_op_ctx_prealloc_pts() {
    ...
    ret = kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count,
                                op_ctx->rsvd_page_tables.pages);
    ...
}

Because pt_cache is created without the SLAB_ACCOUNT flag:

drivers/gpu/drm/panthor/panthor_mmu.c:panthor_mmu_pt_cache_init() {
    pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL);
    ...
}

Can a local user execute this ioctl concurrently in a loop, draining tens of
gigabytes of memory that remain invisible to cgroup limits, to crash the host?

>  	/* If the BO has an exclusive VM attached, it can't be mapped to other VMs. */
>  	if (bo->exclusive_vm_root_gem &&
>  	    bo->exclusive_vm_root_gem != panthor_vm_root_gem(vm))

[ ... ]

> @@ -1432,6 +1439,10 @@ static int panthor_vm_prepare_unmap_op_ctx(struct panthor_vm_op_ctx *op_ctx,
>  	if (op->va != ALIGN(op->va, SZ_2M))
>  		pt_count++;
>  
> +	/* Protect against sparse VA range overflow */
> +	if (is_sparse && check_add_overflow(op->va, op->size, &end))
> +		return -EINVAL;
> +

[Severity: Low]
Is this overflow check mathematically dead code?

In panthor_vm_bind_prepare_op_ctx(), any UNMAP operation containing flags
outside of DRM_PANTHOR_VM_BIND_OP_TYPE_MASK (which includes MAP_SPARSE) is
explicitly rejected with -EINVAL.

Because of this, is_sparse will evaluate to false unconditionally for UNMAP
operations, causing this check to always be skipped.

>  	if (op->va + op->size != ALIGN(op->va + op->size, SZ_2M) &&
>  	    ALIGN(op->va + op->size, SZ_2M) != ALIGN(op->va, SZ_2M))
>  		pt_count++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-vm_bind_checks-v3-0-85e6740f6c2e@collabora.com?part=4