Re: Bug in tty cooked mode kernel 3.12.3

Re: Bug in tty cooked mode kernel 3.12.3

[Greg KH]

On Sat, Dec 07, 2013 at 06:46:07PM -0500, Karl Dahlke wrote:
> This may have appeared in 3.9.11 with the changes to tty.
> Don't know; I just started running 3.12 from kernel.org.
> Perhaps nobody saw it before because nobody runs command line programs,
> we're all on desktops or x or whatever,
> and even those in the command line are still raw.
> bash is raw by default, but I run it cooked.
> You can supress its readline functions like this.
> 
> set +o emacs
> set +o vi
> set +o histexpand
> set +o history
> 
> That puts its tty back into cooked mode.
> 
> Come up in run level 3, command line mode,
> and bring up two consoles with bash in cooked mode as above.
> Set a simple prompt like this.
> 
> PS1='$ '
> 
> Switch to console 2 via alt-f2.
> Hit return.
> The cursor should drop to a new line then give you the $ prompt.
> But it puts the $ prompt next too the old one, then drops down
> to a blank line.
> The $ prompt and the crlf are out of sequence.
> Hit return a few more times and it will straighten itself out.
> Then switch back to console 1 alt-f1
> Hit return and the same bug,
> $ $
> 
> hit return a few more times to straighten it out.
> Switch back to console 2 and the same problem.
> For me it's repeatable.
> 
> It's also very confusing when I'm running other command line programs with cooked tty.
> For unknown reasons the crlf can come out at the wrong time,
> breaking lines or leaving lines together.
> My editor can run either way, so I've switched it over to readline mode
> and I haven't noticed any problems this way, yet.
> But I would prefer the cooked mode.
> 
> I looked through MAINTAINERS but couldn't find
> a clear maintainer for drivers/tty/tty*.c

Did you see the big "TTY LAYER" section in MAINTAINERS?  :)

Anyway, we've had a number of fixes in this area, I've just sent a pull
request to Linus to fix this issue (I think.)  If you could test the
patch below, that would be great.

thanks,

greg k-h

----------------
commit 39434abd942c8e4b9c14c06a03b3245beaf8467f
Author: Peter Hurley <peter@hurleysoftware.com>
Date:   Fri Nov 29 12:56:10 2013 -0500

    n_tty: Fix missing newline echo
    
    When L_ECHONL is on, newlines are echoed regardless of the L_ECHO
    state; if set, ensure accumulated echoes are flushed before finishing
    the current input processing and before more output.
    
    Cc: <stable@vger.kernel.org> # 3.12.x
    Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com
    Tested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
    Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0f74945af624..268b62768f2b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -810,7 +810,8 @@ static void process_echoes(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_tail)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_tail)
 		return;
 
 	mutex_lock(&ldata->output_lock);
@@ -825,7 +826,8 @@ static void flush_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (!L_ECHO(tty) || ldata->echo_commit == ldata->echo_head)
+	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
+	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);

Bug in tty cooked mode kernel 3.12.3

[Karl Dahlke]

As per your direction, I applied Peters patch to n_tty.c,
which looked like it was in the ballpark, echoing newline etc,
but it did not fix the problem.
It is still reproducible on my system as below.

> Come up in run level 3, command line mode,
> and bring up two consoles with bash in cooked mode.
> Set a simple prompt like this.
>
> PS1='$ '
>
> Switch to console 2 via alt-f2.
> Hit return.
> The cursor should drop to a new line then give you the $ prompt.
> But it puts the $ prompt next too the old one, then drops down
> to a blank line.
> The $ prompt and the crlf are out of sequence.
> Hit return a few more times and it will straighten itself out.
> Then switch back to console 1 alt-f1
> Hit return and the same bug,
> $ $
>
> hit return a few more times to straighten it out.
> Switch back to console 2 and the same problem.

Thank you for your quick reply,and the patch to try.
I'm afraid however this is still an unsolved problem.
Realize that it pops up rarely and randomly in just about any cooked program;
I chose the console switch example because it most reliably reproduces the bug.

Thank you.

Karl Dahlke

Re: Bug in tty cooked mode kernel 3.12.3

[Greg KH]

