public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] TTY: do not reset master's packet mode
@ 2013-01-15 22:26 Jiri Slaby
  2013-02-01 17:59 ` Josh Boyer
  2013-02-20 13:19 ` Karel Zak
  0 siblings, 2 replies; 6+ messages in thread
From: Jiri Slaby @ 2013-01-15 22:26 UTC (permalink / raw)
  To: gregkh; +Cc: alan, jirislaby, linux-kernel, Mauro Carvalho Chehab, Bryan Mason

Now that login from util-linux is forced to drop all references to a
TTY which it wants to hangup (to reach reference count 1) we are
seeing issues with telnet. When login closes its last reference to the
slave PTY, it also resets packet mode on the *master* side. And we
have a race here.

What telnet does is fork+exec of `login'. Then there are two
scenarios:
* `login' closes the slave TTY and resets thus master's packet mode,
  but even now telnet properly sets the mode, or
* `telnetd' sets packet mode on the master, `login' closes the slave
  TTY and resets master's packet mode.

The former case is OK. However the latter happens in much more cases,
by the order of magnitude to be precise. So when one tries to login to
such a messed telnet setup, they see the following:
inux login:
            ogin incorrect

Note the missing first letters -- telnet thinks it is still in the
packet mode, so when it receives "linux login" from `login', it
considers "l" as the type of the packet and strips it.

SuS does not mention how the implementation should behave. Both BSDs I
checked (Free and Net) do not reset the flag upon the last close.

By this I am resurrecting an old bug, see References. We are hitting
it regularly now, i.e. with updated util-linux, ergo login.

Here, I am changing a behavior introduced back in 2.1 times. It would
better have a long time testing before goes upstream.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Bryan Mason <bmason@redhat.com>
References: https://lkml.org/lkml/2009/11/11/223
References: https://bugzilla.redhat.com/show_bug.cgi?id=504703
References: https://bugzilla.novell.com/show_bug.cgi?id=797042
---
 drivers/tty/pty.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index d2fd6f0..d7690dcf 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -47,7 +47,6 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
 	/* Review - krefs on tty_link ?? */
 	if (!tty->link)
 		return;
-	tty->link->packet = 0;
 	set_bit(TTY_OTHER_CLOSED, &tty->link->flags);
 	wake_up_interruptible(&tty->link->read_wait);
 	wake_up_interruptible(&tty->link->write_wait);
-- 
1.8.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] TTY: do not reset master's packet mode
  2013-01-15 22:26 [PATCH] TTY: do not reset master's packet mode Jiri Slaby
@ 2013-02-01 17:59 ` Josh Boyer
  2013-02-04 15:55   ` Jiri Slaby
  2013-02-20 13:19 ` Karel Zak
  1 sibling, 1 reply; 6+ messages in thread
From: Josh Boyer @ 2013-02-01 17:59 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: gregkh, alan, jirislaby, linux-kernel, Mauro Carvalho Chehab,
	Bryan Mason

On Tue, Jan 15, 2013 at 5:26 PM, Jiri Slaby <jslaby@suse.cz> wrote:
> Now that login from util-linux is forced to drop all references to a
> TTY which it wants to hangup (to reach reference count 1) we are
> seeing issues with telnet. When login closes its last reference to the
> slave PTY, it also resets packet mode on the *master* side. And we
> have a race here.
>
> What telnet does is fork+exec of `login'. Then there are two
> scenarios:
> * `login' closes the slave TTY and resets thus master's packet mode,
>   but even now telnet properly sets the mode, or
> * `telnetd' sets packet mode on the master, `login' closes the slave
>   TTY and resets master's packet mode.
>
> The former case is OK. However the latter happens in much more cases,
> by the order of magnitude to be precise. So when one tries to login to
> such a messed telnet setup, they see the following:
> inux login:
>             ogin incorrect
>
> Note the missing first letters -- telnet thinks it is still in the
> packet mode, so when it receives "linux login" from `login', it
> considers "l" as the type of the packet and strips it.
>
> SuS does not mention how the implementation should behave. Both BSDs I
> checked (Free and Net) do not reset the flag upon the last close.
>
> By this I am resurrecting an old bug, see References. We are hitting
> it regularly now, i.e. with updated util-linux, ergo login.
>
> Here, I am changing a behavior introduced back in 2.1 times. It would
> better have a long time testing before goes upstream.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
> Cc: Bryan Mason <bmason@redhat.com>
> References: https://lkml.org/lkml/2009/11/11/223
> References: https://bugzilla.redhat.com/show_bug.cgi?id=504703
> References: https://bugzilla.novell.com/show_bug.cgi?id=797042

Shouldn't this be CC'd to stable?

josh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] TTY: do not reset master's packet mode
  2013-02-01 17:59 ` Josh Boyer
