On Wed, Jun 28, 2017 at 09:51:55AM +0100, Okash Khawaja wrote: > So the idea is to make kopen of tty exclusive and protect that > exclusivity with TTY_KOPENED flag. That way we don't have to worry about > complexities, races etc of the regular user-space oriented tty code. Here's what I mean. I've done some rudimentary testing. It seems to clean up alright, allowing ttyUSB0 or ttyS0 to be reused by user space programs. It also prevents user program from using the tty held by speakup while speakup is loaded. We don't need tty_kclose though because we can instead call tty_release_struct which will free up the tty, removing the need to clear TTY_KOPENED flag. --- drivers/staging/speakup/spk_ttyio.c | 6 ++- drivers/tty/tty_io.c | 56 ++++++++++++++++++++++++++++++++++-- include/linux/tty.h | 7 +--- 3 files changed, 61 insertions(+), 8 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); @@ -300,7 +300,9 @@ void spk_ttyio_release(void) tty_ldisc_flush(speakup_tty); tty_unlock(speakup_tty); - tty_ldisc_release(speakup_tty); + tty_release_struct(speakup_tty, speakup_tty->index); + if (tty_unregister_ldisc(N_SPEAKUP)) + pr_warn("speakup: failed to unregister line discipline N_SPEAKUP\n"); } EXPORT_SYMBOL_GPL(spk_ttyio_release); --- 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; } _______________________________________________ Speakup mailing list Speakup@xxxxxxxxxxxxxxxxx http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup