unaddressed issues around tty_open_by_driver

[Okash Khawaja]

Hi,

On Tue, Jun 27, 2017 at 01:38:35AM +0200, Samuel Thibault wrote:
> Okash Khawaja, on dim. 25 juin 2017 10:21:26 +0100, wrote:
> > Third one is interesting. I haven't investigated it yet, but it seems to
> > throw a kernel oops from tty_ldisc_reinit.
> 
> So it's probably some missing cleanups in the speakup code, which just
> needs fixing :)
Yes, there are a couple of things. The kernel oops comes from ldisc.
Because we call tty_ldisc_release on module exit but don't set
tty->ldiscs[N_SPEAKUP] to NULL, the next time that tty is opened, it
tries to use N_SPEAKUP ldisc whose memory was released in
tty_ldisc_release. Calling tty_ldisc_unregister resolves it by setting
tty->ldiscs[N_SPEAKUP] to NULL.

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.

Finally, there is tty->count mismatch. On module exit, we don't
decrement tty->count which was incremented by tty_open_by_driver. I
don't know yet how to fix that.

So in summary, we need above changes, and also the flags to prevent
contention between user and kernel space.

> 
> > We can avoid 2 and 3 above by calling tty_open_by_driver only when
> > speakup is built into kernel and not when loaded as module.
> 
> That looks odd to me :)
Yes it does :) Not very familiar with kernel side use cases yet. My
thinking was to use different way of acquiring tty_struct when loaded as
module, because then we will have the device file. Probably not very
elegant.

Thanks,
Okash

[1]
http://elixir.free-electrons.com/linux/latest/source/drivers/tty/tty_io.c#L1484