au0828_v4l2_device_register()

[Shuah Khan <shuahkh@osg.samsung.com>]

Hi Mauro/Javier,

I can't figure out when au0828_v4l2_device_register() was added. Must be in
Linux 4.5 as I can't find this change in Linux 4.4 This used to be a call to
v4l2_device_register() from au0828_usb_probe(). When the code was moved, locking
bugs are introduced.

Notice that au0828_v4l2_device_register() does the following in error legs:

                mutex_unlock(&dev->lock);
                kfree(dev);


And au0828_usb_probe() also does the same cleanup when au0828_v4l2_device_register()
returns error:

        retval = au0828_v4l2_device_register(interface, dev);
        if (retval) {
                au0828_usb_v4l2_media_release(dev);
                mutex_unlock(&dev->lock);
                kfree(dev);
                return retval;
        }

We could be seeing some problems if this fails.

Please let me know if you would like a patch to fix this.

The following is the right fix:

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 32d7db9..7d0ec4c 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -679,8 +679,6 @@ int au0828_v4l2_device_register(struct usb_interface *interface,
        if (retval) {
                pr_err("%s() v4l2_device_register failed\n",
                       __func__);
-               mutex_unlock(&dev->lock);
-               kfree(dev);
                return retval;
        }
 
@@ -691,8 +689,6 @@ int au0828_v4l2_device_register(struct usb_interface *interface,
        if (retval) {
                pr_err("%s() v4l2_ctrl_handler_init failed\n",
                       __func__);
-               mutex_unlock(&dev->lock);
-               kfree(dev);
                return retval;
        }
        dev->v4l2_dev.ctrl_handler = &dev->v4l2_ctrl_hdl;


thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com