[PATCH] uvc: fix access to uninitialized fields on probe error

[PATCH] uvc: fix access to uninitialized fields on probe error

[Oliver Neukum]

We need to check whether this work we are canceling actually is
initialized.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: syzbot+2e1ef9188251d9cc7944@syzkaller.appspotmail.com
---
 drivers/media/usb/uvc/uvc_ctrl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 14cff91b7aea..2b510c56cf91 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2350,7 +2350,9 @@ void uvc_ctrl_cleanup_device(struct uvc_device *dev)
 	struct uvc_entity *entity;
 	unsigned int i;
 
-	cancel_work_sync(&dev->async_ctrl.work);
+	/* can be uninitialized if we are aborting on probe error */
+	if (dev->async_ctrl.work.func)
+		cancel_work_sync(&dev->async_ctrl.work);
 
 	/* Free controls and control mappings for all entities. */
 	list_for_each_entry(entity, &dev->entities, list) {
-- 
2.16.4

Re: [PATCH] uvc: fix access to uninitialized fields on probe error

[Laurent Pinchart]

Hi Oliver,

Thank you for the patch.

On Tue, Apr 30, 2019 at 02:28:14PM +0200, Oliver Neukum wrote:
> We need to check whether this work we are canceling actually is
> initialized.

I think we should add

Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")

> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: syzbot+2e1ef9188251d9cc7944@syzkaller.appspotmail.com
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 14cff91b7aea..2b510c56cf91 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2350,7 +2350,9 @@ void uvc_ctrl_cleanup_device(struct uvc_device *dev)
>  	struct uvc_entity *entity;
>  	unsigned int i;
>  
> -	cancel_work_sync(&dev->async_ctrl.work);
> +	/* can be uninitialized if we are aborting on probe error */

Let's capitalise the comment and end it with a period to match the rest
of the driver. With these small issues fixed,

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

If you agree with the changes there's no need to resubmit, I'll fix when
applying.

> +	if (dev->async_ctrl.work.func)
> +		cancel_work_sync(&dev->async_ctrl.work);
>  
>  	/* Free controls and control mappings for all entities. */
>  	list_for_each_entry(entity, &dev->entities, list) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH] uvc: fix access to uninitialized fields on probe error

[Oliver Neukum]

On Do, 2019-05-02 at 14:43 +0300, Laurent Pinchart wrote:
> Let's capitalise the comment and end it with a period to match the rest
> of the driver. With these small issues fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Hi,

thank you. I agree with them.

	Regards
		Oliver