1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Stop ssh2_msg_channel_response using a stale ssh_channel.

When it calls through ocr->handler() to process the response to a
channel request, sometimes that call ends up back in the main SSH-2
authconn coroutine, and sometimes _that_ will call bomb_out(), which
closes the whole SSH connection and frees all the channels - so that
when control returns back up the call stack to
ssh2_msg_channel_response itself which continues working with the
channel it was passed, it's using freed memory and things go badly.

This is the sort of thing I'd _like_ to fix using some kind of
large-scale refactoring along the lines of moving all the actual
free() calls out into top-level callbacks, so that _any_ function
which is holding a pointer to something can rely on that pointer still
being valid after it calls a subroutine. But I haven't worked out all
the details of how that system should work, and doubtless it will turn
out to have problems of its own once I do, so here's a point fix which
simply checks if the whole SSH session has been closed (which is easy
- much easier than checking if that _channel_ structure still exists)
and fixes the immediate bug.

(I think this is the real fix for the problem reported by the user I
mention in commit f0126dd19, because I actually got the details wrong
in the log message for that previous commit: the user's SSH server
wasn't rejecting the _opening_ of the main session channel, it was
rejecting the "shell" channel request, so this code path was the one
being exercised. Still, the other bug was real too, so no harm done!)
This commit is contained in:
Simon Tatham 2017-07-19 07:22:03 +01:00
parent f0126dd198
commit 0a93b5d9bc

2
ssh.c
View File

@ -8105,6 +8105,8 @@ static void ssh2_msg_channel_response(Ssh ssh, struct Packet *pktin)
return;
}
ocr->handler(c, pktin, ocr->ctx);
if (ssh->state == SSH_STATE_CLOSED)
return; /* in case the handler called bomb_out(), which some can */
c->v.v2.chanreq_head = ocr->next;
sfree(ocr);
/*