Re: [PATCH 1/4] drm/imagination: Limit access to the MIPS firmware interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imagination: kernel panic in pvr_vm_mips_map() on 64KB-page systems due to 0-page vmap
--

commit f751c0095eefbac6525746590080b3626f16f23d
Author: Alessio Belle <alessio.belle@imgtec.com>

drm/imagination: Limit access to the MIPS firmware interface

This commit restricts access to the MIPS firmware interface by using the
generic firmware interface or the MIPS-specific VM interface. It splits the
pvr_fw_mips_data struct into internal structures and removes the public
MIPS firmware interface header.

> diff --git a/drivers/gpu/drm/imagination/pvr_vm_mips.c b/drivers/gpu/drm/imagination/pvr_vm_mips.c
> index 5847a1c92bea8..264ea0fccf9c1 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm_mips.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm_mips.c
> @@ -43,49 +69,49 @@ pvr_vm_mips_init(struct pvr_device *pvr_dev)

[ ... ]

> -	mips_data->pt = vmap(mips_data->pt_pages, pt_size >> PAGE_SHIFT, VM_MAP,
> -			     pgprot_writecombine(PAGE_KERNEL));
> -	if (!mips_data->pt) {
> +	vm_data->pt = vmap(vm_data->pt_pages, pt_size >> PAGE_SHIFT, VM_MAP,
> +			   pgprot_writecombine(PAGE_KERNEL));
> +	if (!vm_data->pt) {

[Severity: High]
Does this code handle architectures with 64KB pages correctly?

During DRM driver initialization, pvr_vm_mips_init() allocates firmware
page tables. The page table size pt_size evaluates to 16384 bytes (16KB).
When vmap() is called here, pt_size >> PAGE_SHIFT evaluates to 0 on
architectures with 64KB pages (PAGE_SHIFT=16). Passing a 0 page count to
vmap() returns a non-NULL pointer to an unmapped guard page area.

> @@ -190,10 +216,10 @@ pvr_vm_mips_map(struct pvr_device *pvr_dev, struct pvr_fw_object *fw_obj)
>  			goto err_unmap_pages;
>  
>  		pte = ((dma_addr >> ROGUE_MIPSFW_LOG2_PAGE_SIZE_4K)
> -		       << ROGUE_MIPSFW_ENTRYLO_PFN_SHIFT) & mips_data->pfn_mask;
> +		       << ROGUE_MIPSFW_ENTRYLO_PFN_SHIFT) & vm_data->pfn_mask;
>  		pte |= pte_flags;
>  
> -		WRITE_ONCE(mips_data->pt[pfn], pte);
> +		WRITE_ONCE(vm_data->pt[pfn], pte);

[Severity: High]
Can this write trigger a kernel panic on 64KB-page systems?

If vm_data->pt points to an unmapped guard page returned by vmap() due to
the 0 page count, writing to vm_data->pt[pfn] here will dereference that
unmapped address and cause a page fault.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-simplify-mips-pte-defs-v1-0-b4ad7bb68e22@imgtec.com?part=1