The commit 12e84c71b7d4 ("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 overloading 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. tty_kclose closes the tty opened by tty_kopen. To address the mismatch between tty->count and #fd's, this patch adds #kopen's to the count before comparing it with tty->count. That way check_tty_count reflects correct usage count. 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@xxxxxxxxx> --- drivers/tty/tty_io.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++--- include/linux/tty.h | 21 ++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -280,7 +280,7 @@ static int check_tty_count(struct tty_st { #ifdef CHECK_TTY_COUNT struct list_head *p; - int count = 0; + int count = 0, kopen_count = 0; spin_lock(&tty->files_lock); list_for_each(p, &tty->tty_files) { @@ -291,10 +291,12 @@ static int check_tty_count(struct tty_st tty->driver->subtype == PTY_TYPE_SLAVE && tty->link && tty->link->count) count++; - if (tty->count != count) { - tty_warn(tty, "%s: tty->count(%d) != #fd's(%d)\n", - routine, tty->count, count); - return count; + if (tty_port_kopened(tty->port)) + kopen_count++; + if (tty->count != (count + kopen_count)) { + tty_warn(tty, "%s: tty->count(%d) != (#fd's(%d) + #kopen's(%d))\n", + routine, tty->count, count, kopen_count); + return (count + kopen_count); } #endif return 0; @@ -1513,6 +1515,38 @@ static int tty_release_checks(struct tty } /** + * tty_kclose - closes tty opened by tty_kopen + * @tty: tty device + * + * Performs the final steps to release and free a tty device. It is the + * same as tty_release_struct except that it also resets TTY_PORT_KOPENED + * flag on tty->port. + */ +void tty_kclose(struct tty_struct *tty) +{ + /* + * Ask the line discipline code to release its structures + */ + tty_ldisc_release(tty); + + /* Wait for pending work before tty destruction commmences */ + tty_flush_works(tty); + + tty_debug_hangup(tty, "freeing structure\n"); + /* + * The release_tty function takes care of the details of clearing + * the slots and preserving the termios structure. The tty_unlock_pair + * should be safe as we keep a kref while the tty is locked (so the + * unlock never unlocks a freed tty). + */ + mutex_lock(&tty_mutex); + tty_port_set_kopened(tty->port, 0); + release_tty(tty, tty->index); + mutex_unlock(&tty_mutex); +} +EXPORT_SYMBOL_GPL(tty_kclose); + +/** * tty_release_struct - release a tty struct * @tty: tty device * @idx: index of the tty @@ -1786,6 +1820,56 @@ 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); + if (IS_ERR(tty)) + goto out; + tty_port_set_kopened(tty->port, 1); + } +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 +1908,12 @@ struct tty_struct *tty_open_by_driver(de } if (tty) { + if (tty_port_kopened(tty->port)) { + 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 @@ -261,6 +261,8 @@ struct tty_port { */ #define TTY_PORT_CTS_FLOW 3 /* h/w flow control enabled */ #define TTY_PORT_CHECK_CD 4 /* carrier detect enabled */ +#define TTY_PORT_KOPENED 5 /* device exclusively opened by + kernel */ /* * Where all of the state associated with a tty is kept while the tty @@ -401,6 +403,8 @@ 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 void tty_kclose(struct tty_struct *tty); 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 +429,10 @@ 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 ERR_PTR(-ENODEV); } +static inline void tty_kclose(struct tty_struct *tty) +{ } static inline int tty_dev_name_to_number(const char *name, dev_t *number) { return -ENOTSUPP; } #endif @@ -652,6 +660,19 @@ static inline void tty_port_set_initiali clear_bit(TTY_PORT_INITIALIZED, &port->iflags); } +static inline bool tty_port_kopened(struct tty_port *port) +{ + return test_bit(TTY_PORT_KOPENED, &port->iflags); +} + +static inline void tty_port_set_kopened(struct tty_port *port, bool val) +{ + if (val) + set_bit(TTY_PORT_KOPENED, &port->iflags); + else + clear_bit(TTY_PORT_KOPENED, &port->iflags); +} + extern struct tty_struct *tty_port_tty_get(struct tty_port *port); extern void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty); extern int tty_port_carrier_raised(struct tty_port *port); _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel