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: 6718541 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 9E5999F319 for ; Sun, 5 Jul 2015 16:49:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C30C520645 for ; Sun, 5 Jul 2015 16:49:50 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id D27892063F for ; Sun, 5 Jul 2015 16:49:49 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 77AB96E0D2; Sun, 5 Jul 2015 09:49:49 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mailout1.hostsharing.net (mailout1.hostsharing.net [83.223.95.204]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7F12C6E0D2 for ; Sun, 5 Jul 2015 09:49:48 -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 mailout1.hostsharing.net (Postfix) with ESMTPS id B532010391B3A; Sun, 5 Jul 2015 18:49:46 +0200 (CEST) Received: from localhost (5-38-90-81.adsl.cmo.de [81.90.38.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 038D6600BEA2; Sun, 5 Jul 2015 18:49:44 +0200 (CEST) X-Mailbox-Line: From 874c025b04129f1ca71a66be4568258fc5b6c133 Mon Sep 17 00:00:00 2001 Message-Id: <874c025b04129f1ca71a66be4568258fc5b6c133.1436041426.git.lukas@wunner.de> In-Reply-To: <6ce2e2be9d2d19a3fb740a017f9f41de30dd9493.1436041426.git.lukas@wunner.de> References: <20150704123148.GC5312@nuc-i3427.alporthouse.com>, <6ce2e2be9d2d19a3fb740a017f9f41de30dd9493.1436041426.git.lukas@wunner.de> From: Lukas Wunner Date: Sat, 4 Jul 2015 11:50:58 +0200 MIME-Version: 1.0 To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v4 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=-3.4 required=5.0 tests=BAYES_00, DATE_IN_PAST_24_48, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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. 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ä Cc: Daniel Vetter --- 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 9079fcd..d597afa 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8876,20 +8876,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); } @@ -8929,6 +8926,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 }; @@ -8943,7 +8941,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 * @@ -13379,6 +13381,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, @@ -13386,7 +13389,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)