Re: [PATCH 6.2.y] media: uvcvideo: Fix race condition with usb_kill_urb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Greg



On Fri, 10 Mar 2023 at 13:17, Greg KH <greg@xxxxxxxxx> wrote:
>
> On Fri, Mar 10, 2023 at 12:54:48PM +0100, Greg KH wrote:
> > On Wed, Mar 08, 2023 at 02:30:25PM +0100, Ricardo Ribalda wrote:
> > > usb_kill_urb warranties that all the handlers are finished when it
> > > returns, but does not protect against threads that might be handling
> > > asynchronously the urb.
> > >
> > > For UVC, the function uvc_ctrl_status_event_async() takes care of
> > > control changes asynchronously.
> > >
> > > If the code is executed in the following order:
> > >
> > > CPU 0                                       CPU 1
> > > =====                                       =====
> > > uvc_status_complete()
> > >                                     uvc_status_stop()
> > > uvc_ctrl_status_event_work()
> > >                                     uvc_status_start() -> FAIL
> > >
> > > Then uvc_status_start will keep failing and this error will be shown:
> > >
> > > <4>[    5.540139] URB 0000000000000000 submitted while active
> > > drivers/usb/core/urb.c:378 usb_submit_urb+0x4c3/0x528
> > >
> > > Let's improve the current situation, by not re-submiting the urb if
> > > we are stopping the status event. Also process the queued work
> > > (if any) during stop.
> > >
> > > CPU 0                                       CPU 1
> > > =====                                       =====
> > > uvc_status_complete()
> > >                                     uvc_status_stop()
> > >                                     uvc_status_start()
> > > uvc_ctrl_status_event_work() -> FAIL
> > >
> > > Hopefully, with the usb layer protection this should be enough to cover
> > > all the cases.
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
> > > Reviewed-by: Yunke Cao <yunkec@xxxxxxxxxxxx>
> > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > (cherry picked from commit 619d9b710cf06f7a00a17120ca92333684ac45a8)
> > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > > ---
> > >  drivers/media/usb/uvc/uvc_ctrl.c   |  5 ++++
> > >  drivers/media/usb/uvc/uvc_status.c | 37 ++++++++++++++++++++++++++++++
> > >  drivers/media/usb/uvc/uvcvideo.h   |  1 +
> > >  3 files changed, 43 insertions(+)
> >
> > This fails to apply to the 6.2.y queue right now:
> >
> > checking file drivers/media/usb/uvc/uvc_ctrl.c
> > Hunk #1 FAILED at 6.
> > Hunk #2 succeeded at 1509 (offset 67 lines).
> > 1 out of 2 hunks FAILED
> > checking file drivers/media/usb/uvc/uvc_status.c
> > checking file drivers/media/usb/uvc/uvcvideo.h
> > Hunk #1 succeeded at 559 (offset -1 lines).
> >
> > Can you redo this?
>
> I got 5.15.y and newer to work here on my own, can you redo the older
> ones as well?

Sure thanks!

>
> thanks,
>
> greg k-h



-- 
Ricardo Ribalda



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux