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 <[email protected]> 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/[email protected]?part=6
