On Sun, Jul 09, 2017 at 12:41:54PM +0100, Okash Khawaja wrote: > 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.khaw...@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; }
Like I said in response to patch 2, maybe this should return an error? thanks, greg k-h