Re: [PATCH 2/2] usb: gadget: uvc: refactor video cleanup into uvcg_video_deinit()

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Aug 21, 2026 at 04:22:37PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
>
> kthread_destroy_worker() was never called during unbind, leaving the
> UVCG kthread running after the gadget function is unbound. Also, if
> uvcg_video_init() or uvc_register_video() fails during bind, async_wq
> and kworker were not cleaned up, causing resource leaks.
>
> Both issues require the same teardown sequence: cancel the pending work,
> destroy the kworker, and destroy the workqueue. Consolidate this logic
> into a new uvcg_video_deinit() helper and call it from both
> uvc_function_unbind() and the v4l2_error path in uvc_function_bind().
>
> In uvc_function_unbind(), uvcg_video_deinit() is placed after
> video_unregister_device() to fix the ordering. Without this ordering,
> tearing down the workers before unregistering the V4L2 device could lead
> to use-after-free on video device resources still accessed by those
> workers.
>
> Fixes: f0bbfbd16b3b ("usb: gadget: uvc: rework to enqueue in pump worker from encoded queue")
> Assisted-by: Claude:claude-sonnet-4.6
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/usb/gadget/function/f_uvc.c     |  7 ++-----
>  drivers/usb/gadget/function/uvc_video.c | 15 +++++++++++++++
>  drivers/usb/gadget/function/uvc_video.h |  1 +
>  3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index a4fb2790f4ff..fa2f9d0e4ce0 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -879,6 +879,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>  	return 0;
>
>  v4l2_error:
> +	uvcg_video_deinit(&uvc->video);
>  	v4l2_device_unregister(&uvc->v4l2_dev);
>  error:
>  	if (uvc->control_req) {
> @@ -1028,11 +1029,6 @@ static void uvc_function_unbind(struct usb_configuration *c,
>  		connected = uvc->func_connected;
>  	}
>
> -	kthread_cancel_work_sync(&video->hw_submit);
> -
> -	if (video->async_wq)
> -		destroy_workqueue(video->async_wq);
> -
>  	/*
>  	 * If we know we're connected via v4l2, then there should be a cleanup
>  	 * of the device from userspace either via UVC_EVENT_DISCONNECT or
> @@ -1048,6 +1044,7 @@ static void uvc_function_unbind(struct usb_configuration *c,
>
>  	device_remove_file(&uvc->vdev.dev, &dev_attr_function_name);
>  	video_unregister_device(&uvc->vdev);
> +	uvcg_video_deinit(video);
>  	v4l2_device_unregister(&uvc->v4l2_dev);
>
>  	scoped_guard(mutex, &uvc->lock)
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index 9ba09118bb74..002afca9141e 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -841,3 +841,18 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
>  	return uvcg_queue_init(&video->queue, uvc->v4l2_dev.dev->parent,
>  			V4L2_BUF_TYPE_VIDEO_OUTPUT, &video->mutex);
>  }
> +
> +void uvcg_video_deinit(struct uvc_video *video)
> +{
> +	kthread_cancel_work_sync(&video->hw_submit);
> +
> +	if (!IS_ERR_OR_NULL(video->kworker)) {
> +		kthread_destroy_worker(video->kworker);
> +		video->kworker = NULL;
> +	}
> +
> +	if (video->async_wq) {
> +		destroy_workqueue(video->async_wq);
> +		video->async_wq = NULL;
> +	}
> +}
> diff --git a/drivers/usb/gadget/function/uvc_video.h b/drivers/usb/gadget/function/uvc_video.h
> index 8ef6259741f1..6c5481f107f9 100644
> --- a/drivers/usb/gadget/function/uvc_video.h
> +++ b/drivers/usb/gadget/function/uvc_video.h
> @@ -18,5 +18,6 @@ int uvcg_video_enable(struct uvc_video *video);
>  int uvcg_video_disable(struct uvc_video *video);
>
>  int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc);
> +void uvcg_video_deinit(struct uvc_video *video);
>
>  #endif /* __UVC_VIDEO_H__ */
>
> --
> 2.34.1
>
>