Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751633AbbEYGbZ (ORCPT ); Mon, 25 May 2015 02:31:25 -0400 Received: from collab.rosalab.ru ([195.19.76.181]:50651 "EHLO collab.rosalab.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750843AbbEYGbU (ORCPT ); Mon, 25 May 2015 02:31:20 -0400 Message-ID: <5562C1B5.5050309@rosalab.ru> Date: Mon, 25 May 2015 09:31:17 +0300 From: Eugene Shatokhin Organization: ROSA User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Laurent Pinchart CC: Mauro Carvalho Chehab , LKML Subject: Re: uvcvideo: Race on dev->state between uvc_disconnect() and uvc_v4l2_open() References: <555C9EC9.40303@rosalab.ru> <3959395.nkzPWg6atK@avalon> In-Reply-To: <3959395.nkzPWg6atK@avalon> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3102 Lines: 83 25.05.2015 01:32, Laurent Pinchart пишет: > Hi Eugene, > > On Wednesday 20 May 2015 17:48:41 Eugene Shatokhin wrote: >> Hi, >> >> There is a race in uvcvideo module between uvc_disconnect() and >> uvc_v4l2_open() on dev->state. Checked and reproduced that with kernel >> 4.1-rc1. >> >> drivers/media/usb/uvc/uvc_driver.c, uvc_disconnect(): >> >> dev->state |= UVC_DEV_DISCONNECTED; >> >> drivers/media/usb/uvc/uvc_v4l2.c, uvc_v4l2_open(): >> >> if (stream->dev->state & UVC_DEV_DISCONNECTED) >> return -ENODEV; >> >> I checked that the race does happen by introducing a delay in >> uvc_disconnect() right before that assignment and armed a hardware >> breakpoint to detect the access to stream->dev->state from >> uvc_v4l2_open(). When I disconnected the webcam while Google Hangout was >> running, the hardware breakpoint triggered several times for that read >> in uvc_v4l2_open (uvc_v4l2.c:484). uvc_v4l2_open() was called in the >> context of GoogleTalkPlugin processes. >> >> Not sure if the race is intentional but I guess, better to report it >> anyway. Nothing has crashed during my (brief) testing yet, but still. > > The race condition between disconnect and open is unavoidable. What is > avoidable, though, is the crashes and other ill side-effects that could result > from it. The following commit handles that. > > commit ca9afe6f87b569cdf8e797395381f18ae23a2905 > Author: Hans Verkuil > Date: Fri Nov 26 06:54:53 2010 -0300 > > [media] v4l2-dev: fix race condition > > The unregister function had a race condition with the v4l2_open > function. Ensure that both functions test and clear the REGISTER > flag from within a critical section. > > Thanks to Laurent Pinchart for finding this race. > > Signed-off-by: Hans Verkuil > Signed-off-by: Mauro Carvalho Chehab > > > The race was previously handled by the uvcvideo driver, and the code got > removed in the following commit after the above commit got merged. > > commit 716fdee110ceb816cca8c46c0890d08c5a1addb9 > Author: Laurent Pinchart > Date: Tue Sep 29 21:07:19 2009 -0300 > > V4L/DVB (13152): uvcvideo: Rely on videodev to reference-count the device > > The uvcvideo driver has a driver-wide lock and a reference count to > protect > against a disconnect/open race. Now that videodev handles the race itself, > reference-counting in the driver can be removed. > > Signed-off-by: Laurent Pinchart > Signed-off-by: Mauro Carvalho Chehab > > > Setting the UVC_DEV_DISCONNECTED flag seems unneeded nowadays. I'll have to > carefully think about it though, and it's too late right now to do so :-) > I see. Thanks for the explanation! Regards, Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/