Messages in this thread | | | Date | Mon, 25 May 2015 09:31:17 +0300 | From | Eugene Shatokhin <> | Subject | Re: uvcvideo: Race on dev->state between uvc_disconnect() and uvc_v4l2_open() |
| |
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 <hverkuil@xs4all.nl> > 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 <hverkuil@xs4all.nl> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > > 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 <laurent.pinchart@ideasonboard.com> > 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 <laurent.pinchart@ideasonboard.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com> > > > 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
| |