[Intel-gfx] [PATCH v3 2/2] drm/i915: On fb alloc failure, unref gem object where it gets refed

[Lukas Wunner]

Currently when allocating a framebuffer fails, the gem object gets
unrefed at the bottom of the call chain in __intel_framebuffer_create,
not where it gets refed, which is in intel_framebuffer_create_for_mode
(via i915_gem_alloc_object) and in intel_user_framebuffer_create
(via drm_gem_object_lookup).

This invites mistakes: As discovered by Tvrtko Ursulin, a double unref
has sneaked into intelfb_alloc (which calls __intel_framebuffer_create).

As suggested by Ville Syrjälä, improve code clarity by moving the unref
away from __intel_framebuffer_create to where the gem object gets refed.

Signed-off-by: Lukas Wunner <lu...@wunner.de>
Fixes: a8bb6818270c ("drm/i915: Fix error path leak in fbdev fb
    allocation")
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9079fcd..9499003 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8876,20 +8876,17 @@ __intel_framebuffer_create(struct drm_device *dev,
        int ret;
 
        intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
-       if (!intel_fb) {
-               drm_gem_object_unreference(&obj->base);
+       if (!intel_fb)
                return ERR_PTR(-ENOMEM);
-       }
 
        ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
        if (ret)
                goto err;
 
        return &intel_fb->base;
+
 err:
-       drm_gem_object_unreference(&obj->base);
        kfree(intel_fb);
-
        return ERR_PTR(ret);
 }
 
@@ -8929,6 +8926,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
                                  struct drm_display_mode *mode,
                                  int depth, int bpp)
 {
+       struct drm_framebuffer *fb;
        struct drm_i915_gem_object *obj;
        struct drm_mode_fb_cmd2 mode_cmd = { 0 };
 
@@ -8943,7 +8941,11 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
                                                                bpp);
        mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);
 
-       return intel_framebuffer_create(dev, &mode_cmd, obj);
+       fb = intel_framebuffer_create(dev, &mode_cmd, obj);
+       if (IS_ERR(fb))
+               drm_gem_object_unreference(&obj->base);
+
+       return fb;
 }
 
 static struct drm_framebuffer *
@@ -13379,6 +13381,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
                              struct drm_file *filp,
                              struct drm_mode_fb_cmd2 *mode_cmd)
 {
+       struct drm_framebuffer *fb;
        struct drm_i915_gem_object *obj;
 
        obj = to_intel_bo(drm_gem_object_lookup(dev, filp,
@@ -13386,7 +13389,11 @@ intel_user_framebuffer_create(struct drm_device *dev,
        if (&obj->base == NULL)
                return ERR_PTR(-ENOENT);
 
-       return intel_framebuffer_create(dev, mode_cmd, obj);
+       fb = intel_framebuffer_create(dev, mode_cmd, obj);
+       if (IS_ERR(fb))
+               drm_gem_object_unreference(&obj->base);
+
+       return fb;
 }
 
 static void intel_output_poll_changed(struct drm_device *dev)
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx