Re: [PATCH] media: gspca: finepix: flush the worker before URBs are freed

[Frank Zago]

Hello,

I used the reproducer on a working camera, and indeed the oops was present. After applying the patch, the kernel wouldn't crash anymore.

I see there's a pending patch to remove the whole gspca driver, but anyway, for this fix:

Acked-by frank zago <frank@xxxxxxxx>

Regards,
  Frank.

On 9/10/26 7:21 PM, Farhad Alemi wrote:
> dostream() dereferences gspca_dev->urb[0]->transfer_buffer on entry, but
> finepix flushes its worker only from .stop0, which gspca_stream_off() runs
> after destroy_urbs() has already killed, freed and cleared urb[0], while
> gspca_disconnect() frees the URBs without invoking any subdriver stop
> callback at all. A worker that starts running inside either window faults
> on a NULL urb. Flush the work from .stopN, which runs before
> destroy_urbs(), and add a finepix .disconnect handler that clears ->present
> and flushes the worker before handing over to gspca_disconnect().
> 
> Closes: https://lore.kernel.org/all/CA+0ovCiqm0MVi55RpNtbUQoOjiSxnbjHXGN9rq+P6aJJCqCguQ@xxxxxxxxxxxxxx/
> Signed-off-by: Farhad Alemi <farhad.alemi@xxxxxxxxxxxx>
> ---
> The device was emulated.
> 
> --- a/drivers/media/usb/gspca/finepix.c
> +++ b/drivers/media/usb/gspca/finepix.c
> @@ -217,9 +217,9 @@ static int sd_start(struct gspca_dev *gspca_dev)
>  	return 0;
>  }
> 
> -/* called on streamoff with alt==0 and on disconnect */
> +/* called before destroy_urbs(): the worker must not outlive urb[0] */
>  /* the usb_lock is held at entry - restore on exit */
> -static void sd_stop0(struct gspca_dev *gspca_dev)
> +static void sd_stopN(struct gspca_dev *gspca_dev)
>  {
>  	struct usb_fpix *dev = (struct usb_fpix *) gspca_dev;
> 
> @@ -229,6 +229,17 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
>  	mutex_lock(&gspca_dev->usb_lock);
>  }
> 
> +/* gspca_disconnect() frees the URBs without calling any subdriver callback */
> +static void sd_disconnect(struct usb_interface *intf)
> +{
> +	struct usb_fpix *dev = usb_get_intfdata(intf);
> +
> +	/* let the worker see the device go away, then wait for it */
> +	dev->gspca_dev.present = false;
> +	flush_work(&dev->work_struct);
> +	gspca_disconnect(intf);
> +}
> +
>  /* Table of supported USB devices */
>  static const struct usb_device_id device_table[] = {
>  	{USB_DEVICE(0x04cb, 0x0104)},
> @@ -265,7 +276,7 @@ static const struct sd_desc sd_desc = {
>  	.config = sd_config,
>  	.init   = sd_init,
>  	.start  = sd_start,
> -	.stop0  = sd_stop0,
> +	.stopN  = sd_stopN,
>  };
> 
>  /* -- device connect -- */
> @@ -282,7 +293,7 @@ static struct usb_driver sd_driver = {
>  	.name       = MODULE_NAME,
>  	.id_table   = device_table,
>  	.probe      = sd_probe,
> -	.disconnect = gspca_disconnect,
> +	.disconnect = sd_disconnect,
>  #ifdef CONFIG_PM
>  	.suspend = gspca_suspend,
>  	.resume  = gspca_resume,