[PATCH v2 15/22 RESEND] drm/i915: On fb alloc failure, unref gem object where it gets refed

Lukas Wunner
2015-08-12T11:32:13Z

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.

[Changelog is in preceding patch by Tvrtko Ursulin.]

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
Tested-by: Paul Hordiienko <pvt.gord@gmail.com>
[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina]
Tested-by: William Brown <william@blackhats.net.au>
[MBP 8,2 2011 intel SNB + amd turks pre-retina]
Tested-by: Lukas Wunner <lukas@wunner.de>
[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina]
Tested-by: Bruno Bierbaumer <bruno@bierbaumer.net>
[MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Fixes: a8bb6818270c ("drm/i915: Fix error path leak in fbdev fb
allocation")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Ville Syrjälä <ville.syrjala@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 907b73e..5984d5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10145,20 +10145,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);
}

@@ -10198,6 +10195,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 };

@@ -10212,7 +10210,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_unlocked(&obj->base);
+
+ return fb;
}

static struct drm_framebuffer *
@@ -14468,6 +14470,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,
@@ -14475,7 +14478,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_unlocked(&obj->base);
+
+ return fb;
}

static void intel_output_poll_changed(struct drm_device *dev)
--
1.8.5.2 (Apple Git-48)