All of lore.kernel.org
 help / color / mirror / Atom feed
From: Okash Khawaja <okash.khawaja@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>,
	Samuel Thibault <samuel.thibault@ens-lyon.org>,
	Alan Cox <gnomes@lxorguk.ukuu.org.uk>,
	linux-kernel@vger.kernel.org
Cc: William Hubbs <w.d.hubbs@gmail.com>,
	Chris Brannon <chris@the-brannons.com>,
	Kirk Reiser <kirk@reisers.ca>,
	speakup@linux-speakup.org, devel@driverdev.osuosl.org,
	Okash Khawaja <okash.khawaja@gmail.com>
Subject: [patch 1/3] tty: resolve tty contention between kernel and user space
Date: Sun, 09 Jul 2017 12:41:54 +0100	[thread overview]
Message-ID: <20170709114426.618570903@gmail.com> (raw)
In-Reply-To: 20170709114153.157783481@gmail.com

[-- Attachment #1: 17_add_tty_kopen --]
[-- Type: text/plain, Size: 4377 bytes --]

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.

This patch makes kernel access to tty exclusive, so that if a user
process or kernel opens a kernel opened tty, it gets -EBUSY. It 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.

Returning -EBUSY on tty open is a change in the interface. I have
tested this with minicom, picocom and commands like "echo foo >
/dev/ttyS0". They all correctly report "Device or resource busy" when
the tty is already kernel opened.

Signed-off-by: Okash Khawaja <okash.khawaja@gmail.com>

---
 drivers/tty/tty_io.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/tty.h  |    4 +++
 2 files changed, 58 insertions(+)

--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1786,6 +1786,54 @@ 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) {
+		/* drop kref from tty_driver_lookup_tty() */
+		tty_kref_put(tty);
+		tty = ERR_PTR(-EBUSY);
+	} else { /* tty_init_dev returns tty with the tty_lock held */
+		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_GPL(tty_kopen);
+
+/**
  *	tty_open_by_driver	-	open a tty device
  *	@device: dev_t of device to open
  *	@inode: inode of device file
@@ -1824,6 +1872,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() */
--- 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
@@ -401,6 +402,7 @@ 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)
@@ -425,6 +427,8 @@ static inline const char *tty_name(const
 static inline struct tty_struct *tty_open_by_driver(dev_t device,
 		struct inode *inode, struct file *filp)
 { return NULL; }
+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; }
 #endif

  reply	other threads:[~2017-07-09 11:44 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-07 20:28 tty contention resulting from tty_open_by_device export Okash Khawaja
2017-07-08  8:38 ` Greg Kroah-Hartman
2017-07-08  9:01   ` Okash Khawaja
2017-07-09 11:41   ` [patch 0/3] " Okash Khawaja
2017-07-09 11:41     ` Okash Khawaja [this message]
2017-07-09 11:51       ` [patch 1/3] tty: resolve tty contention between kernel and user space Greg Kroah-Hartman
2017-07-09 15:04       ` Andy Shevchenko
2017-07-09 19:08         ` Okash Khawaja
2017-07-10  8:31         ` Okash Khawaja
2017-07-10 15:21           ` Andy Shevchenko
2017-07-10 16:12             ` Okash Khawaja
2017-07-09 11:41     ` [patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Okash Khawaja
2017-07-09 11:50       ` Greg Kroah-Hartman
2017-07-09 12:28         ` Okash Khawaja
2017-07-09 11:41     ` [patch 3/3] tty: undo export " Okash Khawaja
2017-07-09 11:57     ` [patch 0/3] Re: tty contention resulting from tty_open_by_device export Greg Kroah-Hartman
2017-07-09 12:32     ` [patch 4/3] tty: make tty_kopen return ENODEV in case of no TTY Okash Khawaja
2017-07-10 11:52     ` [patch 0/3] Re: tty contention resulting from tty_open_by_device export Alan Cox
2017-07-10 12:33       ` Okash Khawaja
2017-07-10 16:22         ` Okash Khawaja
2017-07-12 18:20         ` Alan Cox
2017-07-13 11:29           ` Okash Khawaja
2017-07-17 12:31             ` Greg Kroah-Hartman
2017-07-17 13:23               ` Okash Khawaja
2017-07-17 22:04                 ` Alan Cox
2017-07-18 11:29                   ` Okash Khawaja
2017-07-18 12:26                     ` Dan Carpenter
2017-07-18 19:22                       ` Okash Khawaja
2017-07-18 13:49                     ` Alan Cox
2017-07-20  7:22                       ` [patch v3 0/3] tty contention resulting from tty_open_by_driver export Okash Khawaja
2017-07-20  7:22                         ` [patch v3 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja
2017-07-20  7:22                         ` [patch v3 2/3] staging: speakup: use tty_kopen and tty_kclose Okash Khawaja
2017-07-20  7:22                         ` [patch v3 3/3] tty: undo export of tty_open_by_driver Okash Khawaja
2017-07-17 21:04               ` [patch v2 0/3] tty contention resulting from tty_open_by_driver export Okash Khawaja
2017-07-17 21:04                 ` [patch v2 1/3] tty: resolve tty contention between kernel and user space Okash Khawaja
2017-07-17 21:04                 ` [patch v2 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver Okash Khawaja
2017-07-17 21:04                 ` [patch v2 3/3] tty: undo export " Okash Khawaja

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=20170709114426.618570903@gmail.com \
    --to=okash.khawaja@gmail.com \
    --cc=chris@the-brannons.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=kirk@reisers.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=samuel.thibault@ens-lyon.org \
    --cc=speakup@linux-speakup.org \
    --cc=w.d.hubbs@gmail.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.