From patchwork Thu Sep 10 15:30:03 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Corbet X-Patchwork-Id: 46622 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n8AFUh0K003412 for ; Thu, 10 Sep 2009 15:30:43 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751895AbZIJPaD (ORCPT ); Thu, 10 Sep 2009 11:30:03 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751840AbZIJPaD (ORCPT ); Thu, 10 Sep 2009 11:30:03 -0400 Received: from vena.lwn.net ([206.168.112.25]:44005 "EHLO vena.lwn.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750934AbZIJPaC (ORCPT ); Thu, 10 Sep 2009 11:30:02 -0400 Received: from bike.lwn.net (jon-dsl [199.45.162.133]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by vena (Postfix) with ESMTPSA id 14D4B16BD7A; Thu, 10 Sep 2009 09:30:05 -0600 (MDT) Date: Thu, 10 Sep 2009 09:30:03 -0600 From: Jonathan Corbet To: iceberg Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] fix lock imbalances in /drivers/media/video/cafe_ccic.c Message-ID: <20090910093003.194c300f@bike.lwn.net> In-Reply-To: <200909101837.34472.strakh@ispras.ru> References: <200909101837.34472.strakh@ispras.ru> Organization: LWN.net X-Mailer: Claws Mail 3.7.2 (GTK+ 2.16.5; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Thu, 10 Sep 2009 18:37:34 +0000 iceberg wrote: > In ./drivers/media/video/cafe_ccic.c, in function cafe_pci_probe: > Mutex must be unlocked before exit > 1. On paths starting with mutex lock in line 1912, then continuing in lines: > 1929, 1936 (goto unreg) and 1940 (goto iounmap) . > 2. On path starting in line 1971 mutex lock, and then continuing in line 1978 > (goto out_smbus) mutex. That's a definite bug, but I hate all those unlocks in the error branches. As it happens, we don't really need the mutex until the device has been exposed to the rest of the kernel, so I propose the following as a better patch. Thanks for pointing this out, jon --- Fix a mutex leak Certain error exits from cafe_pci_probe() can leave the camera mutex locked. For much of the time, we didn't need the mutex anyway; take it out and add an unlock in the path where it is needed. Reported-by: Alexander Strakh Signed-off-by: Jonathan Corbet --- drivers/media/video/cafe_ccic.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c index c4d181d..0f62b5e 100644 --- a/drivers/media/video/cafe_ccic.c +++ b/drivers/media/video/cafe_ccic.c @@ -1909,7 +1909,6 @@ static int cafe_pci_probe(struct pci_dev *pdev, goto out_free; mutex_init(&cam->s_mutex); - mutex_lock(&cam->s_mutex); spin_lock_init(&cam->dev_lock); cam->state = S_NOTREADY; cafe_set_config_needed(cam, 1); @@ -1949,7 +1948,6 @@ static int cafe_pci_probe(struct pci_dev *pdev, * because the sensor could attach in this call chain, leading to * unsightly deadlocks. */ - mutex_unlock(&cam->s_mutex); /* attach can deadlock */ ret = cafe_smbus_setup(cam); if (ret) goto out_freeirq; @@ -1991,6 +1989,7 @@ static int cafe_pci_probe(struct pci_dev *pdev, return 0; out_smbus: + mutex_unlock(&cam->s_mutex); cafe_smbus_shutdown(cam); out_freeirq: cafe_ctlr_power_down(cam);