Re: [PATCH] media: uvcvideo: Fix deadlock during uvc_probe

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Ricardo,

Thank you for the patch.

On Tue, Oct 22, 2024 at 08:30:30AM +0000, Ricardo Ribalda wrote:
> If uvc_probe() fails, it can end up calling uvc_status_unregister() before
> uvc_status_init() is called.
> 
> Fix this by checking if dev->status is NULL or not in
> uvc_status_unregister()

That will not work in case usb_alloc_urb() fails in uvc_status_init().
In that error path, dev->status is freed but the pointer is not set to
NULL. Setting it to NULL should be enough to fix the problem. I'll do
that and apply this patch to my tree.

> Reported-by: syzbot+9446d5e0d25571e6a212@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-media/20241020160249.GD7770@pendragon.ideasonboard.com/T/#m506744621d72a2ace5dd2ab64055be9898112dbd
> Fixes: c5fe3ed618f9 ("media: uvcvideo: Avoid race condition during unregister")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_status.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
> index 06c867510c8f..b3527895c2f6 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -294,6 +294,8 @@ int uvc_status_init(struct uvc_device *dev)
>  
>  void uvc_status_unregister(struct uvc_device *dev)
>  {
> +	if (!dev->status)
> +		return;

I'd add a blank line here.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	uvc_status_suspend(dev);
>  	uvc_input_unregister(dev);
>  }
> 
> ---
> base-commit: 698b6e3163bafd61e1b7d13572e2c42974ac85ec
> change-id: 20241022-race-unreg-85295d5fbeee

-- 
Regards,

Laurent Pinchart