commit 83d817f410 broke my ability to use Linux with a braille display

commit 83d817f410 broke my ability to use Linux with a braille display

[Nicolas Pitre]

I use Linux with the help of a braille display and the brltty daemon. It 
turns out that the latest mainline kernel I can work with comes from 
commit 231f8fd0cc. Anything past that and I lose the ability to read the 
console barely a few seconds after the system has booted as brltty is 
thrown a wrench and the braille display becomes completely inoperable.

Things get somewhat better with commit c96cf923a9 as brltty is not 
longer incapacitated, but some programs would randomly crash. Even the 
very first login attempt won't work as I soon as I hit enter after my 
user name the password prompt is skipped over, just like if the enter 
key had been hit twice. Then lynx (the text web browser) would crash as 
soon as I switch the virtual console with LeftAlt+FN. Mind you, this 
isn't easy to perform bisection in those conditions.

And the worst commit i.e. 83d817f410 is marked for stable!  :-(

Some interaction with brltty must be at play here otherwise such 
breakage would never have survived up to the mainline kernel.

As far as latest mainline is concerned, I managed to reproduce at least 
one of the unwelcome behavior change (hoping that's all there is to this 
issue) with a very simple test case so you won't have to learn braille 
to debug this:

# from any vt, make sure tty40 is allocated and empty
openvt -c 40 -f -- true

# open it and wait on read()
cat /dev/tty40

# from a second vt, simply open tty40 again
true < /dev/tty40

# come back to the first vt and watch cat bailing out with EAGAIN.

Please fix.


Nicolas

Re: commit 83d817f410 broke my ability to use Linux with a braille display

[Vito Caputo]

On Fri, Jan 11, 2019 at 01:33:09PM -0500, Nicolas Pitre wrote:
> I use Linux with the help of a braille display and the brltty daemon. It 
> turns out that the latest mainline kernel I can work with comes from 
> commit 231f8fd0cc. Anything past that and I lose the ability to read the 
> console barely a few seconds after the system has booted as brltty is 
> thrown a wrench and the braille display becomes completely inoperable.
> 
> Things get somewhat better with commit c96cf923a9 as brltty is not 
> longer incapacitated, but some programs would randomly crash. Even the 
> very first login attempt won't work as I soon as I hit enter after my 
> user name the password prompt is skipped over, just like if the enter 
> key had been hit twice. Then lynx (the text web browser) would crash as 
> soon as I switch the virtual console with LeftAlt+FN. Mind you, this 
> isn't easy to perform bisection in those conditions.
> 
> And the worst commit i.e. 83d817f410 is marked for stable!  :-(
> 
> Some interaction with brltty must be at play here otherwise such 
> breakage would never have survived up to the mainline kernel.
> 
> As far as latest mainline is concerned, I managed to reproduce at least 
> one of the unwelcome behavior change (hoping that's all there is to this 
> issue) with a very simple test case so you won't have to learn braille 
> to debug this:
> 
> # from any vt, make sure tty40 is allocated and empty
> openvt -c 40 -f -- true
> 
> # open it and wait on read()
> cat /dev/tty40
> 
> # from a second vt, simply open tty40 again
> true < /dev/tty40
> 
> # come back to the first vt and watch cat bailing out with EAGAIN.
> 
> Please fix.
> 
> 
> Nicolas


This all sounds familiar, and I suspect this is the fix:

https://lkml.org/lkml/2019/1/8/1379

Regards,
Vito Caputo

Re: commit 83d817f410 broke my ability to use Linux with a braille display

[Nicolas Pitre]

On Fri, 11 Jan 2019, Vito Caputo wrote:

> On Fri, Jan 11, 2019 at 01:33:09PM -0500, Nicolas Pitre wrote:
> > I use Linux with the help of a braille display and the brltty daemon. It 
> > turns out that the latest mainline kernel I can work with comes from 
> > commit 231f8fd0cc. Anything past that and I lose the ability to read the 
> > console barely a few seconds after the system has booted as brltty is 
> > thrown a wrench and the braille display becomes completely inoperable.
> > 
> > Things get somewhat better with commit c96cf923a9 as brltty is not 
> > longer incapacitated, but some programs would randomly crash. Even the 
> > very first login attempt won't work as I soon as I hit enter after my 
> > user name the password prompt is skipped over, just like if the enter 
> > key had been hit twice. Then lynx (the text web browser) would crash as 
> > soon as I switch the virtual console with LeftAlt+FN. Mind you, this 
> > isn't easy to perform bisection in those conditions.
> > 
> > And the worst commit i.e. 83d817f410 is marked for stable!  :-(
> 
> This all sounds familiar, and I suspect this is the fix:
> 
> https://lkml.org/lkml/2019/1/8/1379

Yep, I confirm this patch did solve all my issues.


Nicolas

Re: commit 83d817f410 broke my ability to use Linux with a braille display

[Greg Kroah-Hartman]

On Fri, Jan 11, 2019 at 01:33:09PM -0500, Nicolas Pitre wrote:
> I use Linux with the help of a braille display and the brltty daemon. It 
> turns out that the latest mainline kernel I can work with comes from 
> commit 231f8fd0cc. Anything past that and I lose the ability to read the 
> console barely a few seconds after the system has booted as brltty is 
> thrown a wrench and the braille display becomes completely inoperable.
> 
> Things get somewhat better with commit c96cf923a9 as brltty is not 
> longer incapacitated, but some programs would randomly crash. Even the 
> very first login attempt won't work as I soon as I hit enter after my 
> user name the password prompt is skipped over, just like if the enter 
> key had been hit twice. Then lynx (the text web browser) would crash as 
> soon as I switch the virtual console with LeftAlt+FN. Mind you, this 
> isn't easy to perform bisection in those conditions.
> 
> And the worst commit i.e. 83d817f410 is marked for stable!  :-(
> 
> Some interaction with brltty must be at play here otherwise such 
> breakage would never have survived up to the mainline kernel.
> 
> As far as latest mainline is concerned, I managed to reproduce at least 
> one of the unwelcome behavior change (hoping that's all there is to this 
> issue) with a very simple test case so you won't have to learn braille 
> to debug this:
> 
> # from any vt, make sure tty40 is allocated and empty
> openvt -c 40 -f -- true
> 
> # open it and wait on read()
> cat /dev/tty40
> 
> # from a second vt, simply open tty40 again
> true < /dev/tty40
> 
> # come back to the first vt and watch cat bailing out with EAGAIN.
> 
> Please fix.

Please try the patch below, it was just queued up to my tree and should
resolve the issue.  If not, please let us know.

thanks,

greg k-h


From d3736d82e8169768218ee0ef68718875918091a0 Mon Sep 17 00:00:00 2001
From: Dmitry Safonov <dima@arista.com>
Date: Wed, 9 Jan 2019 01:17:40 +0000
Subject: tty: Don't hold ldisc lock in tty_reopen() if ldisc present

Try to get reference for ldisc during tty_reopen().
If ldisc present, we don't need to do tty_ldisc_reinit() and lock the
write side for line discipline semaphore.
Effectively, it optimizes fast-path for tty_reopen(), but more
importantly it won't interrupt ongoing IO on the tty as no ldisc change
is needed.
Fixes user-visible issue when tty_reopen() interrupted login process for
user with a long password, observed and reported by Lukas.

Fixes: c96cf923a98d ("tty: Don't block on IO when ldisc change is pending")
Fixes: 83d817f41070 ("tty: Hold tty_ldisc_lock() during tty_reopen()")
Cc: Jiri Slaby <jslaby@suse.com>
Reported-by: Lukas F. Hartmann <lukas@mntmn.com>
Tested-by: Lukas F. Hartmann <lukas@mntmn.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/tty_io.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index bfe9ad85b362..23c6fd238422 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1256,7 +1256,8 @@ static void tty_driver_remove_tty(struct tty_driver *driver, struct tty_struct *
 static int tty_reopen(struct tty_struct *tty)
 {
 	struct tty_driver *driver = tty->driver;
-	int retval;
+	struct tty_ldisc *ld;
+	int retval = 0;
 
 	if (driver->type == TTY_DRIVER_TYPE_PTY &&
 	    driver->subtype == PTY_TYPE_MASTER)
@@ -1268,13 +1269,18 @@ static int tty_reopen(struct tty_struct *tty)
 	if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN))
 		return -EBUSY;
 
-	retval = tty_ldisc_lock(tty, 5 * HZ);
-	if (retval)
-		return retval;
+	ld = tty_ldisc_ref_wait(tty);
+	if (ld) {
+		tty_ldisc_deref(ld);
+	} else {
+		retval = tty_ldisc_lock(tty, 5 * HZ);
+		if (retval)
+			return retval;
 
-	if (!tty->ldisc)
-		retval = tty_ldisc_reinit(tty, tty->termios.c_line);
-	tty_ldisc_unlock(tty);
+		if (!tty->ldisc)
+			retval = tty_ldisc_reinit(tty, tty->termios.c_line);
+		tty_ldisc_unlock(tty);
+	}
 
 	if (retval == 0)
 		tty->count++;
-- 
2.20.1

Re: commit 83d817f410 broke my ability to use Linux with a braille display

[Nicolas Pitre]

On Fri, 11 Jan 2019, Greg Kroah-Hartman wrote:

> On Fri, Jan 11, 2019 at 01:33:09PM -0500, Nicolas Pitre wrote:
> > I use Linux with the help of a braille display and the brltty daemon. It 
> > turns out that the latest mainline kernel I can work with comes from 
> > commit 231f8fd0cc. Anything past that and I lose the ability to read the 
> > console barely a few seconds after the system has booted as brltty is 
> > thrown a wrench and the braille display becomes completely inoperable.
> 
> Please try the patch below, it was just queued up to my tree and should
> resolve the issue.  If not, please let us know.

Yes, it works. Thanks.

I also re-validated all the vt/vcs patches I sent you this week for 
which I started entertaining some doubts about their stability. But they 
all work fine again with the tty fix applied.


Nicolas

Re: commit 83d817f410 broke my ability to use Linux with a braille display

[Greg Kroah-Hartman]

On Fri, Jan 11, 2019 at 03:10:04PM -0500, Nicolas Pitre wrote:
> On Fri, 11 Jan 2019, Greg Kroah-Hartman wrote:
> 
> > On Fri, Jan 11, 2019 at 01:33:09PM -0500, Nicolas Pitre wrote:
> > > I use Linux with the help of a braille display and the brltty daemon. It 
> > > turns out that the latest mainline kernel I can work with comes from 
> > > commit 231f8fd0cc. Anything past that and I lose the ability to read the 
> > > console barely a few seconds after the system has booted as brltty is 
> > > thrown a wrench and the braille display becomes completely inoperable.
> > 
> > Please try the patch below, it was just queued up to my tree and should
> > resolve the issue.  If not, please let us know.
> 
> Yes, it works. Thanks.
> 
> I also re-validated all the vt/vcs patches I sent you this week for 
> which I started entertaining some doubts about their stability. But they 
> all work fine again with the tty fix applied.

Great, thanks for testing, I will get this patch to Linus soon.

greg k-h