[media] media: lirc_dev: sanitize locking

Use the irctl mutex for all device operations and only use lirc_dev_lock
to protect the irctls array. Also, make sure that the device is alive
early in each fops function before doing anything else.

Since this patch touches nearly every line where the irctl mutex is
taken/released, it also renames the mutex at the same time (the name
irctl_lock will be misleading once struct irctl goes away in later
patches).

[mchehab@s-opensource.com: fix a merge conflict]
Signed-off-by: David Härdeman <david@hardeman.nu>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index b07d0ab..c83fffe 100644 (file)
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -38,7 +38,7 @@ struct irctl {
        bool attached;
        int open;
 
-       struct mutex irctl_lock;
+       struct mutex mutex;     /* protect from simultaneous accesses */
        struct lirc_buffer *buf;
        bool buf_internal;
 
@@ -46,6 +46,7 @@ struct irctl {
        struct cdev cdev;
 };
 
+/* This mutex protects the irctls array */
 static DEFINE_MUTEX(lirc_dev_lock);
 
 static struct irctl *irctls[MAX_IRCTL_DEVICES];
@@ -53,20 +54,25 @@ static struct irctl *irctls[MAX_IRCTL_DEVICES];
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
 
-static void lirc_release(struct device *ld)
+static void lirc_free_buffer(struct irctl *ir)
 {
-       struct irctl *ir = container_of(ld, struct irctl, dev);
-
        put_device(ir->dev.parent);
 
        if (ir->buf_internal) {
                lirc_buffer_free(ir->buf);
                kfree(ir->buf);
+               ir->buf = NULL;
        }
+}
+
+static void lirc_release(struct device *ld)
+{
+       struct irctl *ir = container_of(ld, struct irctl, dev);
 
        mutex_lock(&lirc_dev_lock);
        irctls[ir->d.minor] = NULL;
        mutex_unlock(&lirc_dev_lock);
+       lirc_free_buffer(ir);
        kfree(ir);
 }
 
@@ -143,6 +149,28 @@ int lirc_register_driver(struct lirc_driver *d)
                return -EBADRQC;
        }
 
+       /* some safety check 8-) */
+       d->name[sizeof(d->name) - 1] = '\0';
+
+       if (d->features == 0)
+               d->features = LIRC_CAN_REC_LIRCCODE;
+
+       ir = kzalloc(sizeof(*ir), GFP_KERNEL);
+       if (!ir)
+               return -ENOMEM;
+
+       mutex_init(&ir->mutex);
+       ir->d = *d;
+
+       if (LIRC_CAN_REC(d->features)) {
+               err = lirc_allocate_buffer(ir);
+               if (err) {
+                       kfree(ir);
+                       return err;
+               }
+               d->rbuf = ir->buf;
+       }
+
        mutex_lock(&lirc_dev_lock);
 
        /* find first free slot for driver */
@@ -152,37 +180,18 @@ int lirc_register_driver(struct lirc_driver *d)
 
        if (minor == MAX_IRCTL_DEVICES) {
                dev_err(d->dev, "no free slots for drivers!\n");
-               err = -ENOMEM;
-               goto out_lock;
-       }
-
-       ir = kzalloc(sizeof(struct irctl), GFP_KERNEL);
-       if (!ir) {
-               err = -ENOMEM;
-               goto out_lock;
+               mutex_unlock(&lirc_dev_lock);
+               lirc_free_buffer(ir);
+               kfree(ir);
+               return -ENOMEM;
        }
 
-       mutex_init(&ir->irctl_lock);
        irctls[minor] = ir;
        d->irctl = ir;
        d->minor = minor;
+       ir->d.minor = minor;
 
-       /* some safety check 8-) */
-       d->name[sizeof(d->name)-1] = '\0';
-
-       if (d->features == 0)
-               d->features = LIRC_CAN_REC_LIRCCODE;
-
-       ir->d = *d;
-
-       if (LIRC_CAN_REC(d->features)) {
-               err = lirc_allocate_buffer(irctls[minor]);
-               if (err) {
-                       kfree(ir);
-                       goto out_lock;
-               }
-               d->rbuf = ir->buf;
-       }
+       mutex_unlock(&lirc_dev_lock);
 
        device_initialize(&ir->dev);
        ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
@@ -196,10 +205,10 @@ int lirc_register_driver(struct lirc_driver *d)
        ir->attached = true;
 
        err = cdev_device_add(&ir->cdev, &ir->dev);
-       if (err)
-               goto out_dev;
-
-       mutex_unlock(&lirc_dev_lock);
+       if (err) {
+               put_device(&ir->dev);
+               return err;
+       }
 
        get_device(ir->dev.parent);
 
@@ -207,13 +216,6 @@ int lirc_register_driver(struct lirc_driver *d)
                 ir->d.name, ir->d.minor);
 
        return 0;
-
-out_dev:
-       put_device(&ir->dev);
-out_lock:
-       mutex_unlock(&lirc_dev_lock);
-
-       return err;
 }
 EXPORT_SYMBOL(lirc_register_driver);
 
@@ -226,11 +228,13 @@ void lirc_unregister_driver(struct lirc_driver *d)
 
        ir = d->irctl;
 
-       mutex_lock(&lirc_dev_lock);
-
        dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
                d->name, d->minor);
 
