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

On Sat, Mar 08, 2025 at 08:53:27AM GMT, Kirill A. Korinsky wrote:

> 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?

ok mglocker@
 
> 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);
>  	}
>  }
>  
>