3.16.7-ckt24 -stable review patch. If anyone has any objections, please let me know. ---8<------------------------------------------------------------ From: Lukas Wunner <lukas@xxxxxxxxx> 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@xxxxxxx> [MBP 5,3 2009 nvidia MCP79 + G96 pre-retina] Tested-by: Paul Hordiienko <pvt.gord@xxxxxxxxx> [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown <william@xxxxxxxxxxxxxxxx> [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner <lukas@xxxxxxxxx> [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer <bruno@xxxxxxxxxxxxxx> [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@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Link: http://patchwork.freedesktop.org/patch/msgid/2161c5062ef5d6458f8ae14d924a26d4d1dba317.1446892879.git.lukas@xxxxxxxxx Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> Signed-off-by: Luis Henriques <luis.henriques@xxxxxxxxxxxxx> --- 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 0d68c70368ff..58563e2574f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8219,20 +8219,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); } @@ -8272,6 +8269,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 }; @@ -8286,7 +8284,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 * @@ -11421,6 +11423,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, @@ -11428,7 +11431,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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html