From a859955689801f8173621677298280507061b13c Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 21 Feb 2021 10:11:13 +0000 Subject: [PATCH] Fix premature exit if 'plink -shareexists' happens early. A user reported a phenomenon where running 'plink -shareexists' very early in the connection would cause the receiving upstream PuTTY to exit cleanly with the message 'All channels closed' in the log. That wasn't hard to track down: that happens as a result of the connection layer callback sharing_no_more_downstreams(), which causes the connection layer to check whether it has any channels left open, and if not, to terminate the connection on the grounds that everything has finished. But it's premature to draw that conclusion if the reason no channels are open if we haven't _started_ yet! Now we have a 'started' flag which is set when we initialise mainchan, and the 'we're all done now' check will never fire before that flag is set. But in the course of investigating that, I found a second problem in the same area: at an even earlier stage of an SSH connection, the connshare system doesn't _even_ have the real ConnectionLayer pointer yet. Instead, it has a pointer to a dummy one provided by the top-level ssh.c, which contains a NULL vtable pointer. So if it calls sharing_no_more_downstreams on _that_ ConnectionLayer, it will dereference NULL and crash. So I've filled in cl_dummy's vtable pointer with a trivial vtable, containing only the one callback sharing_no_more_downstreams, which itself is a no-op function. Hopefully that should all be stable now. --- ssh.c | 8 ++++++++ ssh2connection.c | 10 ++++++++++ ssh2connection.h | 1 + 3 files changed, 19 insertions(+) diff --git a/ssh.c b/ssh.c index 421315e4..d74a0ff2 100644 --- a/ssh.c +++ b/ssh.c @@ -865,6 +865,13 @@ bool ssh_is_bare(Ssh *ssh) return ssh->backend.vt->protocol == PROT_SSHCONN; } +/* Dummy connlayer must provide ssh_sharing_no_more_downstreams, + * because it might be called early due to plink -shareexists */ +static void dummy_sharing_no_more_downstreams(ConnectionLayer *cl) {} +static const ConnectionLayerVtable dummy_connlayer_vtable = { + .sharing_no_more_downstreams = dummy_sharing_no_more_downstreams, +}; + /* * Called to set up the connection. * @@ -900,6 +907,7 @@ static char *ssh_init(const BackendVtable *vt, Seat *seat, ssh->bare_connection = (vt->protocol == PROT_SSHCONN); ssh->seat = seat; + ssh->cl_dummy.vt = &dummy_connlayer_vtable; ssh->cl_dummy.logctx = ssh->logctx = logctx; random_ref(); /* do this now - may be needed by sharing setup code */ diff --git a/ssh2connection.c b/ssh2connection.c index 1c9454cb..ecb421ea 100644 --- a/ssh2connection.c +++ b/ssh2connection.c @@ -1020,6 +1020,7 @@ static void ssh2_connection_process_queue(PacketProtocolLayer *ppl) s->mainchan = mainchan_new( &s->ppl, &s->cl, s->conf, s->term_width, s->term_height, s->ssh_is_simple, &s->mainchan_sc); + s->started = true; /* * Transfer data! @@ -1248,6 +1249,15 @@ static void ssh2_check_termination(struct ssh2_connection_state *s) if (s->persistent) return; /* persistent mode: never proactively terminate */ + if (!s->started) { + /* At startup, we don't have any channels open because we + * haven't got round to opening the main one yet. In that + * situation, we don't want to terminate, even if a sharing + * connection opens and closes and causes a call to this + * function. */ + return; + } + if (count234(s->channels) == 0 && !(s->connshare && share_ndownstreams(s->connshare) > 0)) { /* diff --git a/ssh2connection.h b/ssh2connection.h index f0afb676..d3bb240a 100644 --- a/ssh2connection.h +++ b/ssh2connection.h @@ -19,6 +19,7 @@ struct ssh2_connection_state { bool ssh_is_simple; bool persistent; + bool started; Conf *conf;