From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: sys/uvideo: prevent crashing on parsing isco packeg when stream is closed
To: tech@openbsd.org, mglocker@openbsd.org
Date: Thu, 13 Mar 2025 10:13:58 +0100

On 13/03/25(Thu) 09:59, Kirill A. Korinsky wrote:
> tech@,
> 
> I had discovered one more crash which can be triggered by an application
> when it sends ioctl(VIDIOC_STREAMOFF) into streaming isco webcam, or
> when the device is detached.
> 
> To avoid such crash I'd like to add requirements that pipe is still
> open, before parse header, or schedule a new isoc transfer.

All these checks look like workarounds to me.  Could you describe what
is happening?  Would you mind sharing the panic trace?  What race is
this diff working around?

> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -r1.251 uvideo.c
> --- sys/dev/usb/uvideo.c	10 Mar 2025 07:38:12 -0000	1.251
> +++ sys/dev/usb/uvideo.c	10 Mar 2025 22:01:42 -0000
> @@ -2291,7 +2291,7 @@ uvideo_vs_start_isoc_ixfer(struct uvideo
>  
>  	DPRINTF(2, "%s: %s\n", DEVNAME(sc), __func__);
>  
> -	if (usbd_is_dying(sc->sc_udev))
> +	if (usbd_is_dying(sc->sc_udev) || sc->sc_vs_cur->pipeh == NULL)
>  		return;
>  
>  	for (i = 0; i < sc->sc_nframes; i++)
> @@ -2321,6 +2321,9 @@ uvideo_vs_cb(struct usbd_xfer *xfer, voi
>  	struct uvideo_softc *sc = ixfer->sc;
>  	int len, i, frame_size;
>  	uint8_t *frame, *buf;
> +
> +	if (usbd_is_dying(sc->sc_udev) || sc->sc_vs_cur->pipeh == NULL)
> +		return;
>  
>  	DPRINTF(2, "%s: %s\n", DEVNAME(sc), __func__);
>  
>