Re: [PATCH 2/2] drm/imagination: Fix page count for page table for map() interface

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incorrect calculation of the virtual address range for page table preallocation due to mistakenly adding the physical buffer offset (`sgt_offset`) to the virtual device address (`device_addr`).

Pre-existing issues:
- [High] Massive integer underflow in page table count calculation when a mapping crosses a page table boundary leads to an infinite allocation loop and kernel memory exhaustion (OOM).
--

commit e3abd749cd49c287313d0450da2a3a8485c9e53f
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>

    drm/imagination: Fix page count for page table for map() interface
    
    GPU virtual start address wasn't included in page count for page table
    calculation for mapping an BO object in map() interface. It resulted in
    map failure later due to not enough pages at L0/L1 level.
    Update pvr_mmu_op_context_create() interface to pass device address as well
    to allow correct calculation for page table memory.

> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 175f0ba4d993f..52d8fbc003841 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2347,8 +2348,9 @@ void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx)
>   */
>  struct pvr_mmu_op_context *
>  pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
> -			  u64 sgt_offset, u64 size)
> +			  u64 device_addr, u64 sgt_offset, u64 size)
>  {
> +	u64 start_addr = device_addr + sgt_offset;

[Severity: High]
Does this code incorrectly mix the virtual device address (device_addr) and
the physical buffer offset (sgt_offset) to compute the starting address?

If sgt_offset represents the offset within the backing buffer, adding it to
the virtual address skews the calculation for the required page tables. This
could cause valid userspace mapping requests to randomly fail with -ENOMEM
if the shifted range covers fewer page table boundaries than the actual
mapping, causing the driver to allocate too few tables.

[ ... ]

> @@ -2364,16 +2366,16 @@ pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
>  	if (size) {
>  		/*
>  		 * The number of page table objects we need to prealloc is
> -		 * indicated by the mapping size, start offset and the sizes
> +		 * indicated by the mapping size, start address and the sizes
>  		 * of the areas mapped per PT or PD. The range calculation is
>  		 * identical to that for the index into a table for a device
>  		 * address, so we reuse those functions here.
>  		 */
> -		const u32 l1_start_idx = pvr_page_table_l2_idx(sgt_offset);
> -		const u32 l1_end_idx = pvr_page_table_l2_idx(sgt_offset + size);
> +		const u32 l1_start_idx = pvr_page_table_l2_idx(start_addr);
> +		const u32 l1_end_idx = pvr_page_table_l2_idx(start_addr + size);
>  		const u32 l1_count = l1_end_idx - l1_start_idx + 1;
> -		const u32 l0_start_idx = pvr_page_table_l1_idx(sgt_offset);
> -		const u32 l0_end_idx = pvr_page_table_l1_idx(sgt_offset + size);
> +		const u32 l0_start_idx = pvr_page_table_l1_idx(start_addr);
> +		const u32 l0_end_idx = pvr_page_table_l1_idx(start_addr + size);
>  		const u32 l0_count = l0_end_idx - l0_start_idx + 1;

[Severity: High]
This is a pre-existing issue, but can this subtraction cause a massive
integer underflow when a mapping crosses a page table boundary?

Because pvr_page_table_l1_idx() and pvr_page_table_l2_idx() return local
indices bounded within their parent tables (e.g., 0-511), if the address range
crosses a boundary (1GB for L1 tables or 2MB for L0 tables), l0_end_idx could
be smaller than l0_start_idx (e.g., 0 and 511).

The subtraction l0_end_idx - l0_start_idx + 1 would evaluate to an unsigned
32-bit underflow.

When this happens, the subsequent loop for (int i = 0; i < l0_count; i++)
in pvr_mmu_op_context_create() iterates billions of times, exhausting kernel
memory via kzalloc and triggering the OOM killer. This is directly reachable
by unprivileged userspace via the GPUVM map IOCTL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-mmu_fix-v1-0-c55a23fb7cab@imgtec.com?part=2