Download raw body.
sys/uvideo: add missed lock in uvideo_vs_start_isoc_ixfer
tech@,
uvideo supports two modes of webcam reading, bulk and non-bulk. In bulk
mode it wraps by locks usbd_transfer to prevent call usbd_close_pipe on
used pipe, and in case of non-bulk, no lock here.
When close had happened, it may lead to crash if non-bulk decided to
re-setup usbd_transfer in parallel thread.
Here is the thread https://marc.info/?t=173228582400007&r=1&w=2 where I
encountered this crash, and this patch fixes it.
Feedback? Ok?
Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.222 uvideo.c
--- sys/dev/usb/uvideo.c 1 Sep 2024 03:09:00 -0000 1.222
+++ sys/dev/usb/uvideo.c 25 Nov 2024 19:13:11 -0000
@@ -1933,15 +1933,18 @@ uvideo_vs_open(struct uvideo_softc *sc)
void
uvideo_vs_close(struct uvideo_softc *sc)
{
- if (sc->sc_vs_cur->bulk_running == 1) {
+ struct usbd_pipe *pipeh;
+
+ if (sc->sc_vs_cur->bulk_running == 1)
sc->sc_vs_cur->bulk_running = 0;
- usbd_ref_wait(sc->sc_udev);
- }
- if (sc->sc_vs_cur->pipeh) {
- usbd_close_pipe(sc->sc_vs_cur->pipeh);
- sc->sc_vs_cur->pipeh = NULL;
- }
+ pipeh = sc->sc_vs_cur->pipeh;
+ sc->sc_vs_cur->pipeh = NULL;
+
+ usbd_ref_wait(sc->sc_udev);
+
+ if (pipeh)
+ usbd_close_pipe(pipeh);
/*
* Some devices need time to shutdown before we switch back to
@@ -2063,6 +2066,11 @@ uvideo_vs_start_isoc_ixfer(struct uvideo
for (i = 0; i < sc->sc_nframes; i++)
ixfer->size[i] = sc->sc_vs_cur->psize;
+ usbd_ref_incr(sc->sc_udev);
+
+ if (sc->sc_vs_cur->pipeh == NULL)
+ goto skip;
+
usbd_setup_isoc_xfer(
ixfer->xfer,
sc->sc_vs_cur->pipeh,
@@ -2077,6 +2085,9 @@ uvideo_vs_start_isoc_ixfer(struct uvideo
DPRINTF(1, "%s: usbd_transfer error=%s!\n",
DEVNAME(sc), usbd_errstr(error));
}
+
+skip:
+ usbd_ref_decr(sc->sc_udev);
}
void
--
wbr, Kirill
sys/uvideo: add missed lock in uvideo_vs_start_isoc_ixfer