drm/fb-helper: generic: Fix setup error path

If register_framebuffer() fails during fbdev setup we will leak the
framebuffer, the GEM buffer and the shadow buffer for defio. This is
because drm_fb_helper_fbdev_setup() just calls drm_fb_helper_fini() on
error not taking into account that register_framebuffer() can fail.

Since the generic emulation uses DRM client for its framebuffer and
backing buffer in addition to a shadow buffer, it's necessary to open code
drm_fb_helper_fbdev_setup() to properly handle the error path.

Error cleanup is removed from .fb_probe and is handled by one function for
all paths.

Fixes: 9060d7f49376 ("drm/fb-helper: Finish the generic fbdev emulation")
Reported-by: Peter Wu <peter@lekensteyn.nl>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190105181846.26495-1-noralf@tronnes.org

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5e9ca6f9637929ff76a37db4c8e936eba82c5279..80136a7a5af1f3a8376edd1df7decc8a0a764f37 100644 (file)
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2961,18 +2961,16 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
        return 0;
 }
 
-/*
- * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
- * unregister_framebuffer() or fb_release().
- */
-static void drm_fbdev_fb_destroy(struct fb_info *info)
+static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
 {
-       struct drm_fb_helper *fb_helper = info->par;
        struct fb_info *fbi = fb_helper->fbdev;
        struct fb_ops *fbops = NULL;
        void *shadow = NULL;
 
-       if (fbi->fbdefio) {
+       if (!fb_helper->dev)
+               return;
+
+       if (fbi && fbi->fbdefio) {
                fb_deferred_io_cleanup(fbi);
                shadow = fbi->screen_buffer;
                fbops = fbi->fbops;
@@ -2986,6 +2984,12 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
        }
 
        drm_client_framebuffer_delete(fb_helper->buffer);
+}
+
+static void drm_fbdev_release(struct drm_fb_helper *fb_helper)
+{
+       drm_fbdev_cleanup(fb_helper);
+
        /*
         * FIXME:
         * Remove conditional when all CMA drivers have been moved over to using
@@ -2997,6 +3001,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
        }
 }
 
+/*
+ * fb_ops.fb_destroy is called by the last put_fb_info() call at the end of
+ * unregister_framebuffer() or fb_release().
+ */
+static void drm_fbdev_fb_destroy(struct fb_info *info)
+{
+       drm_fbdev_release(info->par);
+}
+
 static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
        struct drm_fb_helper *fb_helper = info->par;
@@ -3047,7 +3060,6 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
        struct drm_framebuffer *fb;
        struct fb_info *fbi;
        u32 format;
-       int ret;
 
        DRM_DEBUG_KMS("surface width(%d), height(%d) and bpp(%d)\n",
                      sizes->surface_width, sizes->surface_height,
@@ -3064,10 +3076,8 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
        fb = buffer->fb;
 
        fbi = drm_fb_helper_alloc_fbi(fb_helper);
-       if (IS_ERR(fbi)) {
-               ret = PTR_ERR(fbi);
-               goto err_free_buffer;
-       }
+       if (IS_ERR(fbi))
+               return PTR_ERR(fbi);
 
        fbi->par = fb_helper;
        fbi->fbops = &drm_fbdev_fb_ops;
@@ -3098,8 +3108,7 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
                if (!fbops || !shadow) {
                        kfree(fbops);
                        vfree(shadow);
-                       ret = -ENOMEM;
-                       goto err_fb_info_destroy;
+                       return -ENOMEM;
                }
 
                *fbops = *fbi->fbops;
@@ -3111,13 +3120,6 @@ int drm_fb_helper_generic_probe(struct drm_fb_helper *fb_helper,
        }
 
        return 0;
-
-err_fb_info_destroy:
-       drm_fb_helper_fini(fb_helper);
-err_free_buffer:
-       drm_client_framebuffer_delete(buffer);
-
-       return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_generic_probe);
 
@@ -3129,18 +3131,11 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client)
 {
        struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
 
-       if (fb_helper->fbdev) {
-               drm_fb_helper_unregister_fbi(fb_helper);
+       if (fb_helper->fbdev)
                /* drm_fbdev_fb_destroy() takes care of cleanup */
-               return;
-       }
-
-       /* Did drm_fb_helper_fbdev_setup() run? */
-       if (fb_helper->dev)
-               drm_fb_helper_fini(fb_helper);
-
-       drm_client_release(client);
-       kfree(fb_helper);
+               drm_fb_helper_unregister_fbi(fb_helper);
+       else
+               drm_fbdev_release(fb_helper);
 }
 
 static int drm_fbdev_client_restore(struct drm_client_dev *client)
@@ -3158,7 +3153,7 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
        struct drm_device *dev = client->dev;
        int ret;
 
-       /* If drm_fb_helper_fbdev_setup() failed, we only try once */
+       /* Setup is not retried if it has failed */
        if (!fb_helper->dev && fb_helper->funcs)
                return 0;
 
@@ -3170,15 +3165,34 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
                return 0;
        }
 
-       ret = drm_fb_helper_fbdev_setup(dev, fb_helper, &drm_fb_helper_generic_funcs,
-                                       fb_helper->preferred_bpp, 0);
-       if (ret) {
-               fb_helper->dev = NULL;
-               fb_helper->fbdev = NULL;
-               return ret;
-       }
+       drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
+
+       ret = drm_fb_helper_init(dev, fb_helper, dev->mode_config.num_connector);
+       if (ret)
+               goto err;
+
+       ret = drm_fb_helper_single_add_all_connectors(fb_helper);
+       if (ret)
+               goto err_cleanup;
+
+       if (!drm_drv_uses_atomic_modeset(dev))
+               drm_helper_disable_unused_functions(dev);
+
+       ret = drm_fb_helper_initial_config(fb_helper, fb_helper->preferred_bpp);
+       if (ret)
+               goto err_cleanup;
 
        return 0;
+
+err_cleanup:
+       drm_fbdev_cleanup(fb_helper);
+err:
+       fb_helper->dev = NULL;
+       fb_helper->fbdev = NULL;
+
+       DRM_DEV_ERROR(dev->dev, "fbdev: Failed to setup generic emulation (ret=%d)\n", ret);
+
+       return ret;
 }
 
 static const struct drm_client_funcs drm_fbdev_client_funcs = {
@@ -3237,6 +3251,10 @@ int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
 
        drm_client_add(&fb_helper->client);
 
+       if (!preferred_bpp)
+               preferred_bpp = dev->mode_config.preferred_depth;
+       if (!preferred_bpp)
+               preferred_bpp = 32;
        fb_helper->preferred_bpp = preferred_bpp;
 
        ret = drm_fbdev_client_hotplug(&fb_helper->client);