tty contention resulting from tty_open_by_device export

[Okash Khawaja]

Hi,

The commit 12e84c71b7d4ee (tty: export tty_open_by_driver) exports
tty_open_by_device to allow tty to be opened from inside kernel which
works fine except that it doesn't handle contention with user space or
another kernel-space open of the same tty. For example, opening a tty
from user space while it is kernel opened results in failure and a
kernel log message about mismatch between tty->count and tty's file open
count.

I suggest we make kernel access to tty exclusive so that if a user
process or kernel opens a kernel opened tty, it gets -EBUSY. Below is
a patch which does this by adding TTY_KOPENED flag to tty->flags. When
this flag is set, tty_open_by_driver returns -EBUSY. Instead of
overlaoding tty_open_by_driver for both kernel and user space, this
patch creates a separate function tty_kopen which closely follows
tty_open_by_driver.

I am far from an expert on tty and this patch might contain the wrong
approach. But it should convey what I mean.

Thanks,
Okash

---
 drivers/staging/speakup/spk_ttyio.c |    2 -
 drivers/tty/tty_io.c                |   56 ++++++++++++++++++++++++++++++++++--
 include/linux/tty.h                 |    7 +---
 3 files changed, 58 insertions(+), 7 deletions(-)

--- a/drivers/staging/speakup/spk_ttyio.c
+++ b/drivers/staging/speakup/spk_ttyio.c
@@ -164,7 +164,7 @@ static int spk_ttyio_initialise_ldisc(st
 	if (ret)
 		return ret;
 
-	tty = tty_open_by_driver(dev, NULL, NULL);
+	tty = tty_kopen(dev);
 	if (IS_ERR(tty))
 		return PTR_ERR(tty);
 
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1786,6 +1786,53 @@ static struct tty_driver *tty_lookup_dri
 }
 
 /**
+ *	tty_kopen	-	open a tty device for kernel
+ *	@device: dev_t of device to open
+ *
+ *	Opens tty exclusively for kernel. Performs the driver lookup,
+ *	makes sure it's not already opened and performs the first-time
+ *	tty initialization.
+ *
+ *	Returns the locked initialized &tty_struct
+ *
+ *	Claims the global tty_mutex to serialize:
+ *	  - concurrent first-time tty initialization
+ *	  - concurrent tty driver removal w/ lookup
+ *	  - concurrent tty removal from driver table
+ */
+struct tty_struct *tty_kopen(dev_t device)
+{
+        struct tty_struct *tty;
+        struct tty_driver *driver = NULL;
+        int index = -1;
+
+        mutex_lock(&tty_mutex);
+        driver = tty_lookup_driver(device, NULL, &index);
+        if (IS_ERR(driver)) {
+                mutex_unlock(&tty_mutex);
+                return ERR_CAST(driver);
+        }
+
+        /* check whether we're reopening an existing tty */
+        tty = tty_driver_lookup_tty(driver, NULL, index);
+        if (IS_ERR(tty))
+                goto out;
+
+        if (tty) {
+                tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
+                tty = ERR_PTR(-EBUSY);
+        } else { /* Returns with the tty_lock held for now */
+                tty = tty_init_dev(driver, index);
+                set_bit(TTY_KOPENED, &tty->flags);
+        }
+out:
+        mutex_unlock(&tty_mutex);
+        tty_driver_kref_put(driver);
+        return tty;
+}
+EXPORT_SYMBOL(tty_kopen);
+
+/**
  *	tty_open_by_driver	-	open a tty device
  *	@device: dev_t of device to open
  *	@inode: inode of device file
@@ -1801,7 +1848,7 @@ static struct tty_driver *tty_lookup_dri
  *	  - concurrent tty driver removal w/ lookup
  *	  - concurrent tty removal from driver table
  */
-struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
 					     struct file *filp)
 {
 	struct tty_struct *tty;
@@ -1824,6 +1871,12 @@ struct tty_struct *tty_open_by_driver(de
 	}
 
 	if (tty) {
+		if (test_bit(TTY_KOPENED, &tty->flags)) {
+			tty_kref_put(tty);
+			mutex_unlock(&tty_mutex);
+			tty = ERR_PTR(-EBUSY);
+			goto out;
+		}
 		mutex_unlock(&tty_mutex);
 		retval = tty_lock_interruptible(tty);
 		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
@@ -1846,7 +1899,6 @@ out:
 	tty_driver_kref_put(driver);
 	return tty;
 }
-EXPORT_SYMBOL_GPL(tty_open_by_driver);
 
 /**
  *	tty_open		-	open a tty device
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -362,6 +362,7 @@ struct tty_file_private {
 #define TTY_NO_WRITE_SPLIT 	17	/* Preserve write boundaries to driver */
 #define TTY_HUPPED 		18	/* Post driver->hangup() */
 #define TTY_LDISC_HALTED	22	/* Line discipline is halted */
+#define TTY_KOPENED             23      /* TTY exclusively opened by kernel */
 
 /* Values for tty->flow_change */
 #define TTY_THROTTLE_SAFE 1
@@ -399,8 +400,7 @@ extern struct tty_struct *get_current_tt
 /* tty_io.c */
 extern int __init tty_init(void);
 extern const char *tty_name(const struct tty_struct *tty);
-extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
-		struct file *filp);
+extern struct tty_struct *tty_kopen(dev_t device);
 extern int tty_dev_name_to_number(const char *name, dev_t *number);
 #else
 static inline void tty_kref_put(struct tty_struct *tty)
@@ -422,8 +422,7 @@ static inline int __init tty_init(void)
 { return 0; }
 static inline const char *tty_name(const struct tty_struct *tty)
 { return "(none)"; }
-static inline struct tty_struct *tty_open_by_driver(dev_t device,
-		struct inode *inode, struct file *filp)
+static inline struct tty_struct *tty_kopen(dev_t device)
 { return NULL; }
 static inline int tty_dev_name_to_number(const char *name, dev_t *number)
 { return -ENOTSUPP; }