From 818a557eeb9c16a9a2dc93df348b0ff68fbc487f Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Thu, 20 Nov 2008 10:30:26 -0300 Subject: [PATCH] V4L/DVB (9668): em28xx: fix a race condition with hald Newer versions of hald tries to open it to call QUERYCAP. Due to the lack of a proper locking, it is possible to open the device before it finishes initialization. This patch adds a lock to avoid this risk, and to protect the list of em28xx devices. While here, remove the uneeded BKL lock. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/video/em28xx/em28xx-video.c | 128 +++++++++++++--------- 1 file changed, 75 insertions(+), 53 deletions(-) diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c index c32fc7d24a4..90a11b5ac0b 100644 --- a/drivers/media/video/em28xx/em28xx-video.c +++ b/drivers/media/video/em28xx/em28xx-video.c @@ -73,6 +73,7 @@ MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL"); static LIST_HEAD(em28xx_devlist); +static DEFINE_MUTEX(em28xx_devlist_mutex); static unsigned int card[] = {[0 ... (EM28XX_MAXBOARDS - 1)] = UNSET }; static unsigned int video_nr[] = {[0 ... (EM28XX_MAXBOARDS - 1)] = UNSET }; @@ -1519,7 +1520,7 @@ static int em28xx_v4l2_open(struct inode *inode, struct file *filp) struct em28xx_fh *fh; enum v4l2_buf_type fh_type = 0; - lock_kernel(); + mutex_lock(&em28xx_devlist_mutex); list_for_each_entry(h, &em28xx_devlist, devlist) { if (h->vdev->minor == minor) { dev = h; @@ -1535,10 +1536,11 @@ static int em28xx_v4l2_open(struct inode *inode, struct file *filp) dev = h; } } - if (NULL == dev) { - unlock_kernel(); + mutex_unlock(&em28xx_devlist_mutex); + if (NULL == dev) return -ENODEV; - } + + mutex_lock(&dev->lock); em28xx_videodbg("open minor=%d type=%s users=%d\n", minor, v4l2_type_names[fh_type], dev->users); @@ -1547,10 +1549,9 @@ static int em28xx_v4l2_open(struct inode *inode, struct file *filp) fh = kzalloc(sizeof(struct em28xx_fh), GFP_KERNEL); if (!fh) { em28xx_errdev("em28xx-video.c: Out of memory?!\n"); - unlock_kernel(); + mutex_unlock(&dev->lock); return -ENOMEM; } - mutex_lock(&dev->lock); fh->dev = dev; fh->radio = radio; fh->type = fh_type; @@ -1584,7 +1585,6 @@ static int em28xx_v4l2_open(struct inode *inode, struct file *filp) sizeof(struct em28xx_buffer), fh); mutex_unlock(&dev->lock); - unlock_kernel(); return errCode; } @@ -1871,6 +1871,7 @@ int em28xx_register_extension(struct em28xx_ops *ops) { struct em28xx *dev = NULL; + mutex_lock(&em28xx_devlist_mutex); mutex_lock(&em28xx_extension_devlist_lock); list_add_tail(&ops->next, &em28xx_extension_devlist); list_for_each_entry(dev, &em28xx_devlist, devlist) { @@ -1879,6 +1880,7 @@ int em28xx_register_extension(struct em28xx_ops *ops) } printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name); mutex_unlock(&em28xx_extension_devlist_lock); + mutex_unlock(&em28xx_devlist_mutex); return 0; } EXPORT_SYMBOL(em28xx_register_extension); @@ -1887,6 +1889,7 @@ void em28xx_unregister_extension(struct em28xx_ops *ops) { struct em28xx *dev = NULL; + mutex_lock(&em28xx_devlist_mutex); list_for_each_entry(dev, &em28xx_devlist, devlist) { if (dev) ops->fini(dev); @@ -1896,6 +1899,7 @@ void em28xx_unregister_extension(struct em28xx_ops *ops) printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name); list_del(&ops->next); mutex_unlock(&em28xx_extension_devlist_lock); + mutex_unlock(&em28xx_devlist_mutex); } EXPORT_SYMBOL(em28xx_unregister_extension); @@ -1921,6 +1925,60 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev, } +static int register_analog_devices(struct em28xx *dev) +{ + int ret; + + /* allocate and fill video video_device struct */ + dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video"); + if (!dev->vdev) { + em28xx_errdev("cannot allocate video_device.\n"); + return -ENODEV; + } + + /* register v4l2 video video_device */ + ret = video_register_device(dev->vdev, VFL_TYPE_GRABBER, + video_nr[dev->devno]); + if (ret) { + em28xx_errdev("unable to register video device (error=%i).\n", + ret); + return ret; + } + + /* Allocate and fill vbi video_device struct */ + dev->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template, "vbi"); + + /* register v4l2 vbi video_device */ + ret = video_register_device(dev->vbi_dev, VFL_TYPE_VBI, + vbi_nr[dev->devno]); + if (ret < 0) { + em28xx_errdev("unable to register vbi device\n"); + return ret; + } + + if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) { + dev->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template, "radio"); + if (!dev->radio_dev) { + em28xx_errdev("cannot allocate video_device.\n"); + return -ENODEV; + } + ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO, + radio_nr[dev->devno]); + if (ret < 0) { + em28xx_errdev("can't register radio device\n"); + return ret; + } + em28xx_info("Registered radio device as /dev/radio%d\n", + dev->radio_dev->num); + } + + em28xx_info("V4L2 device registered as /dev/video%d and /dev/vbi%d\n", + dev->vdev->num, dev->vbi_dev->num); + + return 0; +} + + /* * em28xx_init_dev() * allocates and inits the device structs, registers i2c bus and v4l device @@ -2000,50 +2058,6 @@ static int em28xx_init_dev(struct em28xx **devhandle, struct usb_device *udev, return errCode; } - list_add_tail(&dev->devlist, &em28xx_devlist); - - /* allocate and fill video video_device struct */ - dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video"); - if (NULL == dev->vdev) { - em28xx_errdev("cannot allocate video_device.\n"); - goto fail_unreg; - } - - /* register v4l2 video video_device */ - retval = video_register_device(dev->vdev, VFL_TYPE_GRABBER, - video_nr[dev->devno]); - if (retval) { - em28xx_errdev("unable to register video device (error=%i).\n", - retval); - goto fail_unreg; - } - - /* Allocate and fill vbi video_device struct */ - dev->vbi_dev = em28xx_vdev_init(dev, &em28xx_video_template, "vbi"); - /* register v4l2 vbi video_device */ - if (video_register_device(dev->vbi_dev, VFL_TYPE_VBI, - vbi_nr[dev->devno]) < 0) { - em28xx_errdev("unable to register vbi device\n"); - retval = -ENODEV; - goto fail_unreg; - } - - if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) { - dev->radio_dev = em28xx_vdev_init(dev, &em28xx_radio_template, "radio"); - if (NULL == dev->radio_dev) { - em28xx_errdev("cannot allocate video_device.\n"); - goto fail_unreg; - } - retval = video_register_device(dev->radio_dev, VFL_TYPE_RADIO, - radio_nr[dev->devno]); - if (retval < 0) { - em28xx_errdev("can't register radio device\n"); - goto fail_unreg; - } - em28xx_info("Registered radio device as /dev/radio%d\n", - dev->radio_dev->num); - } - /* init video dma queues */ INIT_LIST_HEAD(&dev->vidq.active); INIT_LIST_HEAD(&dev->vidq.queued); @@ -2070,8 +2084,14 @@ static int em28xx_init_dev(struct em28xx **devhandle, struct usb_device *udev, video_mux(dev, 0); - em28xx_info("V4L2 device registered as /dev/video%d and /dev/vbi%d\n", - dev->vdev->num, dev->vbi_dev->num); + mutex_lock(&em28xx_devlist_mutex); + list_add_tail(&dev->devlist, &em28xx_devlist); + retval = register_analog_devices(dev); + if (retval < 0) { + em28xx_release_resources(dev); + mutex_unlock(&em28xx_devlist_mutex); + goto fail_reg_devices; + } mutex_lock(&em28xx_extension_devlist_lock); if (!list_empty(&em28xx_extension_devlist)) { @@ -2081,11 +2101,13 @@ static int em28xx_init_dev(struct em28xx **devhandle, struct usb_device *udev, } } mutex_unlock(&em28xx_extension_devlist_lock); + mutex_unlock(&em28xx_devlist_mutex); return 0; fail_unreg: em28xx_release_resources(dev); +fail_reg_devices: mutex_unlock(&dev->lock); return retval; } -- 2.41.1