Re: [PATCH] drm: Set dev->registered back to false in case of register failure

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

Hi Krzysztof,

On Fri, 2026-05-22 at 17:40 +0200, Krzysztof Niemiec wrote:
> The dev->registered variable was initially added in such a way that it
> is only set after all functions that could have failed have been called
> and haven't returned an error. However, since then, the function
> drm_modeset_register_all() is being checked for its return value, which
> opens a possibility of failing the register after setting the registered
> variable anyway. drm_dev_register() cleans up after itself in case of
> failure, mimicking the functions called in drm_dev_unregister(), with
> the exception of drm_client_sysrq_unregister() and
> drm_panic_unregister() (_register() counterparts of which are not
> checked for their return values), and the dev->registered flag.
> 
> This creates a situation in which after calling drm_dev_register()
> nothing is registered (as the function entered an error path and cleaned
> up), but the device is reported as being registered according to the
> dev->registered variable.
> 
> This, for example, confuses the WARN_ON() in drm_mode_object_register(), which
> is raised if a non-dynamic mode object is attempted to be unregistered
> in between the drm_dev_register() and drm_dev_unregister() calls. If
> drm_dev_register() fails - meaning the device *isn't* registered -
> currently it still reports dev->registered = true, which triggers the
> WARN_ON(), even though the usage is correct in that instance.
> 
> Set dev->registered back to false in the error path to prevent this.

While you've also submitted a patch ("drm/i915: Remove drm_dev_unregister()
from the error path during i915_driver_register()") that adjusts i915
handling of register / unregister steps after this fix, have you checked if
other drivers are prepared for that and won't be affected?

Thanks,
Janusz


> 
> Signed-off-by: Krzysztof Niemiec <krzysztof.niemiec@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 985c283cf59f..92403d79bb64 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1116,6 +1116,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	if (dev->driver->unload)
>  		dev->driver->unload(dev);
>  err_minors:
> +	dev->registered = false;
>  	remove_compat_control_link(dev);
>  	drm_minor_unregister(dev, DRM_MINOR_ACCEL);
>  	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);