Bug 1698 - Connection stalls on PTY allocation failure
Summary: Connection stalls on PTY allocation failure
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 5.3p1
Hardware: Other All
: P2 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_5_6
  Show dependency treegraph
 
Reported: 2010-01-15 01:43 AEDT by Alex Beregszaszi
Modified: 2011-01-24 12:33 AEDT (History)
2 users (show)

See Also:


Attachments
kill-on-pty-fail.diff (913 bytes, patch)
2010-04-09 10:35 AEST, Damien Miller
no flags Details | Diff
kill-on-pty-fail2.diff (651 bytes, patch)
2010-04-09 10:36 AEST, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Beregszaszi 2010-01-15 01:43:53 AEDT
If on the server side sshd was not able to allocate a PTY (in my case a wrongly configured FreeBSD's ugidfw rule wont allow opening those files) both sshd and the client ssh will stall.

Client ssh log:

debug1: Requesting no-more-sessions@openssh.com
debug1: Entering interactive session.
debug2: callback start
debug2: client_session2_setup: id 0
debug2: channel 0: request pty-req confirm 1
debug2: channel 0: request shell confirm 1
debug2: fd 3 setting TCP_NODELAY
debug2: callback done
debug2: channel 0: open confirm rwindow 0 rmax 32768
debug2: channel_input_confirm: type 100 id 0
PTY allocation request failed on channel 0
debug2: channel 0: rcvd adjust 2097152
debug2: channel_input_confirm: type 99 id 0
debug2: shell request accepted on channel 0

Server side log:
Jan 14 14:40:09 www sshd[50855]: error: openpty: Permission denied
Jan 14 14:40:09 www sshd[50855]: error: session_pty_req: session 0 alloc failed

--

I traced down this to session.c:session_pty_req:

        if (no_pty_flag) {
                debug("Allocating a pty not permitted for this authentication.");
                return 0;
        }
        if (s->ttyfd != -1) {
                packet_disconnect("Protocol error: you already have a pty.");
                return 0;
        }

...

        debug("Allocating pty.");
        if (!PRIVSEP(pty_allocate(&s->ptyfd, &s->ttyfd, s->tty,
            sizeof(s->tty)))) {
                if (s->term)
                        xfree(s->term);
                s->term = NULL;
                s->ptyfd = -1;
                s->ttyfd = -1;
                error("session_pty_req: session %d alloc failed", s->self);
                return 0;
        }

--

I am not that deeply into openssh but to me it looks like a packet_disconnect() might be missing after the error().
Comment 1 Darren Tucker 2010-01-15 09:57:28 AEDT
I think it's a bug in the client failing to handle the pty allocation failure.  I looked at this once before but at the time the code made if very difficult to associate the failure message with the pty request.  We've improved it since then so it might be time to take another look.

You can't just disconnect.  A single sshd can have multiple sessions and thus multiple ptys, so the failure to allocate one should not cause the connection to terminate.
Comment 2 Alex Beregszaszi 2010-01-15 12:36:04 AEDT
If you say this must be handled on the client part, then maybe in clientloop.c:client_session2_setup() the do_close parameter might be changed to 1 in client_expect_confirm(id, "PTY allocation", 0); inside the want_tty if branch. Looks like that will terminate only that channel.

Also notice the comment there: XXX wait for reply
Comment 3 Damien Miller 2010-01-15 15:22:33 AEDT
yes, the reply callback infrastructure is quite new and not everything uses it yet
Comment 4 Damien Miller 2010-04-09 10:35:25 AEST
Created attachment 1830 [details]
kill-on-pty-fail.diff

Kill channel on PTY allocation failure
Comment 5 Damien Miller 2010-04-09 10:36:18 AEST
Created attachment 1831 [details]
kill-on-pty-fail2.diff

Kill channel on PTY allocation failure
Comment 6 Damien Miller 2010-04-10 15:54:37 AEST
patch applied - will be in OpenSSH 5.6p1
Comment 7 Damien Miller 2011-01-24 12:33:50 AEDT
Move resolved bugs to CLOSED after 5.7 release