On Sun, Dec 08, 2013 at 01:16:09AM -0500, Karl Dahlke wrote:
> As per your direction, I applied Peters patch to n_tty.c,
> which looked like it was in the ballpark, echoing newline etc,
> but it did not fix the problem.
> It is still reproducible on my system as below.
> 
> > Come up in run level 3, command line mode,
> > and bring up two consoles with bash in cooked mode.
> > Set a simple prompt like this.
> >
> > PS1='$ '
> >
> > Switch to console 2 via alt-f2.
> > Hit return.
> > The cursor should drop to a new line then give you the $ prompt.
> > But it puts the $ prompt next too the old one, then drops down
> > to a blank line.
> > The $ prompt and the crlf are out of sequence.
> > Hit return a few more times and it will straighten itself out.
> > Then switch back to console 1 alt-f1
> > Hit return and the same bug,
> > $ $
> >
> > hit return a few more times to straighten it out.
> > Switch back to console 2 and the same problem.
> 
> Thank you for your quick reply,and the patch to try.
> I'm afraid however this is still an unsolved problem.
> Realize that it pops up rarely and randomly in just about any cooked program;
> I chose the console switch example because it most reliably reproduces the bug.

Ah, sorry, wrong patch series.

Search lkml for:
	Subject: [PATCH tty-next 0/4] tty: Fix ^C echo
for a 4 patch series that should resolve this issue.  If you could test
that out, and let us know, that would be great.

thanks,

greg k-h

Re: Bug in tty cooked mode kernel 3.12.3

[Peter Hurley]

On 12/07/2013 06:46 PM, Karl Dahlke wrote:
> This may have appeared in 3.9.11 with the changes to tty.
> Don't know; I just started running 3.12 from kernel.org.
> Perhaps nobody saw it before because nobody runs command line programs,
> we're all on desktops or x or whatever,
> and even those in the command line are still raw.
> bash is raw by default, but I run it cooked.
> You can supress its readline functions like this.
> 
> set +o emacs
> set +o vi
> set +o histexpand
> set +o history
> 
> That puts its tty back into cooked mode.
> 
> Come up in run level 3, command line mode,
> and bring up two consoles with bash in cooked mode as above.
> Set a simple prompt like this.
> 
> PS1='$ '
> 
> Switch to console 2 via alt-f2.
> Hit return.
> The cursor should drop to a new line then give you the $ prompt.
> But it puts the $ prompt next too the old one, then drops down
> to a blank line.
> The $ prompt and the crlf are out of sequence.
> Hit return a few more times and it will straighten itself out.
> Then switch back to console 1 alt-f1
> Hit return and the same bug,
> $ $
> 
> hit return a few more times to straighten it out.
> Switch back to console 2 and the same problem.
> For me it's repeatable.
> 
> It's also very confusing when I'm running other command line programs with cooked tty.
> For unknown reasons the crlf can come out at the wrong time,
> breaking lines or leaving lines together.
> My editor can run either way, so I've switched it over to readline mode
> and I haven't noticed any problems this way, yet.
> But I would prefer the cooked mode.
> 
> I looked through MAINTAINERS but couldn't find
> a clear maintainer for drivers/tty/tty*.c
> Please forward this to the appropriate people.

Thanks for the report, Karl.

Please test the patch below (requires
commit 39434abd942c8e4b9c14c06a03b3245beaf8467f,
'n_tty: Fix missing newline echo').
This should fix the 'newline output after the new prompt' problem.

Note however that the other output order you observe is correct:
you press enter, a newline is echoed to terminal which mixes
with program output, the program ends, and the cooked mode
shell outputs an extra prompt because it reads a newline (which
the program did not read).

Regards,
Peter Hurley

--- >% ---
From: Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH] n_tty: Fix apparent order of echoed output

With block processing of echoed output, observed output order is still
required. Push completed echoes and echo commands prior to output.

Introduce echo_mark echo buffer index, which tracks completed echo
commands; ie., those submitted via commit_echoes but which may not
have been committed. Ensure that completed echoes are output prior
to subsequent terminal writes in process_echoes().

Fixes newline/prompt output order in cooked mode shell.

