Re: [PATCH v1 06/27] drm/panthor: Derive MMU AS register addresses from base and stride

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] panthor: NULL pointer dereference in MMU IRQ handler on spurious interrupts
--

commit 9aecd8c0de2222b2839590424b986eaecb02eee6
Author: Karunika Choo <karunika.choo@arm.com>

drm/panthor: Derive MMU AS register addresses from base and stride

This commit moves the MMU address-space register layout into the hardware
description. It computes the iomem pointer for each AS slot based on the base
offset and per-AS stride to prepare for GPUs with non-fixed MMU AS register
regions.

> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index d75d575473da4..169550413a1a5 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -1871,8 +1877,8 @@ static void panthor_mmu_irq_handler(struct panthor_irq *pirq, u32 status)
>  		u32 access_type;
>  		u32 source_id;
>  
> -		fault_status = gpu_read(mmu->iomem, AS_FAULTSTATUS(as));
> -		addr = gpu_read64(mmu->iomem, AS_FAULTADDRESS(as));
> +		fault_status = gpu_read(as_iomem(mmu, as), AS_FAULTSTATUS);
> +		addr = gpu_read64(as_iomem(mmu, as), AS_FAULTADDRESS);

[Severity: High]
Could this lead to a NULL pointer dereference on a spurious interrupt?

If the MMU interrupt status register has bits set for non-present AS slots
(such as from a hardware glitch, or reading 0xFFFFFFFF during a device
unplug or reset state), the loop in panthor_mmu_irq_handler() will process
these unpopulated slots.

Since as_iomem() returns NULL for slots that weren't initialized, passing
that NULL pointer to gpu_read() would trigger a kernel crash.

[ ... ]

> @@ -3408,7 +3417,13 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> -	mmu->iomem = ptdev->iomem + MMU_AS_BASE;
> +	for_each_set_bit(as_id, &as_present_mask, MAX_AS_SLOTS) {
> +		u64 offset = regbank->offset + (regbank->stride * as_id);
> +
> +		guard(mutex)(&mmu->as.slots_lock);
> +		mmu->as.slots[as_id].iomem = ptdev->iomem + offset;
> +	}
> +

[Severity: High]
Because we only initialize the iomem pointers for present AS slots here,
the remaining array elements are left as NULL. Is there a need to guard
against accessing uninitialized slots in panthor_mmu_irq_handler() before
calling as_iomem() to prevent the issue mentioned above?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922204535.2850094-1-karunika.choo@arm.com?part=6