Reviewed-by: Lyude Paul <[email protected]>
On Sun, 2026-06-28 at 16:53 +0200, Danilo Krummrich wrote:
> If drm_dev_register() fails after registering a minor (e.g. render
> minor
> registered, primary minor fails), userspace could have opened the
> first
> minor and entered a drm_dev_enter() critical section. Since the
> unplugged flag was never set, the ioctl proceeds while the error path
> tears down device resources.
>
> Fix this by introducing drm_dev_synchronize_unplug(), which sets the
> unplugged flag and waits for the SRCU barrier, ensuring all in-flight
> drm_dev_enter() critical sections complete before cleanup proceeds;
> call
> it on the error path of drm_dev_register().
>
> Fixes: bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged")
> Cc: [email protected]
> Reported-by: [email protected]
> Closes:
> https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Danilo Krummrich <[email protected]>
> ---
> drivers/gpu/drm/drm_drv.c | 34 +++++++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 675675480da4..e890052061f3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -473,6 +473,22 @@ void drm_dev_exit(int idx)
> }
> EXPORT_SYMBOL(drm_dev_exit);
>
> +/*
> + * Mark the device as unplugged and wait for any in-flight
> drm_dev_enter()
> + * critical sections to complete.
> + */
> +static void drm_dev_synchronize_unplug(struct drm_device *dev)
> +{
> + /*
> + * After synchronizing any critical read section is
> guaranteed to see
> + * the new value of ->unplugged, and any critical section
> which might
> + * still have seen the old value of ->unplugged is
> guaranteed to have
> + * finished.
> + */
> + dev->unplugged = true;
> + synchronize_srcu(&drm_unplug_srcu);
> +}
> +
> /**
> * drm_dev_unplug - unplug a DRM device
> * @dev: DRM device
> @@ -485,15 +501,7 @@ EXPORT_SYMBOL(drm_dev_exit);
> */
> void drm_dev_unplug(struct drm_device *dev)
> {
> - /*
> - * After synchronizing any critical read section is
> guaranteed to see
> - * the new value of ->unplugged, and any critical section
> which might
> - * still have seen the old value of ->unplugged is
> guaranteed to have
> - * finished.
> - */
> - dev->unplugged = true;
> - synchronize_srcu(&drm_unplug_srcu);
> -
> + drm_dev_synchronize_unplug(dev);
> drm_dev_unregister(dev);
>
> /* Clear all CPU mappings pointing to this device */
> @@ -1091,6 +1099,7 @@ int drm_dev_register(struct drm_device *dev,
> unsigned long flags)
> goto err_minors;
>
> dev->registered = true;
> + dev->unplugged = false;
>
> if (driver->load) {
> ret = driver->load(dev, flags);
> @@ -1118,6 +1127,13 @@ int drm_dev_register(struct drm_device *dev,
> unsigned long flags)
> if (dev->driver->unload)
> dev->driver->unload(dev);
> err_minors:
> + /*
> + * If a minor was registered before the failure, userspace
> could have
> + * opened it and entered a drm_dev_enter() critical section.
> Ensure all
> + * such sections complete before we clean up.
> + */
> + drm_dev_synchronize_unplug(dev);
> +
> remove_compat_control_link(dev);
> drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.