All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Young <sean@mess.org>
To: linux-media@vger.kernel.org
Subject: [PATCH v2 08/19] [media] lirc: use refcounting for lirc devices
Date: Tue, 21 Feb 2017 20:43:32 +0000	[thread overview]
Message-ID: <a9cf90ec83f4be007a0959887a76429c575cf05f.1487709384.git.sean@mess.org> (raw)
In-Reply-To: <cover.1487709384.git.sean@mess.org>
In-Reply-To: <cover.1487709384.git.sean@mess.org>

If a lirc device is unplugged, the struct rc_dev is freed even though
userspace can still have a file descriptor open on the lirc chardev. The
rc_dev structure can be used in a subsequent, or even currently executing
ioctl, read or write.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/lirc_dev.c | 120 +++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 69 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index ccbdce0..988758b 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -54,7 +54,8 @@ struct irctl {
 	struct lirc_buffer *buf;
 	unsigned int chunk_size;
 
-	struct cdev *cdev;
+	struct device dev;
+	struct cdev cdev;
 
 	struct task_struct *task;
 	long jiffies_to_wait;
@@ -76,15 +77,21 @@ static void lirc_irctl_init(struct irctl *ir)
 	ir->d.minor = NOPLUG;
 }
 
-static void lirc_irctl_cleanup(struct irctl *ir)
+static void lirc_release(struct device *ld)
 {
-	device_destroy(lirc_class, MKDEV(MAJOR(lirc_base_dev), ir->d.minor));
+	struct irctl *ir = container_of(ld, struct irctl, dev);
+
+	put_device(ir->dev.parent);
 
 	if (ir->buf != ir->d.rbuf) {
 		lirc_buffer_free(ir->buf);
 		kfree(ir->buf);
 	}
-	ir->buf = NULL;
+
+	mutex_lock(&lirc_dev_lock);
+	irctls[ir->d.minor] = NULL;
+	mutex_unlock(&lirc_dev_lock);
+	kfree(ir);
 }
 
 /*  helper function
@@ -157,32 +164,21 @@ static int lirc_cdev_add(struct irctl *ir)
 	struct cdev *cdev;
 	int retval;
 
-	cdev = cdev_alloc();
-	if (!cdev)
-		return -ENOMEM;
+	cdev = &ir->cdev;
 
 	if (d->fops) {
-		cdev->ops = d->fops;
+		cdev_init(cdev, d->fops);
 		cdev->owner = d->owner;
 	} else {
-		cdev->ops = &lirc_dev_fops;
+		cdev_init(cdev, &lirc_dev_fops);
 		cdev->owner = THIS_MODULE;
 	}
 	retval = kobject_set_name(&cdev->kobj, "lirc%d", d->minor);
 	if (retval)
-		goto err_out;
-
-	retval = cdev_add(cdev, MKDEV(MAJOR(lirc_base_dev), d->minor), 1);
-	if (retval)
-		goto err_out;
-
-	ir->cdev = cdev;
-
-	return 0;
+		return retval;
 
-err_out:
-	cdev_del(cdev);
-	return retval;
+	cdev->kobj.parent = &ir->dev.kobj;
+	return cdev_add(cdev, ir->dev.devt, 1);
 }
 
 static int lirc_allocate_buffer(struct irctl *ir)
@@ -304,9 +300,12 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 
 	ir->d = *d;
 
-	device_create(lirc_class, ir->d.dev,
-		      MKDEV(MAJOR(lirc_base_dev), ir->d.minor), NULL,
-		      "lirc%u", ir->d.minor);
+	ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
+	ir->dev.class = lirc_class;
+	ir->dev.parent = d->dev;
+	ir->dev.release = lirc_release;
+	dev_set_name(&ir->dev, "lirc%d", ir->d.minor);
+	device_initialize(&ir->dev);
 
 	if (d->sample_rate) {
 		ir->jiffies_to_wait = HZ / d->sample_rate;
@@ -329,14 +328,22 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 		goto out_sysfs;
 
 	ir->attached = 1;
+
+	err = device_add(&ir->dev);
+	if (err)
+		goto out_cdev;
+
 	mutex_unlock(&lirc_dev_lock);
 
+	get_device(ir->dev.parent);
+
 	dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
 		 ir->d.name, ir->d.minor);
 	return minor;
-
+out_cdev:
+	cdev_del(&ir->cdev);
 out_sysfs:
-	device_destroy(lirc_class, MKDEV(MAJOR(lirc_base_dev), ir->d.minor));
+	put_device(&ir->dev);
 out_lock:
 	mutex_unlock(&lirc_dev_lock);
 
@@ -364,7 +371,6 @@ EXPORT_SYMBOL(lirc_register_driver);
 int lirc_unregister_driver(int minor)
 {
 	struct irctl *ir;
-	struct cdev *cdev;
 
 	if (minor < 0 || minor >= MAX_IRCTL_DEVICES) {
 		pr_err("minor (%d) must be between 0 and %d!\n",
@@ -378,8 +384,6 @@ int lirc_unregister_driver(int minor)
 		return -ENOENT;
 	}
 
-	cdev = ir->cdev;
-
 	mutex_lock(&lirc_dev_lock);
 
 	if (ir->d.minor != minor) {
@@ -401,22 +405,20 @@ int lirc_unregister_driver(int minor)
 		dev_dbg(ir->d.dev, LOGHEAD "releasing opened driver\n",
 			ir->d.name, ir->d.minor);
 		wake_up_interruptible(&ir->buf->wait_poll);
-		mutex_lock(&ir->irctl_lock);
+	}
 
-		if (ir->d.set_use_dec)
-			ir->d.set_use_dec(ir->d.data);
+	mutex_lock(&ir->irctl_lock);
 
-		module_put(cdev->owner);
-		mutex_unlock(&ir->irctl_lock);
-	} else {
-		lirc_irctl_cleanup(ir);
-		cdev_del(cdev);
-		kfree(ir);
-		irctls[minor] = NULL;
-	}
+	if (ir->d.set_use_dec)
+		ir->d.set_use_dec(ir->d.data);
 
+	mutex_unlock(&ir->irctl_lock);
 	mutex_unlock(&lirc_dev_lock);
 
+	device_del(&ir->dev);
+	cdev_del(&ir->cdev);
+	put_device(&ir->dev);
+
 	return 0;
 }
 EXPORT_SYMBOL(lirc_unregister_driver);
@@ -424,7 +426,6 @@ EXPORT_SYMBOL(lirc_unregister_driver);
 int lirc_dev_fop_open(struct inode *inode, struct file *file)
 {
 	struct irctl *ir;
-	struct cdev *cdev;
 	int retval = 0;
 
 	if (iminor(inode) >= MAX_IRCTL_DEVICES) {
@@ -459,18 +460,14 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file)
 			goto error;
 	}
 
-	cdev = ir->cdev;
-	if (try_module_get(cdev->owner)) {
-		ir->open++;
-		if (ir->d.set_use_inc)
-			retval = ir->d.set_use_inc(ir->d.data);
-
-		if (retval) {
-			module_put(cdev->owner);
-			ir->open--;
-		} else if (ir->buf) {
+	ir->open++;
+	if (ir->d.set_use_inc)
+		retval = ir->d.set_use_inc(ir->d.data);
+	if (retval) {
+		ir->open--;
+	} else {
+		if (ir->buf)
 			lirc_buffer_clear(ir->buf);
-		}
 		if (ir->task)
 			wake_up_process(ir->task);
 	}
@@ -487,7 +484,6 @@ EXPORT_SYMBOL(lirc_dev_fop_open);
 int lirc_dev_fop_close(struct inode *inode, struct file *file)
 {
 	struct irctl *ir = irctls[iminor(inode)];
-	struct cdev *cdev;
 	int ret;
 
 	if (!ir) {
@@ -495,25 +491,14 @@ int lirc_dev_fop_close(struct inode *inode, struct file *file)
 		return -EINVAL;
 	}
 
-	cdev = ir->cdev;
-
 	ret = mutex_lock_killable(&lirc_dev_lock);
 	WARN_ON(ret);
 
 	rc_close(ir->d.rdev);
 
 	ir->open--;
-	if (ir->attached) {
-		if (ir->d.set_use_dec)
-			ir->d.set_use_dec(ir->d.data);
-		module_put(cdev->owner);
-	} else {
-		lirc_irctl_cleanup(ir);
-		cdev_del(cdev);
-		irctls[ir->d.minor] = NULL;
-		kfree(ir);
-	}
-
+	if (ir->d.set_use_dec)
+		ir->d.set_use_dec(ir->d.data);
 	if (!ret)
 		mutex_unlock(&lirc_dev_lock);
 
@@ -780,15 +765,12 @@ static int __init lirc_dev_init(void)
 		return retval;
 	}
 
-
 	pr_info("IR Remote Control driver registered, major %d\n",
 						MAJOR(lirc_base_dev));
 
 	return 0;
 }
 
-
-
 static void __exit lirc_dev_exit(void)
 {
 	class_destroy(lirc_class);
-- 
2.9.3

  parent reply	other threads:[~2017-02-21 20:43 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-21 20:43 [PATCH v2 00/19] Teach lirc how to send and receive scancodes Sean Young
2017-02-21 20:43 ` [PATCH v2 01/19] [media] lirc: document lirc modes better Sean Young
2017-02-21 20:43 ` [PATCH v2 02/19] [media] lirc: return ENOTTY when ioctl is not supported Sean Young
2017-02-21 20:43 ` [PATCH v2 03/19] [media] lirc: return ENOTTY when device does support ioctl Sean Young
2017-02-21 20:43 ` [PATCH v2 04/19] [media] winbond: allow timeout to be set Sean Young
2017-02-21 20:43 ` [PATCH v2 05/19] [media] gpio-ir: do not allow a timeout of 0 Sean Young
2017-02-21 20:43 ` [PATCH v2 06/19] [media] rc: lirc keymap no longer makes any sense Sean Young
2017-02-21 20:43 ` [PATCH v2 07/19] [media] lirc: advertise LIRC_CAN_GET_REC_RESOLUTION and improve Sean Young
2017-02-21 20:43 ` Sean Young [this message]
2017-02-21 20:43 ` [PATCH v2 09/19] [media] mce_kbd: add encoder Sean Young
2017-02-21 20:43 ` [PATCH v2 10/19] [media] serial_ir: iommap is a memory address, not bool Sean Young
2017-02-21 20:43 ` [PATCH v2 11/19] [media] lirc: lirc interface should not be a raw decoder Sean Young
2017-02-21 20:43 ` [PATCH v2 12/19] [media] lirc: exorcise struct irctl Sean Young
2017-02-21 20:43 ` [PATCH v2 13/19] [media] lirc: use plain kfifo rather than lirc_buffer Sean Young
2017-02-21 20:43 ` [PATCH v2 14/19] [media] lirc: implement scancode sending Sean Young
2017-02-21 20:43 ` [PATCH v2 15/19] [media] rc: use the correct carrier for scancode transmit Sean Young
2017-02-21 20:43 ` [PATCH v2 16/19] [media] rc: auto load encoder if necessary Sean Young
2017-02-21 20:43 ` [PATCH v2 17/19] [media] lirc: implement reading scancode Sean Young
2017-02-22  0:14   ` kbuild test robot
2017-02-22 10:45     ` Sean Young
2017-02-22  2:20   ` kbuild test robot
2017-02-21 20:43 ` [PATCH v2 18/19] [media] lirc: scancode rc devices should have a lirc device too Sean Young
2017-02-21 20:43 ` [PATCH v2 19/19] [media] lirc: document LIRC_MODE_SCANCODE Sean Young

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9cf90ec83f4be007a0959887a76429c575cf05f.1487709384.git.sean@mess.org \
    --to=sean@mess.org \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.