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