+       cdev_device_del(&ir->cdev, &ir->dev);
+
+       mutex_lock(&ir->mutex);
+
        ir->attached = false;
        if (ir->open) {
                dev_dbg(ir->d.dev, LOGHEAD "releasing opened driver\n",
@@ -238,9 +242,8 @@ void lirc_unregister_driver(struct lirc_driver *d)
                wake_up_interruptible(&ir->buf->wait_poll);
        }
 
-       mutex_unlock(&lirc_dev_lock);
+       mutex_unlock(&ir->mutex);
 
-       cdev_device_del(&ir->cdev, &ir->dev);
        put_device(&ir->dev);
 }
 EXPORT_SYMBOL(lirc_unregister_driver);
@@ -252,13 +255,24 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file)
 
        dev_dbg(ir->d.dev, LOGHEAD "open called\n", ir->d.name, ir->d.minor);
 
-       if (ir->open)
-               return -EBUSY;
+       retval = mutex_lock_interruptible(&ir->mutex);
+       if (retval)
+               return retval;
+
+       if (!ir->attached) {
+               retval = -ENODEV;
+               goto out;
+       }
+
+       if (ir->open) {
+               retval = -EBUSY;
+               goto out;
+       }
 
        if (ir->d.rdev) {
                retval = rc_open(ir->d.rdev);
                if (retval)
-                       return retval;
+                       goto out;
        }
 
        if (ir->buf)
@@ -268,24 +282,26 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file)
 
        lirc_init_pdata(inode, file);
        nonseekable_open(inode, file);
+       mutex_unlock(&ir->mutex);
 
        return 0;
+
+out:
+       mutex_unlock(&ir->mutex);
+       return retval;
 }
 EXPORT_SYMBOL(lirc_dev_fop_open);
 
 int lirc_dev_fop_close(struct inode *inode, struct file *file)
 {
        struct irctl *ir = file->private_data;
-       int ret;
 
-       ret = mutex_lock_killable(&lirc_dev_lock);
-       WARN_ON(ret);
+       mutex_lock(&ir->mutex);
 
        rc_close(ir->d.rdev);
-
        ir->open--;
-       if (!ret)
-               mutex_unlock(&lirc_dev_lock);
+
+       mutex_unlock(&ir->mutex);
 
        return 0;
 }
@@ -320,19 +336,20 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
        struct irctl *ir = file->private_data;
        __u32 mode;
-       int result = 0;
+       int result;
 
        dev_dbg(ir->d.dev, LOGHEAD "ioctl called (0x%x)\n",
                ir->d.name, ir->d.minor, cmd);
 
+       result = mutex_lock_interruptible(&ir->mutex);
+       if (result)
+               return result;
+
        if (!ir->attached) {
-               dev_err(ir->d.dev, LOGHEAD "ioctl result = -ENODEV\n",
-                       ir->d.name, ir->d.minor);
-               return -ENODEV;
+               result = -ENODEV;
+               goto out;
        }
 
-       mutex_lock(&ir->irctl_lock);
-
        switch (cmd) {
        case LIRC_GET_FEATURES:
                result = put_user(ir->d.features, (__u32 __user *)arg);
@@ -386,8 +403,8 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
                result = -ENOTTY;
        }
 
-       mutex_unlock(&ir->irctl_lock);
-
+out:
+       mutex_unlock(&ir->mutex);
        return result;
 }
 EXPORT_SYMBOL(lirc_dev_fop_ioctl);
@@ -399,27 +416,31 @@ ssize_t lirc_dev_fop_read(struct file *file,
 {
        struct irctl *ir = file->private_data;
        unsigned char *buf;
-       int ret = 0, written = 0;
+       int ret, written = 0;
        DECLARE_WAITQUEUE(wait, current);
 
-       if (!LIRC_CAN_REC(ir->d.features))
-               return -EINVAL;
-
        dev_dbg(ir->d.dev, LOGHEAD "read called\n", ir->d.name, ir->d.minor);
 
        buf = kzalloc(ir->buf->chunk_size, GFP_KERNEL);
        if (!buf)
                return -ENOMEM;
 
-       if (mutex_lock_interruptible(&ir->irctl_lock)) {
-               ret = -ERESTARTSYS;
-               goto out_unlocked;
+       ret = mutex_lock_interruptible(&ir->mutex);
+       if (ret) {
+               kfree(buf);
+               return ret;
        }
+
        if (!ir->attached) {
                ret = -ENODEV;
                goto out_locked;
        }
 
+       if (!LIRC_CAN_REC(ir->d.features)) {
+               ret = -EINVAL;
+               goto out_locked;
+       }
+
        if (length % ir->buf->chunk_size) {
                ret = -EINVAL;
                goto out_locked;
@@ -454,13 +475,13 @@ ssize_t lirc_dev_fop_read(struct file *file,
                                break;
                        }
 
-                       mutex_unlock(&ir->irctl_lock);
+                       mutex_unlock(&ir->mutex);
                        set_current_state(TASK_INTERRUPTIBLE);
                        schedule();
                        set_current_state(TASK_RUNNING);
 
-                       if (mutex_lock_interruptible(&ir->irctl_lock)) {
-                               ret = -ERESTARTSYS;
+                       ret = mutex_lock_interruptible(&ir->mutex);
+                       if (ret) {
                                remove_wait_queue(&ir->buf->wait_poll, &wait);
                                goto out_unlocked;
                        }
@@ -483,7 +504,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
        remove_wait_queue(&ir->buf->wait_poll, &wait);
 
 out_locked:
-       mutex_unlock(&ir->irctl_lock);
+       mutex_unlock(&ir->mutex);
 
 out_unlocked:
        kfree(buf);