From patchwork Sat Jul 4 09:50:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 7123781 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5A48C9F380 for ; Fri, 4 Sep 2015 14:01:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6235520812 for ; Fri, 4 Sep 2015 14:01:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 6B0D7207F9 for ; Fri, 4 Sep 2015 14:01:08 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EC8387A19B; Fri, 4 Sep 2015 07:01:07 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mailout2.hostsharing.net (mailout2.hostsharing.net [83.223.90.233]) by gabe.freedesktop.org (Postfix) with ESMTPS id 703507A19B for ; Fri, 4 Sep 2015 07:01:06 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout2.hostsharing.net (Postfix) with ESMTPS id 1DA331016D57C; Fri, 4 Sep 2015 16:01:04 +0200 (CEST) Received: from localhost (6-38-90-81.adsl.cmo.de [81.90.38.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 1E4F5603D600; Fri, 4 Sep 2015 16:01:01 +0200 (CEST) X-Mailbox-Line: From 521e48fc5fc8d211ed2847070120ff4032b7a383 Mon Sep 17 00:00:00 2001 Message-Id: <521e48fc5fc8d211ed2847070120ff4032b7a383.1441363361.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 4 Jul 2015 11:50:58 +0200 MIME-Version: 1.0 To: intel-gfx@lists.freedesktop.org Cc: Jani Nikula Subject: [Intel-gfx] [PATCH v5 2/2] drm/i915: On fb alloc failure, unref gem object where it gets refed X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-0.8 required=5.0 tests=BAYES_00, DATE_IN_PAST_96_XX, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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.] Tested-by: Pierre Moreau [MBP 5,3 2009 nvidia 9400M + 9600M GT pre-retina] Tested-by: Paul Hordiienko [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer [MBP 11,3 2013 intel HSW + nvidia GK107 retina] Signed-off-by: Lukas Wunner Fixes: a8bb6818270c ("drm/i915: Fix error path leak in fbdev fb allocation") Cc: Chris Wilson Cc: Tvrtko Ursulin Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä --- 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 421ac15..68d051d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10033,20 +10033,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); } @@ -10086,6 +10083,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 }; @@ -10100,7 +10098,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 * @@ -14289,6 +14291,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, @@ -14296,7 +14299,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_FBDEV_EMULATION