@ 2013-02-04 15:55   ` Jiri Slaby
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2013-02-04 15:55 UTC (permalink / raw)
  To: Josh Boyer
  Cc: gregkh, alan, jirislaby, linux-kernel, Mauro Carvalho Chehab,
	Bryan Mason

On 02/01/2013 06:59 PM, Josh Boyer wrote:
> On Tue, Jan 15, 2013 at 5:26 PM, Jiri Slaby <jslaby@suse.cz> wrote:
>> Now that login from util-linux is forced to drop all references to a
>> TTY which it wants to hangup (to reach reference count 1) we are
>> seeing issues with telnet. When login closes its last reference to the
>> slave PTY, it also resets packet mode on the *master* side. And we
>> have a race here.
>>
>> What telnet does is fork+exec of `login'. Then there are two
>> scenarios:
>> * `login' closes the slave TTY and resets thus master's packet mode,
>>   but even now telnet properly sets the mode, or
>> * `telnetd' sets packet mode on the master, `login' closes the slave
>>   TTY and resets master's packet mode.
>>
>> The former case is OK. However the latter happens in much more cases,
>> by the order of magnitude to be precise. So when one tries to login to
>> such a messed telnet setup, they see the following:
>> inux login:
>>             ogin incorrect
>>
>> Note the missing first letters -- telnet thinks it is still in the
>> packet mode, so when it receives "linux login" from `login', it
>> considers "l" as the type of the packet and strips it.
>>
>> SuS does not mention how the implementation should behave. Both BSDs I
>> checked (Free and Net) do not reset the flag upon the last close.
>>
>> By this I am resurrecting an old bug, see References. We are hitting
>> it regularly now, i.e. with updated util-linux, ergo login.
>>
>> Here, I am changing a behavior introduced back in 2.1 times. It would
>> better have a long time testing before goes upstream.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
>> Cc: Bryan Mason <bmason@redhat.com>
>> References: https://lkml.org/lkml/2009/11/11/223
>> References: https://bugzilla.redhat.com/show_bug.cgi?id=504703
>> References: https://bugzilla.novell.com/show_bug.cgi?id=797042
> 
> Shouldn't this be CC'd to stable?

Nope, it will be sent to stable as soon as it proves itself to be stable
enough. Note that we are changing very old code and the last thing we
want to do here is to break stable trees.

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] TTY: do not reset master's packet mode
  2013-01-15 22:26 [PATCH] TTY: do not reset master's packet mode Jiri Slaby
  2013-02-01 17:59 ` Josh Boyer
@ 2013-02-20 13:19 ` Karel Zak
  2013-02-20 16:40   ` Jiri Slaby
  1 sibling, 1 reply; 6+ messages in thread
From: Karel Zak @ 2013-02-20 13:19 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: gregkh, alan, jirislaby, linux-kernel, Mauro Carvalho Chehab,
	Bryan Mason

On Tue, Jan 15, 2013 at 11:26:22PM +0100, Jiri Slaby wrote:
> Now that login from util-linux is forced to drop all references to a
> TTY which it wants to hangup (to reach reference count 1) we are
> seeing issues with telnet. When login closes its last reference to the
> slave PTY, it also resets packet mode on the *master* side. And we
> have a race here.

[...]

