From patchwork Tue Aug 16 23:49:41 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Rankin X-Patchwork-Id: 7620 Return-path: Envelope-to: mchehab@infradead.org Delivery-date: Tue, 16 Aug 2011 23:51:01 +0000 Received: from casper.infradead.org [85.118.1.10] by gaivota with IMAP (fetchmail-6.3.20) for (single-drop); Tue, 16 Aug 2011 16:51:10 -0700 (PDT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QtTPQ-0007dH-QY; Tue, 16 Aug 2011 23:51:01 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752191Ab1HPXtr (ORCPT + 1 other); Tue, 16 Aug 2011 19:49:47 -0400 Received: from nm2.bt.bullet.mail.ukl.yahoo.com ([217.146.183.200]:29775 "HELO nm2.bt.bullet.mail.ukl.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751824Ab1HPXtq (ORCPT ); Tue, 16 Aug 2011 19:49:46 -0400 Received: from [217.146.183.197] by nm2.bt.bullet.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 23:49:45 -0000 Received: from [217.146.183.205] by tm3.bt.bullet.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 23:49:45 -0000 Received: from [127.0.0.1] by omp1003.bt.mail.ukl.yahoo.com with NNFMP; 16 Aug 2011 23:49:45 -0000 X-Yahoo-Newman-Id: 60670.29168.bm@omp1003.bt.mail.ukl.yahoo.com Received: (qmail 77663 invoked from network); 16 Aug 2011 23:49:44 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.com; h=DKIM-Signature:X-Yahoo-Newman-Property:X-YMail-OSG:X-Yahoo-SMTP:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type; b=mkgOyYYmFTfxRnEOZjbNh6xHWyjVf1xHWz9aaOwgKJr+us8Ry/Mi/uPJ/EO5H+riHbrTO7ScDsgJg1Y/Xq8aevXPPMojRucF3DPekUFqScbXkbt6jT2vliIgX0Zh/77se2/vVG0iVhPpr1AiItBmmvyHHGTOu8IFB3kw1KDZ0zI= ; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s1024; t=1313538584; bh=RNJnl6u+eShVzSLlhzWjj5cQqxKD9lg8uES0aCgHiMo=; h=X-Yahoo-Newman-Property:X-YMail-OSG:X-Yahoo-SMTP:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type; b=etu8+RqejVAfVR96SEfIzCGUlJoVo5g4W6fG+7BzpZhOWs2S4vf99Qyl5PGSjxKOrkbLQBJn7cYRR0eAuWy4oQo1HXkutyfmQ78sOfek5Uq+0nQTRAjJgmmlg57sBDZ2c/uz5MNQwNSq9Zbn3OLX9eCLrfG8i7AeGdPF4cTeHds= X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: NrZvrhAVM1k.IPIbHgCO_HRWuLQKCZUzf.rxnJRO35crMR6 Q_WWKdTyIxyVbydgY0VlzoZBuUJyjqVc49UT0ulyPx95z7_lkWRayUPgJKL6 PBWhgt8tNHshJjEEREMQBvSJvBWoWxsE4Qxms_JIU7cc8836aJ4YcMtEhU9C D9CacoA6IsZ3VnCKc10AujfyyXy_wjBEWWjfa_nXTuirb_TNx4pGBJcizzjS PMJcjJPSZzKmb76ExpwXRMpozy911qOsQdZEPsorh_STFgktjFMsrdy3Hx2J U7XqA2PuaTjCDvRhJvXVI7WAZCEzu3BwJYdTxvd46TYT9m32CYBHdF6jMBFL noP0pHK4YNfQT2vwAKzS4UKl1zFX.EPxDJ4iAWsN94jlP58ZDADYxm8V7ERy aoRMh X-Yahoo-SMTP: dMK34oyswBBlfKesWTI5ovDjFOUFE6shtILt.ZXnUEjQHhWq Received: from wellhouse.underworld (rankincj@86.178.152.254 with login) by smtp822.mail.ird.yahoo.com with SMTP; 16 Aug 2011 16:49:44 -0700 PDT Received: from volcano.underworld (volcano.underworld [192.168.0.3]) by wellhouse.underworld (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id p7GNnfic032580 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT) for ; Wed, 17 Aug 2011 00:49:43 +0100 Message-ID: <4E4B0215.7080905@yahoo.com> Date: Wed, 17 Aug 2011 00:49:41 +0100 From: Chris Rankin User-Agent: Mozilla/5.0 (X11; Linux i686; rv:5.0) Gecko/20110707 Thunderbird/5.0 MIME-Version: 1.0 To: linux-media@vger.kernel.org Subject: [PATCH] Fix locking problem, race conditions and resource leaks in em28xx, em28xx-dvb Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi, Here's the latest draft of my patch (against 3.0). Any feedback greatly appreciated. Cheers, Chris --- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-16 19:49:58.000000000 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-16 21:46:35.000000000 +0100 @@ -1160,10 +1160,9 @@ static DEFINE_MUTEX(em28xx_devlist_mutex); /* - * em28xx_realease_resources() - * unregisters the v4l2,i2c and usb devices - * called when the device gets disconected or at module unload -*/ + * em28xx_remove_from_devlist() + * Removes the device from the list of active devices. + */ void em28xx_remove_from_devlist(struct em28xx *dev) { mutex_lock(&em28xx_devlist_mutex); @@ -1171,13 +1170,6 @@ mutex_unlock(&em28xx_devlist_mutex); }; -void em28xx_add_into_devlist(struct em28xx *dev) -{ - mutex_lock(&em28xx_devlist_mutex); - list_add_tail(&dev->devlist, &em28xx_devlist); - mutex_unlock(&em28xx_devlist_mutex); -}; - /* * Extension interface */ @@ -1193,8 +1185,8 @@ list_for_each_entry(dev, &em28xx_devlist, devlist) { ops->init(dev); } - printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name); mutex_unlock(&em28xx_devlist_mutex); + printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name); return 0; } EXPORT_SYMBOL(em28xx_register_extension); @@ -1207,9 +1199,9 @@ list_for_each_entry(dev, &em28xx_devlist, devlist) { ops->fini(dev); } - printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name); list_del(&ops->next); mutex_unlock(&em28xx_devlist_mutex); + printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name); } EXPORT_SYMBOL(em28xx_unregister_extension); @@ -1218,11 +1210,10 @@ struct em28xx_ops *ops = NULL; mutex_lock(&em28xx_devlist_mutex); - if (!list_empty(&em28xx_extension_devlist)) { - list_for_each_entry(ops, &em28xx_extension_devlist, next) { - if (ops->init) - ops->init(dev); - } + list_add_tail(&dev->devlist, &em28xx_devlist); + list_for_each_entry(ops, &em28xx_extension_devlist, next) { + if (ops->init) + ops->init(dev); } mutex_unlock(&em28xx_devlist_mutex); } --- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-16 21:28:25.000000000 +0100 +++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-16 23:10:06.000000000 +0100 @@ -2738,9 +2738,9 @@ #endif /* CONFIG_MODULES */ /* - * em28xx_realease_resources() + * em28xx_release_resources() * unregisters the v4l2,i2c and usb devices - * called when the device gets disconected or at module unload + * called when the device gets disconnected or at module unload */ void em28xx_release_resources(struct em28xx *dev) { @@ -2776,7 +2776,6 @@ { struct em28xx *dev = *devhandle; int retval; - int errCode; dev->udev = udev; mutex_init(&dev->ctrl_urb_lock); @@ -2872,12 +2871,11 @@ } /* register i2c bus */ - errCode = em28xx_i2c_register(dev); - if (errCode < 0) { - v4l2_device_unregister(&dev->v4l2_dev); + retval = em28xx_i2c_register(dev); + if (retval < 0) { em28xx_errdev("%s: em28xx_i2c_register - errCode [%d]!\n", - __func__, errCode); - return errCode; + __func__, retval); + goto fail_reg_i2c; } /* @@ -2891,11 +2889,11 @@ em28xx_card_setup(dev); /* Configure audio */ - errCode = em28xx_audio_setup(dev); - if (errCode < 0) { - v4l2_device_unregister(&dev->v4l2_dev); + retval = em28xx_audio_setup(dev); + if (retval < 0) { em28xx_errdev("%s: Error while setting audio - errCode [%d]!\n", - __func__, errCode); + __func__, retval); + goto fail_setup_audio; } /* wake i2c devices */ @@ -2909,31 +2907,28 @@ if (dev->board.has_msp34xx) { /* Send a reset to other chips via gpio */ - errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7); - if (errCode < 0) { - em28xx_errdev("%s: em28xx_write_regs_req - " + retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7); + if (retval < 0) { + em28xx_errdev("%s: em28xx_write_reg - " "msp34xx(1) failed! errCode [%d]\n", - __func__, errCode); - return errCode; + __func__, retval); + goto fail_write_reg; } msleep(3); - errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff); - if (errCode < 0) { - em28xx_errdev("%s: em28xx_write_regs_req - " + retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff); + if (retval < 0) { + em28xx_errdev("%s: em28xx_write_reg - " "msp34xx(2) failed! errCode [%d]\n", - __func__, errCode); - return errCode; + __func__, retval); + goto fail_write_reg; } msleep(3); } - em28xx_add_into_devlist(dev); - retval = em28xx_register_analog_devices(dev); if (retval < 0) { - em28xx_release_resources(dev); - goto fail_reg_devices; + goto fail_reg_analog_devices; } em28xx_init_extension(dev); @@ -2943,7 +2938,14 @@ return 0; -fail_reg_devices: +fail_setup_audio: +fail_write_reg: +fail_reg_analog_devices: + em28xx_i2c_unregister(dev); + +fail_reg_i2c: + v4l2_device_unregister(&dev->v4l2_dev); + return retval; } @@ -2979,7 +2981,6 @@ ifnum, interface->altsetting[0].desc.bInterfaceClass); - em28xx_devused &= ~(1<= EM28XX_MAXBOARDS) { printk(DRIVER_NAME ": Supports only %i em28xx boards.\n", EM28XX_MAXBOARDS); - em28xx_devused &= ~(1<name, 29, "em28xx #%d", nr); + snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); dev->devno = nr; dev->model = id->driver_info; dev->alt = -1; @@ -3107,7 +3104,6 @@ if (dev->alt_max_pkt_size == NULL) { em28xx_errdev("out of memory!\n"); - em28xx_devused &= ~(1<lock); retval = em28xx_init_dev(&dev, udev, interface, nr); if (retval) { - em28xx_devused &= ~(1<devno); mutex_unlock(&dev->lock); + kfree(dev->alt_max_pkt_size); kfree(dev); goto err; } @@ -3146,6 +3142,8 @@ return 0; err: + em28xx_devused &= ~(1<dvb = dvb; dvb->fe[0] = dvb->fe[1] = NULL; - mutex_lock(&dev->lock); em28xx_set_mode(dev, EM28XX_DIGITAL_MODE); /* init frontend */ switch (dev->model) { @@ -711,7 +710,6 @@ em28xx_info("Successfully loaded em28xx-dvb\n"); ret: em28xx_set_mode(dev, EM28XX_SUSPEND); - mutex_unlock(&dev->lock); return result; out_free: