From patchwork Fri Aug 2 12:27:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sylwester Nawrocki X-Patchwork-Id: 19509 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from ) id 1V5EST-0001rd-E2; Fri, 02 Aug 2013 14:27:49 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-6) with esmtp id 1V5ESR-0007sC-4C; Fri, 02 Aug 2013 14:27:49 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752076Ab3HBM1p (ORCPT + 1 other); Fri, 2 Aug 2013 08:27:45 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:37892 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752018Ab3HBM1o (ORCPT ); Fri, 2 Aug 2013 08:27:44 -0400 Received: from epcpsbgm1.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MQW0044VLA28SZ0@mailout2.samsung.com> for linux-media@vger.kernel.org; Fri, 02 Aug 2013 21:27:43 +0900 (KST) X-AuditID: cbfee61a-b7f196d000007dfa-99-51fba5bf9e0d Received: from epmmp1.local.host ( [203.254.227.16]) by epcpsbgm1.samsung.com (EPCPMTA) with SMTP id 9B.E7.32250.FB5ABF15; Fri, 02 Aug 2013 21:27:43 +0900 (KST) Received: from amdc1344.digital.local ([106.116.147.32]) by mmp1.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MQW004TKL9WYY70@mmp1.samsung.com>; Fri, 02 Aug 2013 21:27:43 +0900 (KST) From: Sylwester Nawrocki To: linux-media@vger.kernel.org Cc: hverkuil@xs4all.nl, laurent.pinchart@ideasonboard.com, Sylwester Nawrocki , Kyungmin Park Subject: [PATCH] V4L: Drop meaningless video_is_registered() call in v4l2_open() Date: Fri, 02 Aug 2013 14:27:29 +0200 Message-id: <1375446449-27066-1-git-send-email-s.nawrocki@samsung.com> X-Mailer: git-send-email 1.7.9.5 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDJMWRmVeSWpSXmKPExsVy+t9jAd39S38HGixvUbQ4NfkZk8XZpjfs Fp0Tl7Bb9GzYympx+E07qwOrx+yOmawefVtWMXp83iTncerrZ/YAligum5TUnMyy1CJ9uwSu jK7e5ywFPeIVT95OZm5gvC3QxcjJISFgItHX+IMRwhaTuHBvPRuILSSwiFFi+grJLkYuILuD SeLBrYXsIAk2AUOJ3qN9YA0iAvIST3pvsIEUMQtMZZRY3fmKGSQhLBAg8X3tbbAGFgFViWWz F4NN5RVwk5g+cTNLFyMH0DYFiTmTbCYwci9gZFjFKJpakFxQnJSea6hXnJhbXJqXrpecn7uJ ERwGz6R2MK5ssDjEKMDBqMTD+yDzV6AQa2JZcWXuIUYJDmYlEd4/s4FCvCmJlVWpRfnxRaU5 qcWHGKU5WJTEeQ+0WgcKCaQnlqRmp6YWpBbBZJk4OKUaGDtyr58VmfFU++/HGI6V639n7mGQ Vlq9beKTnosNvPcXLTug8EYjkLegIH/5caadjx0duw4Yan93DH/szLubd0PdSj3Zg9P63mxT 4svi8FA/EOJ88X3/D8+XfjkSexmcXeeHXrZ702zy4l/qZDnF1ZMS5Ho8TCcqOvIrzMxx/xr4 /9rpdtW6l0osxRmJhlrMRcWJAGdpnVL/AQAA Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2013.8.2.121823 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_3000_3999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' As it currently stands this code doesn't protect against any races between video device open() and its unregistration. Races could be avoided by doing the video_is_registered() check protected by the core mutex, while the video device unregistration is also done with this mutex held. The history of this code is that the second video_is_registered() call has been added in commit ee6869afc922a9849979e49bb3bbcad7948 "V4L/DVB: v4l2: add core serialization lock" together with addition of the core mutex support in fops: mutex_unlock(&videodev_lock); - if (vdev->fops->open) - ret = vdev->fops->open(filp); + if (vdev->fops->open) { + if (vdev->lock) + mutex_lock(vdev->lock); + if (video_is_registered(vdev)) + ret = vdev->fops->open(filp); + else + ret = -ENODEV; + if (vdev->lock) + mutex_unlock(vdev->lock); + } While commit cf5337358548b813479b58478539fc20ee86556c "[media] v4l2-dev: remove V4L2_FL_LOCK_ALL_FOPS" removed only code touching the mutex: mutex_unlock(&videodev_lock); if (vdev->fops->open) { - if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags) && - mutex_lock_interruptible(vdev->lock)) { - ret = -ERESTARTSYS; - goto err; - } if (video_is_registered(vdev)) ret = vdev->fops->open(filp); else ret = -ENODEV; - if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags)) - mutex_unlock(vdev->lock); } Remove the remaining video_is_registered() call as it doesn't provide any real protection and just adds unnecessary overhead. The drivers need to perform the unregistration check themselves inside their file operation handlers, while holding respective mutex. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park --- drivers/media/v4l2-core/v4l2-dev.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index c8859d6..1743119 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -444,13 +444,9 @@ static int v4l2_open(struct inode *inode, struct file *filp) /* and increase the device refcount */ video_get(vdev); mutex_unlock(&videodev_lock); - if (vdev->fops->open) { - if (video_is_registered(vdev)) - ret = vdev->fops->open(filp); - else - ret = -ENODEV; - } + if (vdev->fops->open) + ret = vdev->fops->open(filp); if (vdev->debug) printk(KERN_DEBUG "%s: open (%d)\n", video_device_node_name(vdev), ret);