Re: [PATCH v3 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 1 potential issue(s) to consider:
- [High] drm/imagination: map failure due to incorrect page table preallocation
--

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

drm/imagination: Fix page count for page table for map() interface

This commit aims to fix the page count calculation for mapping a buffer object
by including the GPU virtual start address. It updates the
pvr_mmu_op_context_create function to accept the device address and uses it
to compute the required page table memory.

> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 23261d9ad3fdd..62eae7fcd5a26 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 computation incorrectly add the physical buffer offset (sgt_offset)
to the virtual device address (device_addr)?

It appears that shifting the address window this way can lead to
under-allocation of page tables. For instance, if a user-space application
maps a memory region where the true virtual address range from device_addr
to device_addr + size crosses more page directory boundaries than the
shifted range (start_addr to start_addr + size), the preallocation will
fall short.

As an example, if L0 tables cover 2MB (0x200000), and we have
device_addr = 0x1FF000, size = 0x2000, and sgt_offset = 0x1000: the actual
mapping touches both 0x1FF000 and 0x200000, requiring two L0 tables. However,
start_addr becomes 0x200000, causing the logic to preallocate only one
L0 table. Later, when pvr_mmu_map executes, it could fail to retrieve the
second table from the preallocated list and return -ENOMEM.

Could this result in unprivileged user-space clients experiencing random map
failures depending on the buffer offset?

>  	int err;
>  
>  	struct pvr_mmu_op_context *op_ctx = kzalloc_obj(*op_ctx);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-mmu_fix-v3-0-7b3d73aef041@imgtec.com?part=2