> By this I am resurrecting an old bug, see References. We are hitting
> it regularly now, i.e. with updated util-linux, ergo login.
> 
> Here, I am changing a behavior introduced back in 2.1 times. It would
> better have a long time testing before goes upstream.

 It would be nice to found a quick way how to resolve this problem,
 because telnet+login is broken now... and for end users it seems like
 a regression.

 Maybe the requirement (patch) to close all references to TTY before
 hangup should reverted or we can make an exception in login(1) code to
 keep TTY open when login(1) is executed with -h (as used by telned).

    Karel
-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] TTY: do not reset master's packet mode
  2013-02-20 13:19 ` Karel Zak
@ 2013-02-20 16:40   ` Jiri Slaby
  2013-02-20 17:32     ` Karel Zak
  0 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2013-02-20 16:40 UTC (permalink / raw)
  To: Karel Zak
  Cc: gregkh, alan, jirislaby, linux-kernel, Mauro Carvalho Chehab,
	Bryan Mason

On 02/20/2013 02:19 PM, Karel Zak wrote:
> On Tue, Jan 15, 2013 at 11:26:22PM +0100, Jiri Slaby wrote:
>> Now that login from util-linux is forced to drop all references to a
>> TTY which it wants to hangup (to reach reference count 1) we are
>> seeing issues with telnet. When login closes its last reference to the
>> slave PTY, it also resets packet mode on the *master* side. And we
>> have a race here.
> 
> [...]
> 
>> By this I am resurrecting an old bug, see References. We are hitting
>> it regularly now, i.e. with updated util-linux, ergo login.
>>
>> Here, I am changing a behavior introduced back in 2.1 times. It would
>> better have a long time testing before goes upstream.
> 
>  It would be nice to found a quick way how to resolve this problem,
>  because telnet+login is broken now... and for end users it seems like
>  a regression.

Oh, the patch is in the TTY tree and I suppose it will go to 3.9-rc1
(and to stable as we will get to same later -rc -- to have some testing).

>  Maybe the requirement (patch) to close all references to TTY before
>  hangup should reverted or we can make an exception in login(1) code to
>  keep TTY open when login(1) is executed with -h (as used by telned).

If we reverted the patch in login, we would have to revert also a patch
in the TTY which closes a window allowing you to crash a kernel at will
(in fact that patch disallows multiple opens when hanging up a terminal.
IOW only the one to hang the terminal is allowed to have it opened.)

-- 
js
suse labs

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] TTY: do not reset master's packet mode
  2013-02-20 16:40   ` Jiri Slaby
@ 2013-02-20 17:32     ` Karel Zak
  0 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2013-02-20 17:32 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: gregkh, alan, jirislaby, linux-kernel, Mauro Carvalho Chehab,
	Bryan Mason

On Wed, Feb 20, 2013 at 05:40:34PM +0100, Jiri Slaby wrote:
> On 02/20/2013 02:19 PM, Karel Zak wrote:
> > On Tue, Jan 15, 2013 at 11:26:22PM +0100, Jiri Slaby wrote:
> >> Now that login from util-linux is forced to drop all references to a
> >> TTY which it wants to hangup (to reach reference count 1) we are
> >> seeing issues with telnet. When login closes its last reference to the
> >> slave PTY, it also resets packet mode on the *master* side. And we
> >> have a race here.
> > 
> > [...]
> > 
> >> By this I am resurrecting an old bug, see References. We are hitting
> >> it regularly now, i.e. with updated util-linux, ergo login.
> >>
> >> Here, I am changing a behavior introduced back in 2.1 times. It would
> >> better have a long time testing before goes upstream.
> > 
> >  It would be nice to found a quick way how to resolve this problem,
> >  because telnet+login is broken now... and for end users it seems like
> >  a regression.
> 
> Oh, the patch is in the TTY tree and I suppose it will go to 3.9-rc1
> (and to stable as we will get to same later -rc -- to have some testing).

 Cool. Thanks for feedback.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-20 17:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-15 22:26 [PATCH] TTY: do not reset master's packet mode Jiri Slaby
2013-02-01 17:59 ` Josh Boyer
2013-02-04 15:55   ` Jiri Slaby
2013-02-20 13:19 ` Karel Zak
2013-02-20 16:40   ` Jiri Slaby
2013-02-20 17:32     ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox