Re: [PATCH v13 08/15] drm/panfrost: Move all DRM device initialisation into device_init()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panfrost: UAF and MMIO faults on device unbind due to missing drm_dev_unplug
--

commit 6682d5cdca6ad86a6d0e64fd3ea14cdc2a997862
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Move all DRM device initialisation into device_init()

Ideally the probe() function will do as little as possible, and all device
initialisation and registration should happen inside the panfrost device
subsystem, just like it's done in Panthor. This also simplifies resource
unwinding in the error path. Do the same thing for DRM driver remove.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 907d4a14a0b55..f77780c72a1ae 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c

[ ... ]

> @@ -891,13 +847,7 @@ static void panfrost_remove(struct platform_device *pdev)
>  
>  	drm_dev_unregister(&pfdev->base);
>  
> -	pm_runtime_get_sync(pfdev->base.dev);
> -	pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> -	pm_runtime_disable(pfdev->base.dev);
>  	panfrost_device_fini(pfdev);

[Severity: High]
Does this sequence cause a regression that leaves the driver vulnerable to
use-after-free and MMIO faults on device unbind?

When panfrost_remove() calls drm_dev_unregister(), it removes the device
node but does not revoke existing open file descriptors. The subsequent
call to panfrost_device_fini() shuts down the hardware, and after this
function returns, the driver core will free all devm allocations.

For example, panfrost_jm_init() allocates pfdev->js using devm_kzalloc:

drivers/gpu/drm/panfrost/panfrost_job.c:panfrost_jm_init() {
	...
	js = devm_kzalloc(pfdev->base.dev, sizeof(*js), GFP_KERNEL);
	if (!js)
		return -ENOMEM;
	pfdev->js = js;
	...
}

If a user unbinds the driver (e.g., via sysfs) while holding an open DRM
file descriptor, subsequent ioctls (like PANFROST_IOCTL_SUBMIT) could
access the freed pfdev->js pointer and unmapped or powered-off hardware
registers.

Should this teardown path use drm_dev_unplug() instead, and should the
driver add drm_dev_enter() and drm_dev_exit() guards to safely manage
concurrent ioctls during device removal?

> -	pm_runtime_set_suspended(pfdev->base.dev);
> -	pm_runtime_put_noidle(pfdev->base.dev);
> -
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-claude-fixes-v13-0-d6a493185860@collabora.com?part=8