All of lore.kernel.org
 help / color / mirror / Atom feed
From: pp <mnipxh@gmail.com>
To: jslaby@suse.cz, gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, yanmin_zhang@linux.intel.com,
	xinhuix.pan@intel.com
Subject: [PATCH] tty/tty_io.c: make a check before reuse cdev
Date: Mon, 21 Jul 2014 20:47:16 +0800	[thread overview]
Message-ID: <53CD0BD4.4050007@gmail.com> (raw)

As reuse the cdev may cause panic. After we unregister the tty device, we may use tty_hangup() o
other similar function to send a signal(SIGHUP) to process which has opend our device. But that
not succeed if the process couldn't get the signal. for example, a process forked
but his parent quited never get SIGHUP.

Here is our scence.
tty driver register its device and init the cdevs, then process "A" open one cdev.
tty driver unregister its device and cdev_del the cdevs, call tty_hangup to (S)send signal SIGHUP to process A.
But that step(S) fails.
tty driver register its device and (D)init the cdevs again.
What will happen is that when process "A" close the cdev, cd_forget() get panic as there is list_del_init(&inode->i_devices);
When we init the cdevs in step(D), the list head was initialized by INIT_LIST_HEAD(&cdev->list);
So check the kobj ref-count before init cdev, and use a bool array to detect if it's ok to unregister the cvdes.
Because most tty driver don't check the ret code of tty_register_device(). and they may still call tty_unregister_device().

Signed-off-by: xinhui.pan <xinhuiX.pan@intel.com>
---
 drivers/tty/tty_io.c       |   17 +++++++++++++++--
 include/linux/tty_driver.h |    1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 3411071..7e79281 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3080,6 +3080,10 @@ struct class *tty_class;
 static int tty_cdev_add(struct tty_driver *driver, dev_t dev,
 		unsigned int index, unsigned int count)
 {
+	if (atomic_read(&driver->cdevs[index].kobj.kref.refcount)) {
+		WARN(1, "init a still in-used cdev!");
+		return -EBUSY;
+	}
 	/* init here, since reused cdevs cause crashes */
 	cdev_init(&driver->cdevs[index], &tty_fops);
 	driver->cdevs[index].owner = driver->owner;
@@ -3185,12 +3189,15 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
 	if (retval)
 		goto error;
 
+	driver->cdevsb[index] = 1;
 	return dev;
 
 error:
 	put_device(dev);
 	if (cdev)
 		cdev_del(&driver->cdevs[index]);
+
+	driver->cdevsb[index] = 0;
 	return ERR_PTR(retval);
 }
 EXPORT_SYMBOL_GPL(tty_register_device_attr);
@@ -3208,6 +3215,8 @@ EXPORT_SYMBOL_GPL(tty_register_device_attr);
 
 void tty_unregister_device(struct tty_driver *driver, unsigned index)
 {
+	if (driver->cdevsb[index] == 0)
+		return;
 	device_destroy(tty_class,
 		MKDEV(driver->major, driver->minor_start) + index);
 	if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC))
@@ -3265,14 +3274,17 @@ struct tty_driver *__tty_alloc_driver(unsigned int lines, struct module *owner,
 		cdevs = lines;
 	}
 
-	driver->cdevs = kcalloc(cdevs, sizeof(*driver->cdevs), GFP_KERNEL);
-	if (!driver->cdevs) {
+	driver->cdevs  = kcalloc(cdevs, sizeof(*driver->cdevs), GFP_KERNEL);
+	driver->cdevsb = kcalloc(cdevs, sizeof(bool), GFP_KERNEL);
+	if (!driver->cdevs || !driver->cdevsb) {
 		err = -ENOMEM;
 		goto err_free_all;
 	}
 
 	return driver;
 err_free_all:
+	kfree(driver->cdevs);
+	kfree(driver->cdevsb);
 	kfree(driver->ports);
 	kfree(driver->ttys);
 	kfree(driver->termios);
@@ -3307,6 +3319,7 @@ static void destruct_tty_driver(struct kref *kref)
 			cdev_del(&driver->cdevs[0]);
 	}
 	kfree(driver->cdevs);
+	kfree(driver->cdevsb);
 	kfree(driver->ports);
 	kfree(driver->termios);
 	kfree(driver->ttys);
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 756a609..7294f1b 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -291,6 +291,7 @@ struct tty_driver {
 	int	magic;		/* magic number for this structure */
 	struct kref kref;	/* Reference management */
 	struct cdev *cdevs;
+	bool	*cdevsb;
 	struct module	*owner;
 	const char	*driver_name;
 	const char	*name;
-- 
1.7.9.5


             reply	other threads:[~2014-07-21 12:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-21 12:47 pp [this message]
2014-07-21 15:38 ` [PATCH] tty/tty_io.c: make a check before reuse cdev Greg KH
2014-07-22 11:52   ` xinhui.pan
2014-07-22 16:40     ` Peter Hurley
2014-07-22 17:38       ` Greg KH
2014-07-23  9:21       ` xinhui.pan
2014-07-23 16:04         ` Peter Hurley
2014-07-24 10:23           ` xinhui.pan
2014-07-25 15:24           ` xinhui
2014-07-23 16:07         ` One Thousand Gnomes
2014-07-24 11:01           ` xinhui.pan
2014-07-24 11:03           ` xinhui.pan

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=53CD0BD4.4050007@gmail.com \
    --to=mnipxh@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xinhuix.pan@intel.com \
    --cc=yanmin_zhang@linux.intel.com \
    /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.