[BUG?] tty doesn't handle ^D at cooked/non-cooked switching corner case

[BUG?] tty doesn't handle ^D at cooked/non-cooked switching corner case

[KOSAKI Motohiro]

Hi tty expert,

Recently, ruby (scripting program language) developing group found
that linux doesn't handle ^D properly at cooked/non-cooked switching
corner case.

They (and I) think it isn't typical case, but scripting language vm
shouldn't hang up although script user write any crap and strange code.

They create following patch. is it right way?



reproduce way
--------------
1. run tty_test.sh (see below)
2. input any key before output "cooked"
3. input ^D, but do nothing


tty_test.sh
--------------
#!/bin/sh

exec 3<&0
cat <&3 &
pid=$!
stty -icanon
echo raw
sleep 1
stty icanon
echo cooked
wait $pid

---
 drivers/char/n_tty.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/char/n_tty.c
===================================================================
--- a/drivers/char/n_tty.c
+++ b/drivers/char/n_tty.c
@@ -1893,7 +1893,7 @@ do_it_again:
 			check_unthrottle(tty);
 		}
 
-		if (b - buf >= minimum)
+		if (b - buf >= (tty->icanon ? 0 : minimum))
 			break;
 		if (time)
 			timeout = time;

Re: [BUG?] tty doesn't handle ^D at cooked/non-cooked switching corner case

[Alan Cox]

On Tue, 30 Dec 2008 21:00:23 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

> Hi tty expert,
> 
> Recently, ruby (scripting program language) developing group found
> that linux doesn't handle ^D properly at cooked/non-cooked switching
> corner case.

I don't believe the behaviour of input characters typed across that
boundary is defined by the standards at all. That said I think the
behaviour you expect is reasonable.

> They create following patch. is it right way?

You could recompute minimum in this case but zero is definitely
incorrect. I think what you would actually need to do is to extract out
the code just below do_it_again: which computes minimum and recompute it
properly.

Alan

Re: [BUG?] tty doesn't handle ^D at cooked/non-cooked switching corner case

[KOSAKI Motohiro]

Hi

Sorry for late responce.


> On Tue, 30 Dec 2008 21:00:23 +0900 (JST)
> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:
> 
> > Hi tty expert,
> > 
> > Recently, ruby (scripting program language) developing group found
> > that linux doesn't handle ^D properly at cooked/non-cooked switching
> > corner case.
> 
> I don't believe the behaviour of input characters typed across that
> boundary is defined by the standards at all. That said I think the
> behaviour you expect is reasonable.
> 
> > They create following patch. is it right way?
> 
> You could recompute minimum in this case but zero is definitely
> incorrect. I think what you would actually need to do is to extract out
> the code just below do_it_again: which computes minimum and recompute it
> properly.

How about this?



==
Subject: [PATCH] tty: recompute minimum every wakeup
Impact: corner case bugfix

Generally, tty can be controled by multiple process at the same time.
But unfortunately, linux tty doesn't consider another process might have changed
tty attribute when own process is sleeping.

if following bad scenario happend, tty line discipline can't handle ^D.


reproduce way
=================
1. run tty_test.sh (see below)
2. input any key after "raw" is displayed
3. wait until "cooked" is displayed
4. input ^D  (oops, it's ignored. you can't exit from "cat")

tty_test.sh
--------------
#!/bin/sh

exec 3<&0
cat <&3 &
pid=$!
stty -icanon
echo raw
sleep 3
stty icanon
echo cooked
wait $pid
--------------


Reported-by: Tanaka Akira <akr@fsij.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/char/n_tty.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: b/drivers/char/n_tty.c
===================================================================
--- a/drivers/char/n_tty.c	2009-01-03 09:06:46.000000000 +0900
+++ b/drivers/char/n_tty.c	2009-01-03 09:08:15.000000000 +0900
@@ -1381,6 +1381,11 @@ do_it_again:
 			/* FIXME: does n_tty_set_room need locking ? */
 			n_tty_set_room(tty);
 			timeout = schedule_timeout(timeout);
+
+			/* Someone might have changed VMIN. check again. */
+			minimum = 0;
+			if (!tty->icanon)
+				minimum = MIN_CHAR(tty);
 			continue;
 		}
 		__set_current_state(TASK_RUNNING);