Commit dd004fc2 authored by Lukas Wunner's avatar Lukas Wunner Committed by Luis Henriques

drm/i915: On fb alloc failure, unref gem object where it gets refed

commit dcb1394e 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 a8bb6818 ("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: default avatarPierre Moreau <pierre.morrow@free.fr>
    [MBP  5,3 2009  nvidia MCP79 + G96        pre-retina]
Tested-by: default avatarPaul Hordiienko <pvt.gord@gmail.com>
    [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina]
Tested-by: default avatarWilliam Brown <william@blackhats.net.au>
    [MBP  8,2 2011  intel SNB + amd turks     pre-retina]
Tested-by: default avatarLukas Wunner <lukas@wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina]
Tested-by: default avatarBruno Bierbaumer <bruno@bierbaumer.net>
    [MBP 11,3 2013  intel HSW + nvidia GK107  retina]

Fixes: a8bb6818 ("drm/i915: Fix error path leak in fbdev fb
    allocation")
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/2161c5062ef5d6458f8ae14d924a26d4d1dba317.1446892879.git.lukas@wunner.deSigned-off-by: default avatarJani Nikula <jani.nikula@intel.com>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 1646605d
...@@ -8219,20 +8219,17 @@ __intel_framebuffer_create(struct drm_device *dev, ...@@ -8219,20 +8219,17 @@ __intel_framebuffer_create(struct drm_device *dev,
int ret; int ret;
intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL); intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
if (!intel_fb) { if (!intel_fb)
drm_gem_object_unreference(&obj->base);
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
}
ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj); ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
if (ret) if (ret)
goto err; goto err;
return &intel_fb->base; return &intel_fb->base;
err: err:
drm_gem_object_unreference(&obj->base);
kfree(intel_fb); kfree(intel_fb);
return ERR_PTR(ret); return ERR_PTR(ret);
} }
...@@ -8272,6 +8269,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev, ...@@ -8272,6 +8269,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
struct drm_display_mode *mode, struct drm_display_mode *mode,
int depth, int bpp) int depth, int bpp)
{ {
struct drm_framebuffer *fb;
struct drm_i915_gem_object *obj; struct drm_i915_gem_object *obj;
struct drm_mode_fb_cmd2 mode_cmd = { 0 }; struct drm_mode_fb_cmd2 mode_cmd = { 0 };
...@@ -8286,7 +8284,11 @@ intel_framebuffer_create_for_mode(struct drm_device *dev, ...@@ -8286,7 +8284,11 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
bpp); bpp);
mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth); 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 * static struct drm_framebuffer *
...@@ -11421,6 +11423,7 @@ intel_user_framebuffer_create(struct drm_device *dev, ...@@ -11421,6 +11423,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
struct drm_file *filp, struct drm_file *filp,
struct drm_mode_fb_cmd2 *mode_cmd) struct drm_mode_fb_cmd2 *mode_cmd)
{ {
struct drm_framebuffer *fb;
struct drm_i915_gem_object *obj; struct drm_i915_gem_object *obj;
obj = to_intel_bo(drm_gem_object_lookup(dev, filp, obj = to_intel_bo(drm_gem_object_lookup(dev, filp,
...@@ -11428,7 +11431,11 @@ intel_user_framebuffer_create(struct drm_device *dev, ...@@ -11428,7 +11431,11 @@ intel_user_framebuffer_create(struct drm_device *dev,
if (&obj->base == NULL) if (&obj->base == NULL)
return ERR_PTR(-ENOENT); 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 #ifndef CONFIG_DRM_I915_FBDEV
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment