All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>,
	linux-kernel@vger.kernel.org,
	Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH 09/12] tty: Re-define tty driver remove() interface
Date: Fri, 27 Nov 2015 21:25:54 -0500	[thread overview]
Message-ID: <1448677557-16420-10-git-send-email-peter@hurleysoftware.com> (raw)
In-Reply-To: <1448677557-16420-1-git-send-email-peter@hurleysoftware.com>

The tty driver remove() method is used exclusively by the BSD and Unix98
pty drivers to release the tty index; the BSD drivers clear the tty tables
for the pty pair @index, while the Unix98 drivers free the devpts index.

Although the remove() method is intended as the symmetric operation to the
lookup() method (ie., the index obtained via the lookup() is released by
the remove() method). the actual interface isn't defined reflexively.

This causes cleanup problems for ptmx_open() if initialization fails in
tty_init_dev(): ptmx_open() releases the devpts index, but the index may
have been already been released by release_tty() during cleanup in
tty_init_dev() instead. This will cause mismatched devpts counts and
duplicate removal of the devpts index.

Re-define the driver remove() method as the symmetric operation to the
lookup() method. Propagate the inode parameter dependency from the
file operation to the tty driver method:

 tty_open()  -+
              +-> tty_init_dev() +-> release_tty() -> tty_driver_remove_tty()
 ptmx_open() -+                  |
                                 |
 tty_release() ------------------+

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/pty.c          | 13 ++++++-------
 drivers/tty/tty_io.c       | 24 ++++++++++++++----------
 include/linux/tty.h        |  3 ++-
 include/linux/tty_driver.h |  4 ++--
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 2680044..c71a1ab 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -461,10 +461,10 @@ static int pty_install(struct tty_driver *driver, struct tty_struct *tty)
 	return pty_common_install(driver, tty, true);
 }
 
-static void pty_remove(struct tty_driver *driver, struct tty_struct *tty)
+static void pty_remove(struct tty_driver *driver, struct inode *inode, int idx)
 {
-	driver->ttys[tty->index] = NULL;
-	driver->other->ttys[tty->index] = NULL;
+	driver->ttys[idx] = NULL;
+	driver->other->ttys[idx] = NULL;
 }
 
 static int pty_bsd_ioctl(struct tty_struct *tty,
@@ -660,9 +660,9 @@ static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
 }
 
 /* this is called once with whichever end is closed last */
-static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty)
+static void pty_unix98_remove(struct tty_driver *driver, struct inode *inode, int idx)
 {
-	devpts_kill_index(tty->driver_data, tty->index);
+	devpts_kill_index(inode, idx);
 }
 
 static const struct tty_operations ptm_unix98_ops = {
@@ -738,7 +738,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	mutex_unlock(&devpts_mutex);
 
 	mutex_lock(&tty_mutex);
-	tty = tty_init_dev(ptm_driver, index);
+	tty = tty_init_dev(ptm_driver, inode, index);
 
 	if (IS_ERR(tty)) {
 		retval = PTR_ERR(tty);
@@ -777,7 +777,6 @@ err_release:
 	return retval;
 out:
 	mutex_unlock(&tty_mutex);
-	devpts_kill_index(inode, index);
 err_file:
 	tty_free_file(filp);
 	return retval;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index afd378e..2a6d40c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -158,7 +158,7 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 #endif
 static int __tty_fasync(int fd, struct file *filp, int on);
 static int tty_fasync(int fd, struct file *filp, int on);
-static void release_tty(struct tty_struct *tty, int idx);
+static void release_tty(struct tty_struct *tty, struct inode *inode, int idx);
 
 /**
  *	free_tty_struct		-	free a disused tty
@@ -1435,6 +1435,7 @@ static int tty_driver_install_tty(struct tty_driver *driver,
 /**
  *	tty_driver_remove_tty() - remove a tty from the driver tables
  *	@driver: the driver for the tty
+ *	@inode:	 device file inode
  *	@idx:	 the minor number
  *
  *	Remvoe a tty object from the driver tables. The tty->index field
@@ -1442,12 +1443,13 @@ static int tty_driver_install_tty(struct tty_driver *driver,
  *
  *	Locking: tty_mutex for now
  */
-static void tty_driver_remove_tty(struct tty_driver *driver, struct tty_struct *tty)
+static void tty_driver_remove_tty(struct tty_driver *driver,
+				  struct inode *inode, int idx)
 {
 	if (driver->ops->remove)
-		driver->ops->remove(driver, tty);
+		driver->ops->remove(driver, inode, idx);
 	else
-		driver->ttys[tty->index] = NULL;
+		driver->ttys[idx] = NULL;
 }
 
 /*
@@ -1505,7 +1507,8 @@ static int tty_reopen(struct tty_struct *tty)
  * relaxed for the (most common) case of reopening a tty.
  */
 
-struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
+struct tty_struct *tty_init_dev(struct tty_driver *driver,
+				struct inode *inode, int idx)
 {
 	struct tty_struct *tty;
 	int retval;
@@ -1557,6 +1560,7 @@ err_free_tty:
 	free_tty_struct(tty);
 err_module_put:
 	module_put(driver->owner);
+	tty_driver_remove_tty(driver, inode, idx);
 	return ERR_PTR(retval);
 
 	/* call the tty release_tty routine to clean out this slot */
@@ -1564,7 +1568,7 @@ err_release_tty:
 	tty_unlock(tty);
 	tty_info_ratelimited(tty, "ldisc open failed (%d), clearing slot %d\n",
 			     retval, idx);
-	release_tty(tty, idx);
+	release_tty(tty, inode, idx);
 	return ERR_PTR(retval);
 }
 
@@ -1679,7 +1683,7 @@ EXPORT_SYMBOL(tty_kref_put);
  *	of ttys that the driver keeps.
  *
  */
-static void release_tty(struct tty_struct *tty, int idx)
+static void release_tty(struct tty_struct *tty, struct inode *inode, int idx)
 {
 	/* This should always be true but check for the moment */
 	WARN_ON(tty->index != idx);
@@ -1687,7 +1691,7 @@ static void release_tty(struct tty_struct *tty, int idx)
 	if (tty->ops->shutdown)
 		tty->ops->shutdown(tty);
 	tty_free_termios(tty);
-	tty_driver_remove_tty(tty->driver, tty);
+	tty_driver_remove_tty(tty->driver, inode, idx);
 	tty->port->itty = NULL;
 	if (tty->link)
 		tty->link->port->itty = NULL;
@@ -1910,7 +1914,7 @@ int tty_release(struct inode *inode, struct file *filp)
 	 * unlock never unlocks a freed tty).
 	 */
 	mutex_lock(&tty_mutex);
-	release_tty(tty, idx);
+	release_tty(tty, inode, idx);
 	mutex_unlock(&tty_mutex);
 
 	return 0;
@@ -2078,7 +2082,7 @@ retry_open:
 				tty = ERR_PTR(retval);
 			}
 		} else { /* Returns with the tty_lock held for now */
-			tty = tty_init_dev(driver, index);
+			tty = tty_init_dev(driver, inode, index);
 			mutex_unlock(&tty_mutex);
 		}
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c7c5d3c..94c3983 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -506,7 +506,8 @@ extern struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx);
 extern int tty_alloc_file(struct file *file);
 extern void tty_add_file(struct tty_struct *tty, struct file *file);
 extern void tty_free_file(struct file *file);
-extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx);
+extern struct tty_struct *tty_init_dev(struct tty_driver *driver,
+				       struct inode *inode, int idx);
 extern int tty_release(struct inode *inode, struct file *filp);
 extern void tty_init_termios(struct tty_struct *tty);
 extern int tty_standard_install(struct tty_driver *driver,
diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 1610524..c7aa71c 100644
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -22,7 +22,7 @@
  *
  *	Optional method. Default behaviour is to use the ttys array
  *
- * void (*remove)(struct tty_driver *self, struct tty_struct *tty)
+ * void (*remove)(struct tty_driver *self, struct inode *inode, int idx)
  *
  *	Remove a closed tty from the tty driver internal tables. Used in
  *	conjunction with lookup and remove methods.
@@ -252,7 +252,7 @@ struct tty_operations {
 	struct tty_struct * (*lookup)(struct tty_driver *driver,
 			struct inode *inode, int idx);
 	int  (*install)(struct tty_driver *driver, struct tty_struct *tty);
-	void (*remove)(struct tty_driver *driver, struct tty_struct *tty);
+	void (*remove)(struct tty_driver *driver, struct inode *inode, int idx);
 	int  (*open)(struct tty_struct * tty, struct file * filp);
 	void (*close)(struct tty_struct * tty, struct file * filp);
 	void (*shutdown)(struct tty_struct *tty);
-- 
2.6.3


  parent reply	other threads:[~2015-11-28  2:28 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-28  2:25 [PATCH 00/12] Rework tty_reopen() Peter Hurley
2015-11-28  2:25 ` [PATCH 01/12] tty: Fix ldisc leak in failed tty_init_dev() Peter Hurley
2015-11-28  2:25 ` [PATCH 02/12] tty: Remove !tty check from free_tty_struct() Peter Hurley
2015-11-28  2:25 ` [PATCH 03/12] tty: Fix tty_init_termios() declaration Peter Hurley
2015-11-28  2:25 ` [PATCH 04/12] tty: Re-declare tty_driver_remove_tty() file scope Peter Hurley
2015-11-28  2:25 ` [PATCH 05/12] pty: Remove pty_unix98_shutdown() Peter Hurley
2015-11-28  2:25 ` [PATCH 06/12] tty: Remove __lockfunc annotation from tty lock functions Peter Hurley
2015-11-28  2:25 ` [PATCH 07/12] tty: Wait interruptibly for tty lock on reopen Peter Hurley
2015-11-28  2:25 ` [PATCH 08/12] pty: Prepare to redefine tty driver remove() interface Peter Hurley
2015-11-28  2:25 ` Peter Hurley [this message]
2015-11-28  2:25 ` [PATCH 10/12] tty: Consolidate noctty checks in tty_open() Peter Hurley
2015-11-28  2:25 ` [PATCH 11/12] tty: Refactor tty_open() Peter Hurley
2015-11-28  2:25 ` [PATCH 12/12] tty: Retry failed reopen if tty teardown in-progress Peter Hurley
2015-12-16 15:43 ` [PATCH 00/12] Rework tty_reopen() Peter Hurley
2015-12-17  5:53   ` Pratyush Anand
2015-12-17  7:15   ` Greg Kroah-Hartman
2016-01-10  5:13 ` [PATCH v2 00/10] " Peter Hurley
2016-01-10  5:13   ` [PATCH v2 01/10] tty: Wait interruptibly for tty lock on reopen Peter Hurley
2016-01-10  5:13   ` [PATCH v2 02/10] tty: Retry failed reopen if tty teardown in-progress Peter Hurley
2016-01-10  5:13   ` [PATCH v2 03/10] tty: Fix ldisc leak in failed tty_init_dev() Peter Hurley
2016-01-10  5:13   ` [PATCH v2 04/10] tty: Remove !tty check from free_tty_struct() Peter Hurley
2016-01-10  5:13   ` [PATCH v2 05/10] tty: Fix tty_init_termios() declaration Peter Hurley
2016-01-19  9:17     ` Johan Hovold
2016-01-10  5:13   ` [PATCH v2 06/10] tty: Re-declare tty_driver_remove_tty() file scope Peter Hurley
2016-01-10  5:13   ` [PATCH v2 07/10] pty: Remove pty_unix98_shutdown() Peter Hurley
2016-01-10  5:13   ` [PATCH v2 08/10] tty: Remove __lockfunc annotation from tty lock functions Peter Hurley
2016-01-10  5:13   ` [PATCH v2 09/10] tty: Consolidate noctty checks in tty_open() Peter Hurley
2016-03-26 17:58     ` Richard Weinberger
2016-03-26 19:06       ` Peter Hurley
2016-03-26 19:14         ` Richard Weinberger
2016-03-26 19:14           ` [uml-devel] " Richard Weinberger
2016-01-10  5:13   ` [PATCH v2 10/10] tty: Refactor tty_open() Peter Hurley

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=1448677557-16420-10-git-send-email-peter@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@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.