Cc: <stable@vger.kernel.org> # 3.12.x : 39434ab n_tty: Fix missing newline echo
Reported-by: Karl Dahlke <eklhad@comcast.net>
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ab24fe1..e9304b6 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -94,6 +94,7 @@ struct n_tty_data {
 	size_t canon_head;
 	size_t echo_head;
 	size_t echo_commit;
+	size_t echo_mark;
 	DECLARE_BITMAP(char_map, 256);
 
 	/* private to n_tty_receive_overrun (single-threaded) */
@@ -339,6 +340,7 @@ static void reset_buffer_flags(struct n_tty_data *ldata)
 {
 	ldata->read_head = ldata->canon_head = ldata->read_tail = 0;
 	ldata->echo_head = ldata->echo_tail = ldata->echo_commit = 0;
+	ldata->echo_mark = 0;
 	ldata->line_start = 0;
 
 	ldata->erasing = 0;
@@ -791,6 +793,7 @@ static void commit_echoes(struct tty_struct *tty)
 	size_t head;
 
 	head = ldata->echo_head;
+	ldata->echo_mark = head;
 	old = ldata->echo_commit - ldata->echo_tail;
 
 	/* Process committed echoes if the accumulated # of bytes
@@ -815,10 +818,11 @@ static void process_echoes(struct tty_struct *tty)
 	size_t echoed;
 
 	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
-	    ldata->echo_commit == ldata->echo_tail)
+	    ldata->echo_mark == ldata->echo_tail)
 		return;
 
 	mutex_lock(&ldata->output_lock);
+	ldata->echo_commit = ldata->echo_mark;
 	echoed = __process_echoes(tty);
 	mutex_unlock(&ldata->output_lock);
 
@@ -826,6 +830,7 @@ static void process_echoes(struct tty_struct *tty)
 		tty->ops->flush_chars(tty);
 }
 
+/* NB: echo_mark and echo_head should be equivalent here */
 static void flush_echoes(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
-- 
1.8.1.2

Bug in tty cooked mode kernel 3.12.3

[Karl Dahlke]

> Thanks for the report, Karl.
> 
> Please test the patch below (requires
> commit 39434abd942c8e4b9c14c06a03b3245beaf8467f,
> 'n_tty: Fix missing newline echo').
> This should fix the 'newline output after the new prompt' problem.

Hey hey - you did it!
I did my console switch, which *always* revealed the bug,
and it is not there any more.
I've also spent a couple hours in cooked mode programs, like my editor,
and the lines are all correct.
I'll keep at it for a few days, but for now I'm going to say it's fixed.
Please push this into the next release, and perhaps backport
if possible; for those of us in cooked line mode
it was really an inconvenient bug.
Finally, my complements to you.
I read the code in n_tty.c and I don't understand any of it.
It's way beyond me.
Nice job, and thank you.


Karl Dahlke

Re: Bug in tty cooked mode kernel 3.12.3

[Greg Kroah-Hartman]

On Mon, Dec 09, 2013 at 06:06:07PM -0500, Peter Hurley wrote:
> On 12/07/2013 06:46 PM, Karl Dahlke wrote:
> > This may have appeared in 3.9.11 with the changes to tty.
> > Don't know; I just started running 3.12 from kernel.org.
> > Perhaps nobody saw it before because nobody runs command line programs,
> > we're all on desktops or x or whatever,
> > and even those in the command line are still raw.
> > bash is raw by default, but I run it cooked.
> > You can supress its readline functions like this.
> > 
> > set +o emacs
> > set +o vi
> > set +o histexpand
> > set +o history
> > 
> > That puts its tty back into cooked mode.
> > 
> > Come up in run level 3, command line mode,
> > and bring up two consoles with bash in cooked mode as above.
> > Set a simple prompt like this.
> > 
> > PS1='$ '
> > 
> > Switch to console 2 via alt-f2.
> > Hit return.
> > The cursor should drop to a new line then give you the $ prompt.
> > But it puts the $ prompt next too the old one, then drops down
> > to a blank line.
> > The $ prompt and the crlf are out of sequence.
> > Hit return a few more times and it will straighten itself out.
> > Then switch back to console 1 alt-f1
> > Hit return and the same bug,
> > $ $
> > 
> > hit return a few more times to straighten it out.
> > Switch back to console 2 and the same problem.
> > For me it's repeatable.
> > 
> > It's also very confusing when I'm running other command line programs with cooked tty.
> > For unknown reasons the crlf can come out at the wrong time,
> > breaking lines or leaving lines together.
> > My editor can run either way, so I've switched it over to readline mode
> > and I haven't noticed any problems this way, yet.
> > But I would prefer the cooked mode.
> > 
> > I looked through MAINTAINERS but couldn't find
> > a clear maintainer for drivers/tty/tty*.c
> > Please forward this to the appropriate people.
> 
> Thanks for the report, Karl.
> 
> Please test the patch below (requires
> commit 39434abd942c8e4b9c14c06a03b3245beaf8467f,
> 'n_tty: Fix missing newline echo').
> This should fix the 'newline output after the new prompt' problem.
> 
> Note however that the other output order you observe is correct:
> you press enter, a newline is echoed to terminal which mixes
> with program output, the program ends, and the cooked mode
> shell outputs an extra prompt because it reads a newline (which
> the program did not read).
> 
> Regards,
> Peter Hurley
> 
> --- >% ---
> From: Peter Hurley <peter@hurleysoftware.com>
> Subject: [PATCH] n_tty: Fix apparent order of echoed output
> 
> With block processing of echoed output, observed output order is still
> required. Push completed echoes and echo commands prior to output.
> 
> Introduce echo_mark echo buffer index, which tracks completed echo
> commands; ie., those submitted via commit_echoes but which may not
> have been committed. Ensure that completed echoes are output prior
> to subsequent terminal writes in process_echoes().
> 
> Fixes newline/prompt output order in cooked mode shell.
> 
> Cc: <stable@vger.kernel.org> # 3.12.x : 39434ab n_tty: Fix missing newline echo
> Reported-by: Karl Dahlke <eklhad@comcast.net>
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/n_tty.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Do you want me to queue this up for 3.13-final, or have I already?

thanks,

greg k-h

Re: Bug in tty cooked mode kernel 3.12.3

[Peter Hurley]

On 12/17/2013 12:30 PM, Greg Kroah-Hartman wrote:
> On Mon, Dec 09, 2013 at 06:06:07PM -0500, Peter Hurley wrote:
>> On 12/07/2013 06:46 PM, Karl Dahlke wrote:
>>> This may have appeared in 3.9.11 with the changes to tty.
>>> Don't know; I just started running 3.12 from kernel.org.
>>> Perhaps nobody saw it before because nobody runs command line programs,
>>> we're all on desktops or x or whatever,
>>> and even those in the command line are still raw.
>>> bash is raw by default, but I run it cooked.
>>> You can supress its readline functions like this.
>>>
>>> set +o emacs
>>> set +o vi
>>> set +o histexpand
>>> set +o history
>>>
>>> That puts its tty back into cooked mode.
>>>
>>> Come up in run level 3, command line mode,
>>> and bring up two consoles with bash in cooked mode as above.
>>> Set a simple prompt like this.
>>>
>>> PS1='$ '
>>>
>>> Switch to console 2 via alt-f2.
>>> Hit return.
>>> The cursor should drop to a new line then give you the $ prompt.
>>> But it puts the $ prompt next too the old one, then drops down
>>> to a blank line.
>>> The $ prompt and the crlf are out of sequence.
>>> Hit return a few more times and it will straighten itself out.
>>> Then switch back to console 1 alt-f1
>>> Hit return and the same bug,
>>> $ $
>>>
>>> hit return a few more times to straighten it out.
>>> Switch back to console 2 and the same problem.
>>> For me it's repeatable.
>>>
>>> It's also very confusing when I'm running other command line programs with cooked tty.
>>> For unknown reasons the crlf can come out at the wrong time,
>>> breaking lines or leaving lines together.
>>> My editor can run either way, so I've switched it over to readline mode
>>> and I haven't noticed any problems this way, yet.
>>> But I would prefer the cooked mode.
>>>
>>> I looked through MAINTAINERS but couldn't find
>>> a clear maintainer for drivers/tty/tty*.c
>>> Please forward this to the appropriate people.
>>
>> Thanks for the report, Karl.
>>
>> Please test the patch below (requires
>> commit 39434abd942c8e4b9c14c06a03b3245beaf8467f,
>> 'n_tty: Fix missing newline echo').
>> This should fix the 'newline output after the new prompt' problem.
>>
>> Note however that the other output order you observe is correct:
>> you press enter, a newline is echoed to terminal which mixes
>> with program output, the program ends, and the cooked mode
>> shell outputs an extra prompt because it reads a newline (which
>> the program did not read).
>>
>> Regards,
>> Peter Hurley
>>
>> --- >% ---
>> From: Peter Hurley <peter@hurleysoftware.com>
>> Subject: [PATCH] n_tty: Fix apparent order of echoed output
>>
>> With block processing of echoed output, observed output order is still
>> required. Push completed echoes and echo commands prior to output.
>>
>> Introduce echo_mark echo buffer index, which tracks completed echo
>> commands; ie., those submitted via commit_echoes but which may not
>> have been committed. Ensure that completed echoes are output prior
>> to subsequent terminal writes in process_echoes().
>>
>> Fixes newline/prompt output order in cooked mode shell.
>>
>> Cc: <stable@vger.kernel.org> # 3.12.x : 39434ab n_tty: Fix missing newline echo
>> Reported-by: Karl Dahlke <eklhad@comcast.net>
>> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>>   drivers/tty/n_tty.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> Do you want me to queue this up for 3.13-final, or have I already?

Yes, please queue this for 3.13.

Regards,
Peter Hurley