[PATCH] drm: don't double-free on driver load error

[David Herrmann]

Hi

On Thu, Dec 5, 2013 at 1:19 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> All instances of drm_dev_register are followed by drm_dev_free on
> failure. Don't free dev->control/render/primary on failure, as they will
> be freed by drm_dev_free since commit 8f6599da8e (drm: delay minor
> destruction to drm_dev_free()).
>
> Reported-by: Bruno Prémont <bonbons at linux-vserver.org>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> Bruno, would be nice if you could test this out on your setup without
> the patch that makes load succeed. Ideally this will make drm not die
> in the case that nouveau load fails.
>
>  drivers/gpu/drm/drm_stub.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index f53d524..cd427b8 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -536,17 +536,17 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>         if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
>                 ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
>                 if (ret)
> -                       goto err_control_node;
> +                       goto err_agp;
>         }
>
>         ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
>         if (ret)
> -               goto err_render_node;
> +               goto err_agp;
>
>         if (dev->driver->load) {
>                 ret = dev->driver->load(dev, flags);
>                 if (ret)
> -                       goto err_primary_node;
> +                       goto err_agp;
>         }
>
>         /* setup grouping for legacy outputs */
> @@ -565,12 +565,6 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  err_unload:
>         if (dev->driver->unload)
>                 dev->driver->unload(dev);
> -err_primary_node:
> -       drm_put_minor(dev->primary);
> -err_render_node:
> -       drm_put_minor(dev->render);
> -err_control_node:
> -       drm_put_minor(dev->control);

No, please use drm_unplug_minor() here. Otherwise, our unload ordering is wrong.

Nice catch,
David

>  err_agp:
>         if (dev->driver->bus->agp_destroy)
>                 dev->driver->bus->agp_destroy(dev);
> --
> 1.8.3.2
>