mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-01-10 01:48:00 +00:00
mainchan.c: rewrite handling of open-failure aborts.
This is another case where a stale pointer bug could have arisen from a toplevel callback going off after an object was freed. But here, just adding delete_callbacks_for_context wouldn't help. The actual context parameter for the callback wasn't mainchan itself; it was a tiny separate object, allocated to hold just the parameters of the function the callback wanted to call. So if _those_ parameters became stale before the callback was triggered, then even delete_callbacks_for_context wouldn't have been able to help. Also, mainchan itself would have been freed moments after this callback was queued, so moving it to be a callback on mainchan itself wouldn't help. Solution: move the callback right out to Ssh, by introducing a new ssh_sw_abort_deferred() which is just like ssh_sw_abort but does its main work in a toplevel callback. Then ssh.c's existing call to delete_callbacks_for_context will clean it up if necessary.
This commit is contained in:
parent
108baae60e
commit
f99aeb3129
24
mainchan.c
24
mainchan.c
@ -334,33 +334,13 @@ static void mainchan_ready(mainchan *mc)
|
|||||||
queue_idempotent_callback(&mc->ppl->ic_process_queue);
|
queue_idempotent_callback(&mc->ppl->ic_process_queue);
|
||||||
}
|
}
|
||||||
|
|
||||||
struct mainchan_open_failure_abort_ctx {
|
|
||||||
Ssh *ssh;
|
|
||||||
char *abort_message;
|
|
||||||
};
|
|
||||||
|
|
||||||
static void mainchan_open_failure_abort(void *vctx)
|
|
||||||
{
|
|
||||||
struct mainchan_open_failure_abort_ctx *ctx =
|
|
||||||
(struct mainchan_open_failure_abort_ctx *)vctx;
|
|
||||||
ssh_sw_abort(
|
|
||||||
ctx->ssh, "Server refused to open main channel: %s",
|
|
||||||
ctx->abort_message);
|
|
||||||
sfree(ctx->abort_message);
|
|
||||||
sfree(ctx);
|
|
||||||
}
|
|
||||||
|
|
||||||
static void mainchan_open_failure(Channel *chan, const char *errtext)
|
static void mainchan_open_failure(Channel *chan, const char *errtext)
|
||||||
{
|
{
|
||||||
assert(chan->vt == &mainchan_channelvt);
|
assert(chan->vt == &mainchan_channelvt);
|
||||||
mainchan *mc = container_of(chan, mainchan, chan);
|
mainchan *mc = container_of(chan, mainchan, chan);
|
||||||
|
|
||||||
struct mainchan_open_failure_abort_ctx *ctx =
|
ssh_sw_abort_deferred(mc->ppl->ssh,
|
||||||
snew(struct mainchan_open_failure_abort_ctx);
|
"Server refused to open main channel: %s", errtext);
|
||||||
|
|
||||||
ctx->ssh = mc->ppl->ssh;
|
|
||||||
ctx->abort_message = dupstr(errtext);
|
|
||||||
queue_toplevel_callback(mainchan_open_failure_abort, ctx);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static size_t mainchan_send(Channel *chan, bool is_stderr,
|
static size_t mainchan_send(Channel *chan, bool is_stderr,
|
||||||
|
22
ssh.c
22
ssh.c
@ -133,6 +133,8 @@ struct Ssh {
|
|||||||
|
|
||||||
Pinger *pinger;
|
Pinger *pinger;
|
||||||
|
|
||||||
|
char *deferred_abort_message;
|
||||||
|
|
||||||
bool need_random_unref;
|
bool need_random_unref;
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -555,6 +557,24 @@ void ssh_user_close(Ssh *ssh, const char *fmt, ...)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void ssh_deferred_abort_callback(void *vctx)
|
||||||
|
{
|
||||||
|
Ssh *ssh = (Ssh *)vctx;
|
||||||
|
char *msg = ssh->deferred_abort_message;
|
||||||
|
ssh->deferred_abort_message = NULL;
|
||||||
|
ssh_sw_abort(ssh, msg);
|
||||||
|
sfree(msg);
|
||||||
|
}
|
||||||
|
|
||||||
|
void ssh_sw_abort_deferred(Ssh *ssh, const char *fmt, ...)
|
||||||
|
{
|
||||||
|
if (!ssh->deferred_abort_message) {
|
||||||
|
GET_FORMATTED_MSG;
|
||||||
|
ssh->deferred_abort_message = msg;
|
||||||
|
queue_toplevel_callback(ssh_deferred_abort_callback, ssh);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
static void ssh_socket_log(Plug *plug, int type, SockAddr *addr, int port,
|
static void ssh_socket_log(Plug *plug, int type, SockAddr *addr, int port,
|
||||||
const char *error_msg, int error_code)
|
const char *error_msg, int error_code)
|
||||||
{
|
{
|
||||||
@ -915,6 +935,8 @@ static void ssh_free(Backend *be)
|
|||||||
ssh_gss_cleanup(ssh->gss_state.libs);
|
ssh_gss_cleanup(ssh->gss_state.libs);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
sfree(ssh->deferred_abort_message);
|
||||||
|
|
||||||
delete_callbacks_for_context(ssh); /* likely to catch ic_out_raw */
|
delete_callbacks_for_context(ssh); /* likely to catch ic_out_raw */
|
||||||
|
|
||||||
need_random_unref = ssh->need_random_unref;
|
need_random_unref = ssh->need_random_unref;
|
||||||
|
1
ssh.h
1
ssh.h
@ -407,6 +407,7 @@ void ssh_remote_error(Ssh *ssh, const char *fmt, ...);
|
|||||||
void ssh_remote_eof(Ssh *ssh, const char *fmt, ...);
|
void ssh_remote_eof(Ssh *ssh, const char *fmt, ...);
|
||||||
void ssh_proto_error(Ssh *ssh, const char *fmt, ...);
|
void ssh_proto_error(Ssh *ssh, const char *fmt, ...);
|
||||||
void ssh_sw_abort(Ssh *ssh, const char *fmt, ...);
|
void ssh_sw_abort(Ssh *ssh, const char *fmt, ...);
|
||||||
|
void ssh_sw_abort_deferred(Ssh *ssh, const char *fmt, ...);
|
||||||
void ssh_user_close(Ssh *ssh, const char *fmt, ...);
|
void ssh_user_close(Ssh *ssh, const char *fmt, ...);
|
||||||
|
|
||||||
/* Bit positions in the SSH-1 cipher protocol word */
|
/* Bit positions in the SSH-1 cipher protocol word */
|
||||||
|
Loading…
Reference in New Issue
Block a user