[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1454030348-17736-150-git-send-email-kamal@canonical.com>
Date: Thu, 28 Jan 2016 17:18:07 -0800
From: Kamal Mostafa <kamal@...onical.com>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
kernel-team@...ts.ubuntu.com
Cc: Tvrtko Ursulin <tvrtko.ursulin@...el.com>,
Chris Wilson <chris@...is-wilson.co.uk>,
Lukas Wunner <lukas@...ner.de>,
Jani Nikula <jani.nikula@...el.com>,
Kamal Mostafa <kamal@...onical.com>
Subject: [PATCH 3.19.y-ckt 149/210] drm/i915: On fb alloc failure, unref gem object where it gets refed
3.19.8-ckt14 -stable review patch. If anyone has any objections, please let me know.
---8<------------------------------------------------------------
From: Lukas Wunner <lukas@...ner.de>
commit dcb1394e74e32e33856e877bca1a0e88c78323d8 upstream.
Currently when allocating a framebuffer fails, the gem object gets
unrefed at the bottom of the call stack 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: __intel_framebuffer_create is also called from
intelfb_alloc, and as discovered by Tvrtko Ursulin, a double unref
was introduced there with a8bb6818270c ("drm/i915: Fix error path leak
in fbdev fb allocation").
As suggested by Ville Syrjälä, fix the double unref and improve code
clarity by moving the unref away from __intel_framebuffer_create to
where the gem object gets refed.
Based on Tvrtko Ursulin's original v2.
v3: On fb alloc failure, unref gem object where it gets refed,
fix double unref in separate commit (Ville Syrjälä)
v4: Lock struct_mutex on unref (Chris Wilson)
v5: Rebase on drm-intel-nightly 2015y-09m-01d-09h-06m-08s UTC,
rephrase commit message (Jani Nicula)
Tested-by: Pierre Moreau <pierre.morrow@...e.fr>
[MBP 5,3 2009 nvidia MCP79 + G96 pre-retina]
Tested-by: Paul Hordiienko <pvt.gord@...il.com>
[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina]
Tested-by: William Brown <william@...ckhats.net.au>
[MBP 8,2 2011 intel SNB + amd turks pre-retina]
Tested-by: Lukas Wunner <lukas@...ner.de>
[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina]
Tested-by: Bruno Bierbaumer <bruno@...rbaumer.net>
[MBP 11,3 2013 intel HSW + nvidia GK107 retina]
Fixes: a8bb6818270c ("drm/i915: Fix error path leak in fbdev fb
allocation")
Cc: Tvrtko Ursulin <tvrtko.ursulin@...el.com>
Cc: Chris Wilson <chris@...is-wilson.co.uk>
Signed-off-by: Lukas Wunner <lukas@...ner.de>
Reviewed-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/2161c5062ef5d6458f8ae14d924a26d4d1dba317.1446892879.git.lukas@wunner.de
Signed-off-by: Jani Nikula <jani.nikula@...el.com>
Signed-off-by: Kamal Mostafa <kamal@...onical.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 48a19dc..9198724 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8544,20 +8544,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);
}
@@ -8597,6 +8594,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 };
@@ -8611,7 +8609,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 *
@@ -12731,6 +12733,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,
@@ -12738,7 +12741,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;
}
#ifndef CONFIG_DRM_I915_FBDEV
--
1.9.1
Powered by blists - more mailing lists