nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 09:58:01 +00:00
Code Issues Projects Releases Wiki Activity

Better handling of outstanding CHANNEL_REQUESTS on channel destruction.

Browse Source 
Part the first: make sure that all structures describing channel
requests are freed when the SSH connection is freed.  This involves
adding a means to ask a response handler to free any memory it holds.

Part the second: in ssh_channel_try_eof(), call
ssh2_channel_check_close() rather than emitting an SSH_MSG_CHANNEL_EOF
directly.  This avoids the possibility of closing the channel while a
CHANNEL_REQUEST is outstanding.

Also add some assertions that helped with tracking down the latter
problem.

[originally from svn r9623]
This commit is contained in:
Ben Harris 2012-08-25 21:06:48 +00:00
parent 4e623f5b23
commit 3d466aec90
1 changed files with 49 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
ssh.c
View File

@ -4244,6 +4244,7 @@ static void ssh_channel_try_eof(struct ssh_channel *c)
if (ssh->version == 2 && bufchain_size(&c->v.v2.outbuffer) > 0) if (ssh->version == 2 && bufchain_size(&c->v.v2.outbuffer) > 0)
return; /* can't send EOF: pending outgoing data */ return; /* can't send EOF: pending outgoing data */
c->pending_eof = FALSE; /* we're about to send it */
if (ssh->version == 1) { if (ssh->version == 1) {
send_packet(ssh, SSH1_MSG_CHANNEL_CLOSE, PKT_INT, c->remoteid, send_packet(ssh, SSH1_MSG_CHANNEL_CLOSE, PKT_INT, c->remoteid,
PKT_END); PKT_END);
@ -4254,17 +4255,8 @@ static void ssh_channel_try_eof(struct ssh_channel *c)
ssh2_pkt_adduint32(pktout, c->remoteid); ssh2_pkt_adduint32(pktout, c->remoteid);
ssh2_pkt_send(ssh, pktout); ssh2_pkt_send(ssh, pktout);
c->closes |= CLOSES_SENT_EOF; c->closes |= CLOSES_SENT_EOF;
if (!((CLOSES_SENT_EOF | CLOSES_RCVD_EOF) & ~c->closes)) { ssh2_channel_check_close(c);
/*
* Also send MSG_CLOSE.
*/
pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_CLOSE);
ssh2_pkt_adduint32(pktout, c->remoteid);
ssh2_pkt_send(ssh, pktout);
c->closes |= CLOSES_SENT_CLOSE;
}
} }
c->pending_eof = FALSE; /* we've sent it now */
} }
void sshfwd_write_eof(struct ssh_channel *c) void sshfwd_write_eof(struct ssh_channel *c)
@ -6599,6 +6591,7 @@ static void ssh2_queue_chanreq_handler(struct ssh_channel *c,
struct outstanding_channel_request *ocr = struct outstanding_channel_request *ocr =
snew(struct outstanding_channel_request); snew(struct outstanding_channel_request);
assert(!(c->closes & (CLOSES_SENT_CLOSE | CLOSES_RCVD_CLOSE)));
ocr->handler = handler; ocr->handler = handler;
ocr->ctx = ctx; ocr->ctx = ctx;
ocr->next = NULL; ocr->next = NULL;
@ -6615,12 +6608,18 @@ static void ssh2_queue_chanreq_handler(struct ssh_channel *c,
* when it arrives. The returned packet is ready to have any * when it arrives. The returned packet is ready to have any
* request-specific data added and be sent. Note that if a handler is * request-specific data added and be sent. Note that if a handler is
* provided, it's essential that the request actually be sent. * provided, it's essential that the request actually be sent.
*
* The handler will usually be passed the response packet in pktin.
* If pktin is NULL, this means that no reply will ever be forthcoming
* (e.g. because the entire connection is being destroyed) and the
* handler should free any storage it's holding.
*/ */
static struct Packet *ssh2_chanreq_init(struct ssh_channel *c, char *type, static struct Packet *ssh2_chanreq_init(struct ssh_channel *c, char *type,
cchandler_fn_t handler, void *ctx) cchandler_fn_t handler, void *ctx)
{ {
struct Packet *pktout; struct Packet *pktout;
assert(!(c->closes & (CLOSES_SENT_CLOSE | CLOSES_RCVD_CLOSE)));
pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_REQUEST); pktout = ssh2_pkt_init(SSH2_MSG_CHANNEL_REQUEST);
ssh2_pkt_adduint32(pktout, c->remoteid); ssh2_pkt_adduint32(pktout, c->remoteid);
ssh2_pkt_addstring(pktout, type); ssh2_pkt_addstring(pktout, type);
@ -6911,8 +6910,10 @@ static void ssh_channel_destroy(struct ssh_channel *c)
} }
del234(ssh->channels, c); del234(ssh->channels, c);
if (ssh->version == 2) if (ssh->version == 2) {
bufchain_clear(&c->v.v2.outbuffer); bufchain_clear(&c->v.v2.outbuffer);
assert(c->v.v2.chanreq_head == NULL);
}
sfree(c); sfree(c);
/* /*
@ -6958,6 +6959,7 @@ static void ssh2_channel_check_close(struct ssh_channel *c)
} }
if (!((CLOSES_SENT_CLOSE | CLOSES_RCVD_CLOSE) & ~c->closes)) { if (!((CLOSES_SENT_CLOSE | CLOSES_RCVD_CLOSE) & ~c->closes)) {
assert(c->v.v2.chanreq_head == NULL);
/* /*
* We have both sent and received CLOSE, which means we're * We have both sent and received CLOSE, which means we're
* completely done with the channel. * completely done with the channel.
@ -7465,7 +7467,6 @@ static void ssh2_send_ttymode(void *data, char *mode, char *val)
ssh2_pkt_adduint32(pktout, arg); ssh2_pkt_adduint32(pktout, arg);
} }
static void ssh2_msg_authconn(Ssh ssh, struct Packet *pktin);
static void ssh2_maybe_setup_x11(struct ssh_channel *c, struct Packet *pktin, static void ssh2_maybe_setup_x11(struct ssh_channel *c, struct Packet *pktin,
void *ctx) void *ctx)
{ {
@ -7504,11 +7505,13 @@ static void ssh2_maybe_setup_x11(struct ssh_channel *c, struct Packet *pktin,
crWaitUntilV(pktin); crWaitUntilV(pktin);
if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) { if (pktin) {
logevent("X11 forwarding enabled"); if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) {
ssh->X11_fwd_enabled = TRUE; logevent("X11 forwarding enabled");
} else ssh->X11_fwd_enabled = TRUE;
logevent("X11 forwarding refused"); } else
logevent("X11 forwarding refused");
}
} }
sfree(s); sfree(s);
crFinishV; crFinishV;
@ -7534,11 +7537,13 @@ static void ssh2_maybe_setup_agent(struct ssh_channel *c, struct Packet *pktin,
crWaitUntilV(pktin); crWaitUntilV(pktin);
if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) { if (pktin) {
logevent("Agent forwarding enabled"); if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) {
ssh->agentfwd_enabled = TRUE; logevent("Agent forwarding enabled");
} else ssh->agentfwd_enabled = TRUE;
logevent("Agent forwarding refused"); } else
logevent("Agent forwarding refused");
}
} }
sfree(s); sfree(s);
crFinishV; crFinishV;
@ -7581,13 +7586,15 @@ static void ssh2_maybe_setup_pty(struct ssh_channel *c, struct Packet *pktin,
crWaitUntilV(pktin); crWaitUntilV(pktin);
if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) { if (pktin) {
logeventf(ssh, "Allocated pty (ospeed %dbps, ispeed %dbps)", if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) {
ssh->ospeed, ssh->ispeed); logeventf(ssh, "Allocated pty (ospeed %dbps, ispeed %dbps)",
ssh->got_pty = TRUE; ssh->ospeed, ssh->ispeed);
} else { ssh->got_pty = TRUE;
c_write_str(ssh, "Server refused to allocate pty\r\n"); } else {
ssh->editing = ssh->echoing = 1; c_write_str(ssh, "Server refused to allocate pty\r\n");
ssh->editing = ssh->echoing = 1;
}
} }
} else { } else {
ssh->editing = ssh->echoing = 1; ssh->editing = ssh->echoing = 1;
@ -7639,6 +7646,7 @@ static void ssh2_setup_env(struct ssh_channel *c, struct Packet *pktin,
while (s->env_left > 0) { while (s->env_left > 0) {
crWaitUntilV(pktin); crWaitUntilV(pktin);
if (!pktin) goto out;
if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS) if (pktin->type == SSH2_MSG_CHANNEL_SUCCESS)
s->env_ok++; s->env_ok++;
s->env_left--; s->env_left--;
@ -7655,6 +7663,7 @@ static void ssh2_setup_env(struct ssh_channel *c, struct Packet *pktin,
c_write_str(ssh, "Server refused to set all environment variables\r\n"); c_write_str(ssh, "Server refused to set all environment variables\r\n");
} }
} }
out:
sfree(s); sfree(s);
crFinishV; crFinishV;
} }
@ -9725,6 +9734,17 @@ static void ssh_free(void *handle)
pfd_close(c->u.pfd.s); pfd_close(c->u.pfd.s);
break; break;
} }
if (ssh->version == 2) {
struct outstanding_channel_request *ocr, *nocr;
ocr = c->v.v2.chanreq_head;
while (ocr) {
ocr->handler(c, NULL, ocr->ctx);
nocr = ocr->next;
sfree(ocr);
ocr = nocr;
}
bufchain_clear(&c->v.v2.outbuffer);
}
sfree(c); sfree(c);
} }
freetree234(ssh->channels); freetree234(ssh->channels);