From 6246ff3f0a08c3d551f59c07813fee4640a67926 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 27 Jun 2021 13:52:48 +0100 Subject: [PATCH] New Seat callback, seat_sent(). This is used to notify the Seat that some data has been cleared from the backend's outgoing data buffer. In other words, it notifies the Seat that it might be worth calling backend_sendbuffer() again. We've never needed this before, because until now, Seats have always been the 'main program' part of the application, meaning they were also in control of the event loop. So they've been able to call backend_sendbuffer() proactively, every time they go round the event loop, instead of having to wait for a callback. But now, the SSH proxy is the first example of a Seat without privileged access to the event loop, so it has no way to find out that the backend's sendbuffer has got smaller. And without that, it can't pass that notification on to plug_sent, to unblock in turn whatever the proxied connection might have been waiting to send. In fact, before this commit, sshproxy.c never called plug_sent at all. As a result, large data uploads over an SSH jump host would hang forever as soon as the outgoing buffer filled up for the first time: the main backend (to which sshproxy.c was acting as a Socket) would carefully stop filling up the buffer, and then never receive the call to plug_sent that would cause it to start again. The new callback is ignored everywhere except in sshproxy.c. It might be a good idea to remove backend_sendbuffer() entirely and convert all previous uses of it into non-empty implementations of this callback, so that we've only got one system; but for the moment, I haven't done that. --- otherbackends/raw.c | 1 + otherbackends/rlogin.c | 1 + otherbackends/supdup.c | 1 + otherbackends/telnet.c | 1 + pscp.c | 1 + psftp.c | 1 + putty.h | 14 ++++++++++++++ ssh.h | 1 + ssh/bpp-bare.c | 2 ++ ssh/bpp1.c | 2 ++ ssh/bpp2.c | 2 ++ ssh/connection2.c | 1 + ssh/server.c | 5 +++++ ssh/sesschan.c | 1 + ssh/ssh.c | 6 ++++++ ssh/transport2.c | 2 ++ sshproxy.c | 7 +++++++ unix/plink.c | 1 + unix/serial.c | 1 + unix/window.c | 1 + utils/nullseat.c | 1 + windows/plink.c | 1 + windows/serial.c | 1 + windows/window.c | 1 + 24 files changed, 56 insertions(+) diff --git a/otherbackends/raw.c b/otherbackends/raw.c index 7977f386..9c803594 100644 --- a/otherbackends/raw.c +++ b/otherbackends/raw.c @@ -104,6 +104,7 @@ static void raw_sent(Plug *plug, size_t bufsize) { Raw *raw = container_of(plug, Raw, plug); raw->bufsize = bufsize; + seat_sent(raw->seat, raw->bufsize); } static const PlugVtable Raw_plugvt = { diff --git a/otherbackends/rlogin.c b/otherbackends/rlogin.c index 8289a508..30ad0526 100644 --- a/otherbackends/rlogin.c +++ b/otherbackends/rlogin.c @@ -116,6 +116,7 @@ static void rlogin_sent(Plug *plug, size_t bufsize) { Rlogin *rlogin = container_of(plug, Rlogin, plug); rlogin->bufsize = bufsize; + seat_sent(rlogin->seat, rlogin->bufsize); } static void rlogin_startup(Rlogin *rlogin, const char *ruser) diff --git a/otherbackends/supdup.c b/otherbackends/supdup.c index 36dbb447..e7eff6ee 100644 --- a/otherbackends/supdup.c +++ b/otherbackends/supdup.c @@ -600,6 +600,7 @@ static void supdup_sent(Plug *plug, size_t bufsize) { Supdup *supdup = container_of(plug, Supdup, plug); supdup->bufsize = bufsize; + seat_sent(supdup->seat, supdup->bufsize); } static void supdup_send_36bits(Supdup *supdup, unsigned long long thirtysix) diff --git a/otherbackends/telnet.c b/otherbackends/telnet.c index f8b41ac3..4b784036 100644 --- a/otherbackends/telnet.c +++ b/otherbackends/telnet.c @@ -662,6 +662,7 @@ static void telnet_sent(Plug *plug, size_t bufsize) { Telnet *telnet = container_of(plug, Telnet, plug); telnet->bufsize = bufsize; + seat_sent(telnet->seat, telnet->bufsize); } static const PlugVtable Telnet_plugvt = { diff --git a/pscp.c b/pscp.c index 82085962..f8a6965c 100644 --- a/pscp.c +++ b/pscp.c @@ -65,6 +65,7 @@ static bool pscp_eof(Seat *); static const SeatVtable pscp_seat_vt = { .output = pscp_output, .eof = pscp_eof, + .sent = nullseat_sent, .get_userpass_input = filexfer_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/psftp.c b/psftp.c index 8eaa6fc5..1fff45ef 100644 --- a/psftp.c +++ b/psftp.c @@ -47,6 +47,7 @@ static bool psftp_eof(Seat *); static const SeatVtable psftp_seat_vt = { .output = psftp_output, .eof = psftp_eof, + .sent = nullseat_sent, .get_userpass_input = filexfer_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/putty.h b/putty.h index d67b4e75..254e370b 100644 --- a/putty.h +++ b/putty.h @@ -886,6 +886,17 @@ struct SeatVtable { */ bool (*eof)(Seat *seat); + /* + * Called by the back end to notify that the output backlog has + * changed size. A front end in control of the event loop won't + * necessarily need this (they can just keep checking it via + * backend_sendbuffer at every opportunity), but one buried in the + * depths of something else (like an SSH proxy) will need to be + * proactively notified that the amount of buffered data has + * become smaller. + */ + void (*sent)(Seat *seat, size_t new_sendbuffer); + /* * Try to get answers from a set of interactive login prompts. The * prompts are provided in 'p'; the bufchain 'input' holds the @@ -1117,6 +1128,8 @@ static inline size_t seat_output( { return seat->vt->output(seat, err, data, len); } static inline bool seat_eof(Seat *seat) { return seat->vt->eof(seat); } +static inline void seat_sent(Seat *seat, size_t bufsize) +{ seat->vt->sent(seat, bufsize); } static inline int seat_get_userpass_input( Seat *seat, prompts_t *p, bufchain *input) { return seat->vt->get_userpass_input(seat, p, input); } @@ -1190,6 +1203,7 @@ static inline size_t seat_stderr_pl(Seat *seat, ptrlen data) size_t nullseat_output( Seat *seat, bool is_stderr, const void *data, size_t len); bool nullseat_eof(Seat *seat); +void nullseat_sent(Seat *seat, size_t bufsize); int nullseat_get_userpass_input(Seat *seat, prompts_t *p, bufchain *input); void nullseat_notify_remote_exit(Seat *seat); void nullseat_notify_remote_disconnect(Seat *seat); diff --git a/ssh.h b/ssh.h index 61fc6d14..23bed390 100644 --- a/ssh.h +++ b/ssh.h @@ -402,6 +402,7 @@ bool ssh_is_bare(Ssh *ssh); /* Communications back to ssh.c from the BPP */ void ssh_conn_processed_data(Ssh *ssh); +void ssh_sendbuffer_changed(Ssh *ssh); void ssh_check_frozen(Ssh *ssh); /* Functions to abort the connection, for various reasons. */ diff --git a/ssh/bpp-bare.c b/ssh/bpp-bare.c index 3546d160..f1a889aa 100644 --- a/ssh/bpp-bare.c +++ b/ssh/bpp-bare.c @@ -201,4 +201,6 @@ static void ssh2_bare_bpp_handle_output(BinaryPacketProtocol *bpp) ssh2_bare_bpp_format_packet(s, pkt); ssh_free_pktout(pkt); } + + ssh_sendbuffer_changed(bpp->ssh); } diff --git a/ssh/bpp1.c b/ssh/bpp1.c index f84d787b..b82932f7 100644 --- a/ssh/bpp1.c +++ b/ssh/bpp1.c @@ -376,6 +376,8 @@ static void ssh1_bpp_handle_output(BinaryPacketProtocol *bpp) break; } } + + ssh_sendbuffer_changed(bpp->ssh); } static void ssh1_bpp_queue_disconnect(BinaryPacketProtocol *bpp, diff --git a/ssh/bpp2.c b/ssh/bpp2.c index f29b962f..dc98e27c 100644 --- a/ssh/bpp2.c +++ b/ssh/bpp2.c @@ -979,4 +979,6 @@ static void ssh2_bpp_handle_output(BinaryPacketProtocol *bpp) ssh2_bpp_enable_pending_compression(s); } } + + ssh_sendbuffer_changed(bpp->ssh); } diff --git a/ssh/connection2.c b/ssh/connection2.c index 48436848..2e7102db 100644 --- a/ssh/connection2.c +++ b/ssh/connection2.c @@ -1157,6 +1157,7 @@ static size_t ssh2_try_send(struct ssh2_channel *c) if (!bufsize && c->pending_eof) ssh2_channel_try_eof(c); + ssh_sendbuffer_changed(s->ppl.ssh); return bufsize; } diff --git a/ssh/server.c b/ssh/server.c index 642d4ce0..6abbf332 100644 --- a/ssh/server.c +++ b/ssh/server.c @@ -107,6 +107,7 @@ static int server_confirm_weak_cached_hostkey( static const SeatVtable server_seat_vt = { .output = nullseat_output, .eof = nullseat_eof, + .sent = nullseat_sent, .get_userpass_input = nullseat_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, @@ -187,6 +188,10 @@ LogContext *ssh_get_logctx(Ssh *ssh) return srv->logctx; } +void ssh_sendbuffer_changed(Ssh *ssh) +{ +} + void ssh_throttle_conn(Ssh *ssh, int adjust) { server *srv = container_of(ssh, server, ssh); diff --git a/ssh/sesschan.c b/ssh/sesschan.c index f1bed6f2..f16faad2 100644 --- a/ssh/sesschan.c +++ b/ssh/sesschan.c @@ -186,6 +186,7 @@ static bool sesschan_get_window_pixel_size(Seat *seat, int *w, int *h); static const SeatVtable sesschan_seat_vt = { .output = sesschan_seat_output, .eof = sesschan_seat_eof, + .sent = nullseat_sent, .get_userpass_input = nullseat_get_userpass_input, .notify_remote_exit = sesschan_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/ssh/ssh.c b/ssh/ssh.c index e8724777..096c8b01 100644 --- a/ssh/ssh.c +++ b/ssh/ssh.c @@ -640,6 +640,7 @@ static void ssh_sent(Plug *plug, size_t bufsize) if (bufsize < SSH_MAX_BACKLOG) { ssh_throttle_all(ssh, false, bufsize); queue_idempotent_callback(&ssh->ic_out_raw); + ssh_sendbuffer_changed(ssh); } } @@ -1042,6 +1043,11 @@ static size_t ssh_sendbuffer(Backend *be) return backlog; } +void ssh_sendbuffer_changed(Ssh *ssh) +{ + seat_sent(ssh->seat, ssh_sendbuffer(&ssh->backend)); +} + /* * Called to set the size of the window from SSH's POV. */ diff --git a/ssh/transport2.c b/ssh/transport2.c index fcd2667a..2966f2d7 100644 --- a/ssh/transport2.c +++ b/ssh/transport2.c @@ -1484,6 +1484,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) * layer's outgoing queue on to our own. */ pq_concatenate(s->ppl.out_pq, s->ppl.out_pq, &s->pq_out_higher); + ssh_sendbuffer_changed(s->ppl.ssh); /* * Expect SSH2_MSG_NEWKEYS from server. @@ -1620,6 +1621,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) /* Pass through outgoing packets from the higher layer. */ pq_concatenate(s->ppl.out_pq, s->ppl.out_pq, &s->pq_out_higher); + ssh_sendbuffer_changed(s->ppl.ssh); /* Wait for either a KEXINIT, or something setting * s->rekey_class. This call to ssh2_transport_pop also has diff --git a/sshproxy.c b/sshproxy.c index 0ffa74ff..f36a2a9b 100644 --- a/sshproxy.c +++ b/sshproxy.c @@ -258,6 +258,12 @@ static bool sshproxy_eof(Seat *seat) return false; } +static void sshproxy_sent(Seat *seat, size_t new_bufsize) +{ + SshProxy *sp = container_of(seat, SshProxy, seat); + plug_sent(sp->plug, new_bufsize); +} + static void sshproxy_notify_remote_disconnect(Seat *seat) { SshProxy *sp = container_of(seat, SshProxy, seat); @@ -415,6 +421,7 @@ static bool sshproxy_set_trust_status(Seat *seat, bool trusted) static const SeatVtable SshProxy_seat_vt = { .output = sshproxy_output, .eof = sshproxy_eof, + .sent = sshproxy_sent, .get_userpass_input = sshproxy_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = sshproxy_notify_remote_disconnect, diff --git a/unix/plink.c b/unix/plink.c index 606bfc69..7663b976 100644 --- a/unix/plink.c +++ b/unix/plink.c @@ -389,6 +389,7 @@ static bool plink_seat_interactive(Seat *seat) static const SeatVtable plink_seat_vt = { .output = plink_output, .eof = plink_eof, + .sent = nullseat_sent, .get_userpass_input = plink_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/unix/serial.c b/unix/serial.c index d4a1e0ba..d17e4cdd 100644 --- a/unix/serial.c +++ b/unix/serial.c @@ -454,6 +454,7 @@ static void serial_try_write(Serial *serial) bufchain_consume(&serial->output_data, ret); } + seat_sent(serial->seat, bufchain_size(&serial->output_data)); serial_uxsel_setup(serial); } diff --git a/unix/window.c b/unix/window.c index f787fa9b..8582777b 100644 --- a/unix/window.c +++ b/unix/window.c @@ -389,6 +389,7 @@ static bool gtk_seat_get_cursor_position(Seat *seat, int *x, int *y); static const SeatVtable gtk_seat_vt = { .output = gtk_seat_output, .eof = gtk_seat_eof, + .sent = nullseat_sent, .get_userpass_input = gtk_seat_get_userpass_input, .notify_remote_exit = gtk_seat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/utils/nullseat.c b/utils/nullseat.c index 9b69a07a..0c773af0 100644 --- a/utils/nullseat.c +++ b/utils/nullseat.c @@ -7,6 +7,7 @@ size_t nullseat_output( Seat *seat, bool is_stderr, const void *data, size_t len) { return 0; } bool nullseat_eof(Seat *seat) { return true; } +void nullseat_sent(Seat *seat, size_t bufsize) {} int nullseat_get_userpass_input( Seat *seat, prompts_t *p, bufchain *input) { return 0; } void nullseat_notify_remote_exit(Seat *seat) {} diff --git a/windows/plink.c b/windows/plink.c index 70bf1567..ac68ce7d 100644 --- a/windows/plink.c +++ b/windows/plink.c @@ -83,6 +83,7 @@ static bool plink_seat_interactive(Seat *seat) static const SeatVtable plink_seat_vt = { .output = plink_output, .eof = plink_eof, + .sent = nullseat_sent, .get_userpass_input = plink_get_userpass_input, .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, diff --git a/windows/serial.c b/windows/serial.c index 7f4bcf2e..3d5ea8e5 100644 --- a/windows/serial.c +++ b/windows/serial.c @@ -88,6 +88,7 @@ static void serial_sentdata(struct handle *h, size_t new_backlog, int err) seat_connection_fatal(serial->seat, "%s", error_msg); } else { serial->bufsize = new_backlog; + seat_sent(serial->seat, serial->bufsize); } } diff --git a/windows/window.c b/windows/window.c index 36284927..6ab8e16b 100644 --- a/windows/window.c +++ b/windows/window.c @@ -330,6 +330,7 @@ static bool win_seat_get_window_pixel_size(Seat *seat, int *x, int *y); static const SeatVtable win_seat_vt = { .output = win_seat_output, .eof = win_seat_eof, + .sent = nullseat_sent, .get_userpass_input = win_seat_get_userpass_input, .notify_remote_exit = win_seat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect,