On 15/05/2025 19:04, Daniel Stone wrote:
Hi Steven,
On Thu, 8 May 2025 at 11:42, Steven Price steven.price@arm.com wrote:
I'm also seeing a splat when running this, see below. I haven't got my head around how this is happening, but I see it when glmark quits at the end of the test.
[ 399.505066] Unable to handle kernel NULL pointer dereference at virtual address 00000004 when write [...] [ 399.882216] Call trace: [ 399.882222] panfrost_gem_free_object [panfrost] from drm_gem_handle_delete+0x84/0xb0 [ 399.893813] drm_gem_handle_delete from drm_ioctl+0x2b8/0x4f4 [ 399.900237] drm_ioctl from sys_ioctl+0x428/0xe30 [ 399.905496] sys_ioctl from ret_fast_syscall+0x0/0x1c
Soooo. Let's assume it has to actually occur in panfrost_gem_debugfs_bo_rm(), since that's all that's changed here.
I don't think pfdev can be NULL here, because we've already dereferenced ptdev and written to a structure member earlier in panfrost_gem_free_object(). I don't think it can be the debugfs mutex, because a) that's initialised with the device, and b) wouldn't be offset 0x4.
I'm looking then at list_del_init(&bo->debugfs.node), which would effectively execute bo->debugfs.node->next->prev = bo->debugfs.node->prev. So if bo->debugfs.node->next was NULL, that would explain a write to 0x4 on 32-bit systems.
So I finally got some time to do some debugging on this. And you are absolutely correct on where the fault is triggered.
The cause of it is that panfrost_gem_debugfs_bo_add() is called from panfrost_gem_create(), but that isn't the only place that Panfrost GEM objects are created - it turns out panfrost_perfcnt_enable_locked() also calls drm_gem_shmem_create(). And in that case the list next/prev pointers are left set to NULL, causing things to blow up when the GEM object is freed.
The below patch gets things working, or alternatively just init the list in panfrost_gem_create_object() if we don't want to include the perfcnt buffer in the list.
Steve
---8<-- diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index fe2cdbe8baf0..51da13cd81f0 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -297,13 +297,14 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t obj->base.map_wc = !pfdev->coherent; mutex_init(&obj->label.lock);
+ panfrost_gem_debugfs_bo_add(pfdev, obj); + return &obj->base.base; }
struct panfrost_gem_object * panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) { - struct panfrost_device *pfdev = dev->dev_private; struct drm_gem_shmem_object *shmem; struct panfrost_gem_object *bo;
@@ -319,8 +320,6 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) bo->noexec = !!(flags & PANFROST_BO_NOEXEC); bo->is_heap = !!(flags & PANFROST_BO_HEAP);
- panfrost_gem_debugfs_bo_add(pfdev, bo); - return bo; }