Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: sys/uvideo: fix crash on close isochronous endpoint's webcam
To:
Marcus Glocker <marcus@nazgul.ch>
Cc:
tech@openbsd.org
Date:
Sat, 08 Mar 2025 08:53:27 +0100

Download raw body.

Thread
On Sat, 08 Mar 2025 08:26:29 +0100,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> On Sat, Mar 08, 2025 at 12:57:22AM GMT, Kirill A. Korinsky wrote:
> 
> > On Fri, 07 Mar 2025 21:39:30 +0100,
> > Marcus Glocker <marcus@nazgul.ch> wrote:
> > >
> > > On Thu, Mar 06, 2025 at 09:12:55AM GMT, Kirill A. Korinsky wrote:
> > >
> > > > tech@,
> > > >
> > > > I'd like to fix a crash in uvide when I detach my Elgato webcam when
> > > > it's streamming.
> > > >
> > > > uvideo_vs_close is called from videoclose and by VIDIOC_STREAMOFF. When
> > > > it's happened on detached webcam with isochronous endpoint, the system
> > > > crashes inside usbd_set_interface.
> > > >
> > > > Ok?
> > >
> > > Usually we're using usbd_is_dying() to return from a function at the
> > > very beginning.  Would it also work in your case to return from
> > > uvideo_vs_close() at the very beginning instead of only wrapping it
> > > around usbd_set_interface()?
> > >
> > 
> > Well, I assume that usually driver doesn't sleep before call set interface.
> > 
> > I had tried to move usbd_is_dying(sc->sc_udev) to the first line of
> > uvideo_vs_close, and it crashed inside usbd_set_interface().
> > 
> > And when I had moved it back, after sleep, it works.
> 
> I see.  Which indicates that at the beginning of uvideo_vs_close()
> usbd_is_dying() isn't set.  Probably the usb state converts to dying
> after the delay in your case.
> 
> I'm basically OK with that then, and it shouldn't have regression to
> other devices.
> 
> Just one style thing;  Can you please move the comment above the 'if'
> to avoid line break?

Sure, like this?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.249 uvideo.c
--- sys/dev/usb/uvideo.c	4 Mar 2025 22:59:01 -0000	1.249
+++ sys/dev/usb/uvideo.c	8 Mar 2025 07:52:31 -0000
@@ -2170,7 +2170,8 @@ uvideo_vs_close(struct uvideo_softc *sc)
 		usbd_delay_ms(sc->sc_udev, 100);
 
 		/* switch back to default interface (turns off cam LED) */
-		(void)usbd_set_interface(sc->sc_vs_cur->ifaceh, 0);
+		if (!usbd_is_dying(sc->sc_udev))
+			(void)usbd_set_interface(sc->sc_vs_cur->ifaceh, 0);
 	}
 }