On 28/02/2024 09:45, Hans Verkuil wrote: > On 20/02/2024 09:06, Benjamin Gaignard wrote: >> Remove locks calls in usbtv_video_free() because >> are useless and may led to a deadlock as reported here: >> https://syzkaller.appspot.com/x/bisect.txt?x=166dc872180000 >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx> >> Fixes: c838530d230b ("media: media videobuf2: Be more flexible on the number of queue stored buffers") > > Hmm, the code was really always wrong, it is just that before this commit > it would only fail if you did a disconnect while streaming, and now it also > fails if you disconnect when you are not streaming, so it is much more > noticeable now. > > That should probably be explained better in the commit log. You could add a > second Fixes line: > > Fixes: f3d27f34fdd7 ("[media] usbtv: Add driver for Fushicai USBTV007 video frame grabber") > > but I am not sure how useful that is. > >> --- >> drivers/media/usb/usbtv/usbtv-video.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c >> index 62a583040cd4..96276358d116 100644 >> --- a/drivers/media/usb/usbtv/usbtv-video.c >> +++ b/drivers/media/usb/usbtv/usbtv-video.c >> @@ -963,15 +963,9 @@ int usbtv_video_init(struct usbtv *usbtv) >> >> void usbtv_video_free(struct usbtv *usbtv) >> { >> - mutex_lock(&usbtv->vb2q_lock); >> - mutex_lock(&usbtv->v4l2_lock); >> - >> usbtv_stop(usbtv); > > This call should also be dropped... > >> vb2_video_unregister_device(&usbtv->vdev); > > ...since this function will call stop_streaming if streaming is in progress, > which will call usbtv_stop with all the correct locks held. With that change you can add my: Tested-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> Regards, Hans > > Regards, > > Hans > >> v4l2_device_disconnect(&usbtv->v4l2_dev); >> >> - mutex_unlock(&usbtv->v4l2_lock); >> - mutex_unlock(&usbtv->vb2q_lock); >> - >> v4l2_device_put(&usbtv->v4l2_dev); >> } > >
