On (23/11/21 19:53), Ricardo Ribalda wrote: > The call to uvc_disconnect() is not protected by any mutex. > This means it can and will be called while other accesses to the video > device are in progress. This can cause all kinds of race conditions, > including crashes such as the following. > [..] > > Call Trace: > usb_hcd_alloc_bandwidth+0x1ee/0x30f > usb_set_interface+0x1a3/0x2b7 > uvc_video_start_transfer+0x29b/0x4b8 [uvcvideo] > uvc_video_start_streaming+0x91/0xdd [uvcvideo] > uvc_start_streaming+0x28/0x5d [uvcvideo] > vb2_start_streaming+0x61/0x143 [videobuf2_common] > vb2_core_streamon+0xf7/0x10f [videobuf2_common] > uvc_queue_streamon+0x2e/0x41 [uvcvideo] > uvc_ioctl_streamon+0x42/0x5c [uvcvideo] > __video_do_ioctl+0x33d/0x42a > video_usercopy+0x34e/0x5ff > ? video_ioctl2+0x16/0x16 > v4l2_ioctl+0x46/0x53 > do_vfs_ioctl+0x50a/0x76f > ksys_ioctl+0x58/0x83 > __x64_sys_ioctl+0x1a/0x1e > do_syscall_64+0x54/0xde > > usb_set_interface() should not be called after the USB device has been > unregistered. However, in the above case the disconnect happened after > v4l2_ioctl() was called, but before the call to usb_ifnum_to_if(). > > Acquire various mutexes in uvc_unregister_video() to fix the majority > (maybe all) of the observed race conditions. > > The uvc_device lock prevents races against suspend and resume calls > and the poll function. > > The uvc_streaming lock prevents races against stream related functions; > for the most part, those are ioctls. This lock also requires other > functions using this lock to check if a video device is still registered > after acquiring it. For example, it was observed that the video device > was already unregistered by the time the stream lock was acquired in > uvc_ioctl_streamon(). > > The uvc_queue lock prevents races against queue functions, Most of > those are already protected by the uvc_streaming lock, but some > are called directly. This is done as added protection; an actual race > was not (yet) observed. > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Cc: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> > Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx> > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx> Reviewed-by: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
