All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: Heorhi Valakhanovich <valahanovich@tut.by>
Cc: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	gregkh@linuxfoundation.org
Subject: Re: [PATCH] tty: Only hangup once
Date: Tue, 19 Nov 2013 08:46:27 -0500	[thread overview]
Message-ID: <528B6BB3.7020303@hurleysoftware.com> (raw)
In-Reply-To: <20131119000923.5ca40e51@tormoz-pc>

On 11/18/2013 04:09 PM, Heorhi Valakhanovich wrote:
> On Mon, 18 Nov 2013 15:32:59 -0500
> Peter Hurley <peter@hurleysoftware.com> wrote:
>
>> On 11/18/2013 12:37 PM, Peter Hurley wrote:
>>> On 11/18/2013 08:42 AM, One Thousand Gnomes wrote:
>>>>> After upgrading to kernel 3.12 I noticed one issue with tmux
>>>>> software. The easiest way to reproduce will be:
>>>>> 1. Start tmux session as root.
>>>>> 2. Connect via ssh and use "tmux attach" to attach to the running
>>>>> session.
>>>>> 3. Kill ssh client.
>>>
>>> Heorhi,
>>>
>>> Thanks for the report.
>>>
>>>>> You can notice that shell (zsh in my case) and "tmux attach" are
>>>>> still remains in process' list. That didn't happen in previous
>>>>> kernels.
>>
>> Heorhi,
>>
>> I could not reproduce this behavior with zsh or bash.
>>
>> In case I misunderstood, the steps I took to reproduce were,
>>
>> [On ssh server machine running 3.12]
>> 1.  Add 'set-option -g default-shell /usr/bin/zsh' to /etc/tmux.conf
>
> This is really not necessary. I also reproduced it with bash.
>
>> 2a. In terminal window, 'sudo tmux'
>>         - also tried -
>> 2b. At root login on tty2, 'tmux'
>>
>> Successfully starts tmux session. Then,
>>
>> [On ssh client machine]
>> 1. 'ssh user@server'
>> 2. 'sudo tmux attach'
>> 3. Switch to 2nd terminal window,
>> 4. ps -ef | grep ssh
>> 5. kill $pid_from_step4
>>
>> This *did not* leave the 'tmux attach' process alive.
>>
>> Is zsh your login shell as well? Maybe that's the required
>> component to reproduce the behavior you've observed.
>>
>> Regards,
>> Peter Hurley
>
> Thank you for your attention.
> I tried to reproduce it exactly as you did and notice strange thing.
> You use "sudo tmux attach" and tmux client uses the terminal created
> for regular user and those works good.
>
> But if you will use "ssh root@server" (you can use localhost too) the
> "tmux attach" will use terminal created from root (i checked this with
> lsof and ls -al /dev/pts) and behavior is different. Are terminals
> created for root is somehow different from those created for regular
> users?
>
> If you can't use root ssh login I will try to reproduce it's for sudo by
> mixing some kind of dtach and tmux but it will be very unclear at the
> end.

Thanks for the additional testing; yes, the "ssh root@server" is the
crucial difference. I was able to reproduce the problem immediately.

Before sshd execs a new root shell, it hangs up and then re-opens
the current tty as a security measure. Because the tty state was not
being reset, the subsequent re-open could not be hung up.

Would you please test the patch below and confirm the fix?

--->%---
Subject: [PATCH] tty: Reset hupped state on open

A common security idiom is to hangup the current tty (via vhangup())
after forking but before execing a root shell. This hangs up any
existing opens which other processes may have and ensures subsequent
opens have the necessary permissions to open the root shell tty/pty.

Reset the TTY_HUPPED state after the driver has successfully
returned the opened tty (perform the reset while the tty is locked
to avoid racing with concurrent hangups).

Reported-by: Heorhi Valakhanovich <valahanovich@tut.by>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
  drivers/tty/tty_io.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 3a1a01a..c74a00a 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2086,6 +2086,7 @@ retry_open:
  			filp->f_op = &tty_fops;
  		goto retry_open;
  	}
+	clear_bit(TTY_HUPPED, &tty->flags);
  	tty_unlock(tty);


-- 
1.8.1.2



  reply	other threads:[~2013-11-19 13:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-31 18:05 [PATCH] tty: Only hangup once Peter Hurley
2013-08-02  3:46 ` Greg Kroah-Hartman
2013-08-02 23:02   ` Peter Hurley
2013-11-17 17:38 ` Heorhi Valakhanovich
2013-11-17 17:38   ` Heorhi Valakhanovich
2013-11-18 13:42   ` One Thousand Gnomes
2013-11-18 17:37     ` Peter Hurley
2013-11-18 20:32       ` Peter Hurley
2013-11-18 21:09         ` Heorhi Valakhanovich
2013-11-19 13:46           ` Peter Hurley [this message]
2013-11-19 17:19             ` Heorhi Valakhanovich
2013-11-19 17:40               ` Greg KH
2013-11-19 21:34                 ` Peter Hurley
2013-11-19 23:05                   ` Greg KH
2013-11-18 23:03       ` One Thousand Gnomes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=528B6BB3.7020303@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=valahanovich@tut.by \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.