From patchwork Wed Oct 14 09:52:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 7391681 Return-Path: X-Original-To: patchwork-dri-devel@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 8F0B89F1B9 for ; Wed, 14 Oct 2015 09:52:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B156D207D2 for ; Wed, 14 Oct 2015 09:52:19 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id EA723207C9 for ; Wed, 14 Oct 2015 09:52:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4ED456EBF6; Wed, 14 Oct 2015 02:52:17 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id B97A16EBF6 for ; Wed, 14 Oct 2015 02:52:15 -0700 (PDT) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 46592988-1500048 for multiple; Wed, 14 Oct 2015 10:52:15 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Wed, 14 Oct 2015 10:52:02 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm/prime: Move all unreferences on fd_to_handle error paths to after unlock Date: Wed, 14 Oct 2015 10:52:00 +0100 Message-Id: <1444816320-3038-1-git-send-email-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.6.1 In-Reply-To: References: X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Cc: Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_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 If we need to take the error path and drop the references to the objects and handle we created earlier, there is a possibility that we then free the object and then attempt to free any associated PRIME resources under the prime mutex. If we are holding the prime mutex when we attempt to free the handle/object, we risk a recursive deadlock. [ 677.932957] ============================================= [ 677.932957] [ INFO: possible recursive locking detected ] [ 677.932957] 4.3.0-rc5-virtio-gpu+ #11 Not tainted [ 677.932957] --------------------------------------------- [ 677.932957] Xorg/2661 is trying to acquire lock: [ 677.932957] (&prime_fpriv->lock){+.+.+.}, at: [] drm_gem_remove_prime_handles.isra.7+0x20/0x50 [drm] [ 677.932957] but task is already holding lock: [ 677.932957] (&prime_fpriv->lock){+.+.+.}, at: [] drm_gem_prime_fd_to_handle+0x4b/0x240 [drm] Reported-by: Dave Airlie Signed-off-by: Chris Wilson Cc: Daniel Vetter Cc: Dave Airlie --- drivers/gpu/drm/drm_prime.c | 47 ++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 27aa7183b20b..1bdd69e1ef43 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -564,26 +564,26 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, uint32_t *handle) { struct dma_buf *dma_buf; - struct drm_gem_object *obj; + struct drm_gem_object *obj = NULL; int ret; + *handle = 0; + dma_buf = dma_buf_get(prime_fd); if (IS_ERR(dma_buf)) return PTR_ERR(dma_buf); mutex_lock(&file_priv->prime.lock); - ret = drm_prime_lookup_buf_handle(&file_priv->prime, - dma_buf, handle); + ret = drm_prime_lookup_buf_handle(&file_priv->prime, dma_buf, handle); if (ret == 0) - goto out_put; + goto out; /* never seen this one, need to import */ - mutex_lock(&dev->object_name_lock); obj = dev->driver->gem_prime_import(dev, dma_buf); if (IS_ERR(obj)) { ret = PTR_ERR(obj); - goto out_unlock; + goto out; } if (obj->dma_buf) { @@ -593,33 +593,24 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, get_dma_buf(dma_buf); } - /* drm_gem_handle_create_tail unlocks dev->object_name_lock. */ - ret = drm_gem_handle_create_tail(file_priv, obj, handle); - drm_gem_object_unreference_unlocked(obj); + ret = drm_gem_handle_create(file_priv, obj, handle); if (ret) - goto out_put; + goto out; - ret = drm_prime_add_buf_handle(&file_priv->prime, - dma_buf, *handle); - if (ret) - goto fail; + ret = drm_prime_add_buf_handle(&file_priv->prime, dma_buf, *handle); +out: mutex_unlock(&file_priv->prime.lock); - - dma_buf_put(dma_buf); - - return 0; - -fail: - /* hmm, if driver attached, we are relying on the free-object path - * to detach.. which seems ok.. - */ - drm_gem_handle_delete(file_priv, *handle); -out_unlock: - mutex_unlock(&dev->object_name_lock); -out_put: + if (ret) { + /* hmm, if driver attached, we are relying on the + * free-object path to detach.. which seems ok.. + */ + if (*handle) + drm_gem_handle_delete(file_priv, *handle); + } + if (!IS_ERR_OR_NULL(obj)) + drm_gem_object_unreference_unlocked(obj); dma_buf_put(dma_buf); - mutex_unlock(&file_priv->prime.lock); return ret; } EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);