unaddressed issues around tty_open_by_driver

[Okash Khawaja]

Hi,

On Tue, Jun 27, 2017 at 12:27:14PM +0200, Samuel Thibault wrote:
> Okash Khawaja, on mar. 27 juin 2017 11:20:34 +0100, wrote:
> > On Tue, Jun 27, 2017 at 11:50:18AM +0200, Samuel Thibault wrote:
> > > Okash Khawaja, on mar. 27 juin 2017 10:44:45 +0100, wrote:
> > > > Next, I think, we need to restore tty->termios.c_line to the ldisc
> > > > before speakup was loaded, because that's what is used when opening tty
> > > > [1]. Locally, I have done that by caching original ldisc and then
> > > > calling tty_set_termios_ldisc on module exit which resolves that issue.
> > > > However, in kernel tty_set_termios_ldisc is static function and will
> > > > need to be exported if we want to call it.
> > > 
> > > Mmm, it looks odd to be doing such kind of hackery. It'd probably be
> > > good to check how other line disciplines are doing.
> > Other ldiscs don't call tty_set_ldisc. They seem to rely on user space
> > setting the ldisc and hence to clean up also. They just register ldisc
> > on module init and unregister it on module exit.
> 
> Ah, right. So we need to do what the unregistration triggered by
> userland achieves, ok.
> 
> So yes, it could make sense to have to unstaticify some functions.
> 

Regarding tty->count mismatch, it seems like several things depend on
tty->count and manipulating it won't be straightforward. More generally,
existing code to open and close tty is designed to be triggered from user
space. I think some of the problems mentioned above are there because we
are trying to overlaod existing user space oriented code with kernel
space.

Rather than dealing with subtleties of changing this code to allow
kernel access to tty, I suggest we create a separate kernel api which is
clean and simple. That way, we also avoid increasing complexity of
existing code. Here are broad lines of what I am suggesting.

Two functions:

struct tty_struct *tty_kopen(dev_t device);
void tty_kclose(struct tty_struct *tty, int idx);

And a tty->flag:

TTY_KOPENED to indicate that this tty_struct is in use by kernel.

tty_kopen will be a subset of tty_open_by_driver that we currently use
to open tty from kernel, i.e. it will only take the path that is valid
for opening from kernel. It will also check for and set TTY_KOPENED
flag.

tty_kclose will be similar to tty_release_struct. It will clear
TTY_KOPENED flag.

tty_open method in tty_io.c will check for TTY_KOPENED and return
-EBUSY if it is set.

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.

Thanks,
Okash