Thread safe replacement of handles_by_evtomain global

Before it was used for local proxy command only, so probability of problems was relatively small. In PuTTY 0.75 it's used even for Pageant communication.

(cherry picked from commit 675cfb552b32da3b15dae944cb4f319d03b96499)

Source commit: e95234c0c1ab3db66d29fa4b321596cac02539e7

source/core/SecureShell.cpp

@@ -77,10 +77,13 @@ __fastcall TSecureShell::TSecureShell(TSessionUI* UI,
   FWaitingForData = 0;
   FCallbackSet.reset(new callback_set());
   memset(FCallbackSet.get(), 0, sizeof(callback_set));
+  FCallbackSet->handles_by_evtomain = new_handles_by_evtomain();
 }
 //---------------------------------------------------------------------------
 __fastcall TSecureShell::~TSecureShell()
 {
+  freetree234(FCallbackSet->handles_by_evtomain);
+  FCallbackSet->handles_by_evtomain = NULL;
   DebugAssert(FWaiting == 0);
   Active = false;
   ResetConnection();
@@ -2003,10 +2006,8 @@ bool __fastcall TSecureShell::EventSelectLoop(unsigned int MSec, bool ReadEventR
         {
           sfree(Handles);
         }
-        // Note that this returns all handles, not only the this-session-related handles,
-        // so we can possibly be processing handles of other sessions, what may be very wrong.
         // It returns only busy handles, so the set can change with every call to run_toplevel_callbacks.
-        Handles = handle_get_events(&HandleCount);
+        Handles = handle_get_events(FCallbackSet->handles_by_evtomain, &HandleCount);
         Handles = sresize(Handles, HandleCount + 1, HANDLE);
         Handles[HandleCount] = FSocketEvent;
         WaitResult = WaitForMultipleObjects(HandleCount + 1, Handles, FALSE, TimeoutStep);
@@ -2027,7 +2028,7 @@ bool __fastcall TSecureShell::EventSelectLoop(unsigned int MSec, bool ReadEventR
 
       if (WaitResult < WAIT_OBJECT_0 + HandleCount)
       {
-        if (handle_got_event(Handles[WaitResult - WAIT_OBJECT_0]))
+        if (handle_got_event(FCallbackSet->handles_by_evtomain, Handles[WaitResult - WAIT_OBJECT_0]))
         {
           Result = true;
         }

source/putty/agentf.c

@@ -12,6 +12,7 @@
 #include "sshchan.h"
 
 typedef struct agentf {
+    Plug *plug; // WINSCP
     SshChannel *c;
     bufchain inbuffer;
     agent_pending_query *pending;
@@ -97,7 +98,7 @@ static void agentf_try_forward(agentf *af)
         bufchain_fetch_consume(
             &af->inbuffer, strbuf_append(message, length), length);
         af->pending = agent_query(
-            message, &reply, &replylen, agentf_callback, af);
+            message, &reply, &replylen, agentf_callback, af, get_callback_set(af->plug)); // WINSCP
         strbuf_free(message);
 
         if (af->pending)
@@ -173,9 +174,10 @@ static const ChannelVtable agentf_channelvt = {
     /*.request_response =*/ chan_no_request_response,
 };
 
-Channel *agentf_new(SshChannel *c)
+Channel *agentf_new(SshChannel *c, Plug *plug) // WINSCP
 {
     agentf *af = snew(agentf);
+    af->plug = plug; // WINSCP
     af->c = c;
     af->chan.vt = &agentf_channelvt;
     af->chan.initial_fixed_window_size = 0;

source/putty/putty.h

@@ -2072,7 +2072,7 @@ int mk_wcswidth_cjk(const unsigned int *pwcs, size_t n);
 typedef struct agent_pending_query agent_pending_query;
 agent_pending_query *agent_query(
     strbuf *in, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx);
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set); // WINSCP
 void agent_cancel_query(agent_pending_query *);
 void agent_query_synchronous(strbuf *in, void **out, int *outlen);
 bool agent_exists(void);
@@ -2376,6 +2376,7 @@ struct callback_set {
     struct callback *cbcurr, *cbhead, *cbtail;
     IdempotentCallback * ic_pktin_free;
     PacketQueueNode * pktin_freeq_head;
+    tree234 * handles_by_evtomain;
 };
 #define CALLBACK_SET_ONLY struct callback_set * callback_set_v
 #define CALLBACK_SET CALLBACK_SET_ONLY,
@@ -2393,6 +2394,7 @@ void delete_callbacks_for_context(CALLBACK_SET void *ctx);
 LogPolicy *log_get_logpolicy(LogContext *ctx); // WINSCP
 Seat * get_log_seat(LogContext * lp); // WINSCP
 struct callback_set * get_log_callback_set(LogContext * lp); // WINSCP
+tree234 *new_handles_by_evtomain(); // WINSCP
 
 /*
  * Another facility in callback.c deals with 'idempotent' callbacks,

source/putty/ssh.c

@@ -270,7 +270,8 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv,
                     NULL
 #endif
                     ,conf_get_str(ssh->conf, CONF_loghost),
-                    conf_get_bool(ssh->conf, CONF_change_password) // WINSCP
+                    conf_get_bool(ssh->conf, CONF_change_password), // WINSCP
+                    ssh->seat
                     );
                 ssh_connect_ppl(ssh, userauth_layer);
                 transport_child_layer = userauth_layer;

source/putty/ssh.h

@@ -1226,7 +1226,7 @@ void x11_format_auth_for_authfile(
 int x11_identify_auth_proto(ptrlen protoname);
 void *x11_dehexify(ptrlen hex, int *outlen);
 
-Channel *agentf_new(SshChannel *c);
+Channel *agentf_new(SshChannel *c, Plug *plug); // WINSCP
 
 bool dh_is_gex(const ssh_kex *kex);
 dh_ctx *dh_setup_group(const ssh_kex *kex);

source/putty/ssh2connection-client.c

@@ -114,7 +114,7 @@ static ChanopenResult chan_open_auth_agent(
          * forwarded data stream ourselves for message boundaries, and
          * passing each individual message to the one-off agent_query().
          */
-        CHANOPEN_RETURN_SUCCESS(agentf_new(sc));
+        CHANOPEN_RETURN_SUCCESS(agentf_new(sc, plug));
     }
     } // WINSCP
 }

source/putty/ssh2userauth.c

@@ -95,6 +95,8 @@ struct ssh2_userauth_state {
     bool ki_scc_initialised;
     bool ki_printed_header;
 
+    Seat *seat; // WINSCP
+
     PacketProtocolLayer ppl;
 };
 
@@ -142,10 +144,11 @@ PacketProtocolLayer *ssh2_userauth_new(
     const char *default_username, bool change_username,
     bool try_ki_auth, bool try_gssapi_auth, bool try_gssapi_kex_auth,
     bool gssapi_fwd, struct ssh_connection_shared_gss_state *shgss,
-    const char * loghost, bool change_password) // WINSCP
+    const char * loghost, bool change_password, Seat *seat) // WINSCP
 {
     struct ssh2_userauth_state *s = snew(struct ssh2_userauth_state);
     memset(s, 0, sizeof(*s));
+    s->seat = seat;
     s->ppl.vt = &ssh2_userauth_vtable;
 
     s->successor_layer = successor_layer;
@@ -1829,7 +1832,7 @@ static void ssh2_userauth_agent_query(
     s->agent_response_to_free = NULL;
 
     s->auth_agent_query = agent_query(req, &response, &response_len,
-                                      ssh2_userauth_agent_callback, s);
+                                      ssh2_userauth_agent_callback, s, get_seat_callback_set(s->seat)); // WINSCP
     if (!s->auth_agent_query)
         ssh2_userauth_agent_callback(s, response, response_len);
 }

source/putty/sshppl.h

@@ -125,7 +125,7 @@ PacketProtocolLayer *ssh2_userauth_new(
     bool try_ki_auth,
     bool try_gssapi_auth, bool try_gssapi_kex_auth,
     bool gssapi_fwd, struct ssh_connection_shared_gss_state *shgss,
-    const char * loghost, bool change_password); // WINSCP
+    const char * loghost, bool change_password, Seat *seat); // WINSCP
 PacketProtocolLayer *ssh2_connection_new(
     Ssh *ssh, ssh_sharing_state *connshare, bool is_simple,
     Conf *conf, const char *peer_verstring, ConnectionLayer **cl_out);

source/putty/windows/winhandl.c

@@ -402,7 +402,9 @@ struct handle {
     } u;
 };
 
+#ifndef WINSCP
 static tree234 *handles_by_evtomain;
+#endif
 
 static int handle_cmp_evtomain(void *av, void *bv)
 {
@@ -430,7 +432,7 @@ static int handle_find_evtomain(void *av, void *bv)
         return 0;
 }
 
-struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
+struct handle *handle_input_new(tree234 * handles_by_evtomain, HANDLE handle, handle_inputfn_t gotdata,
                                 void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
@@ -447,8 +449,10 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
     h->u.i.privdata = privdata;
     h->u.i.flags = flags;
 
+    #ifndef WINSCP
     if (!handles_by_evtomain)
         handles_by_evtomain = newtree234(handle_cmp_evtomain);
+    #endif
     add234(handles_by_evtomain, h);
 
     { // WINSCP
@@ -462,7 +466,7 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
     } // WINSCP
 }
 
-struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
+struct handle *handle_output_new(tree234 * handles_by_evtomain, HANDLE handle, handle_outputfn_t sentdata, // WINSCP
                                  void *privdata, int flags)
 {
     struct handle *h = snew(struct handle);
@@ -482,8 +486,10 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
     h->u.o.sentdata = sentdata;
     h->u.o.flags = flags;
 
+    #ifndef WINSCP
     if (!handles_by_evtomain)
         handles_by_evtomain = newtree234(handle_cmp_evtomain);
+    #endif
     add234(handles_by_evtomain, h);
 
     { // WINSCP
@@ -496,6 +502,7 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
     } // WINSCP
 }
 
+#ifndef WINSCP
 struct handle *handle_add_foreign_event(HANDLE event,
                                         void (*callback)(void *), void *ctx)
 {
@@ -519,6 +526,7 @@ struct handle *handle_add_foreign_event(HANDLE event,
 
     return h;
 }
+#endif
 
 size_t handle_write(struct handle *h, const void *data, size_t len)
 {
@@ -545,7 +553,7 @@ void handle_write_eof(struct handle *h)
     }
 }
 
-HANDLE *handle_get_events(int *nevents)
+HANDLE *handle_get_events(tree234 * handles_by_evtomain, int *nevents) // WINSCP
 {
     HANDLE *ret;
     struct handle *h;
@@ -571,7 +579,7 @@ HANDLE *handle_get_events(int *nevents)
     return ret;
 }
 
-static void handle_destroy(struct handle *h)
+static void handle_destroy(tree234 * handles_by_evtomain, struct handle *h) // WINSCP
 {
     if (h->type == HT_OUTPUT)
         bufchain_clear(&h->u.o.queued_data);
@@ -579,15 +587,9 @@ static void handle_destroy(struct handle *h)
     CloseHandle(h->u.g.ev_to_main);
     del234(handles_by_evtomain, h);
     sfree(h);
-    // WINSCP (memory leak)
-    if (count234(handles_by_evtomain) == 0)
-    {
-        freetree234(handles_by_evtomain);
-        handles_by_evtomain = NULL;
-    }
 }
 
-void handle_free(struct handle *h)
+void handle_free(tree234 * handles_by_evtomain, struct handle *h) // WINSCP
 {
     assert(h && !h->u.g.moribund);
     if (h->u.g.busy && h->type != HT_FOREIGN) {
@@ -608,7 +610,7 @@ void handle_free(struct handle *h)
          * There isn't even a subthread; we can go straight to
          * handle_destroy.
          */
-        handle_destroy(h);
+        handle_destroy(handles_by_evtomain, h); // WINSCP
     } else {
         /*
          * The subthread is alive but not busy, so we now signal it
@@ -622,7 +624,7 @@ void handle_free(struct handle *h)
     }
 }
 
-int handle_got_event(HANDLE event) // WINSCP
+int handle_got_event(tree234 * handles_by_evtomain, HANDLE event) // WINSCP
 {
     struct handle *h;
 
@@ -650,7 +652,7 @@ int handle_got_event(HANDLE event) // WINSCP
          * we haven't yet done so, or destroy the handle if not.
          */
         if (h->u.g.done) {
-            handle_destroy(h);
+            handle_destroy(handles_by_evtomain, h); // WINSCP
         } else {
             h->u.g.done = true;
             h->u.g.busy = true;
@@ -739,3 +741,9 @@ void handle_sink_init(handle_sink *sink, struct handle *h)
     sink->h = h;
     BinarySink_INIT(sink, handle_sink_write);
 }
+
+// WINSCP
+tree234 *new_handles_by_evtomain()
+{
+    return newtree234(handle_cmp_evtomain);
+}

source/putty/windows/winhsock.c

@@ -119,6 +119,7 @@ static Plug *sk_handle_plug(Socket *s, Plug *p)
 static void sk_handle_close(Socket *s)
 {
     HandleSocket *hs = container_of(s, HandleSocket, sock);
+    tree234 * handles_by_evtomain = get_callback_set(hs->plug)->handles_by_evtomain; // WINSCP
 
     if (hs->defer_close) {
         hs->deferred_close = true;
@@ -130,8 +131,8 @@ static void sk_handle_close(Socket *s)
     do_select(hs->plug, INVALID_SOCKET, 0);
     #endif
 
-    handle_free(hs->send_h);
-    handle_free(hs->recv_h);
+    handle_free(handles_by_evtomain, hs->send_h); // WINSCP
+    handle_free(handles_by_evtomain, hs->recv_h); // WINSCP
     CloseHandle(hs->send_H);
     if (hs->recv_H != hs->send_H)
         CloseHandle(hs->recv_H);
@@ -139,7 +140,7 @@ static void sk_handle_close(Socket *s)
 #ifdef MPEXT
     if (hs->stderr_h)
     {
-        handle_free(hs->stderr_h);
+        handle_free(handles_by_evtomain, hs->stderr_h); // WINSCP
     }
     if (hs->stderr_H)
     {
@@ -341,6 +342,7 @@ Socket *make_handle_socket(HANDLE send_H, HANDLE recv_H, HANDLE stderr_H,
 {
     HandleSocket *hs;
     int flags = (overlapped ? HANDLE_FLAG_OVERLAPPED : 0);
+    tree234 * handles_by_evtomain = get_callback_set(hs->plug)->handles_by_evtomain; // WINSCP
 
     hs = snew(HandleSocket);
     hs->sock.vt = &HandleSocket_sockvt;
@@ -351,12 +353,12 @@ Socket *make_handle_socket(HANDLE send_H, HANDLE recv_H, HANDLE stderr_H,
     psb_init(&hs->psb);
 
     hs->recv_H = recv_H;
-    hs->recv_h = handle_input_new(hs->recv_H, handle_gotdata, hs, flags);
+    hs->recv_h = handle_input_new(handles_by_evtomain, hs->recv_H, handle_gotdata, hs, flags); // WINSCP
     hs->send_H = send_H;
-    hs->send_h = handle_output_new(hs->send_H, handle_sentdata, hs, flags);
+    hs->send_h = handle_output_new(handles_by_evtomain, hs->send_H, handle_sentdata, hs, flags); // WINSCP
     hs->stderr_H = stderr_H;
     if (hs->stderr_H)
-        hs->stderr_h = handle_input_new(hs->stderr_H, handle_stderr,
+        hs->stderr_h = handle_input_new(handles_by_evtomain, hs->stderr_H, handle_stderr, // WINSCP
                                         hs, flags);
 
     hs->defer_close = hs->deferred_close = false;

source/putty/windows/winpgntc.c

@@ -172,6 +172,7 @@ struct agent_pending_query {
     strbuf *response;
     void (*callback)(void *, void *, int);
     void *callback_ctx;
+    struct callback_set *callback_set; // WINSCP
 };
 
 static int named_pipe_agent_accumulate_response(
@@ -221,7 +222,7 @@ static size_t named_pipe_agent_gotdata(
 
 static agent_pending_query *named_pipe_agent_query(
     strbuf *query, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx)
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set) // WINSCP
 {
     agent_pending_query *pq = NULL;
     char *err = NULL, *pipename = NULL;
@@ -271,7 +272,8 @@ static agent_pending_query *named_pipe_agent_query(
     }
 
     pq = snew(agent_pending_query);
-    pq->handle = handle_input_new(pipehandle, named_pipe_agent_gotdata, pq, 0);
+    pq->callback_set = callback_set; // WINSCP
+    pq->handle = handle_input_new(callback_set->handles_by_evtomain, pipehandle, named_pipe_agent_gotdata, pq, 0); // WINSCP
     pipehandle = NULL;  /* prevent it being closed below */
     pq->response = strbuf_new_nm();
     pq->callback = callback;
@@ -296,7 +298,7 @@ static agent_pending_query *named_pipe_agent_query(
 
 void agent_cancel_query(agent_pending_query *pq)
 {
-    handle_free(pq->handle);
+    handle_free(pq->callback_set->handles_by_evtomain, pq->handle);
     if (pq->response)
         strbuf_free(pq->response);
     sfree(pq);
@@ -304,10 +306,10 @@ void agent_cancel_query(agent_pending_query *pq)
 
 agent_pending_query *agent_query(
     strbuf *query, void **out, int *outlen,
-    void (*callback)(void *, void *, int), void *callback_ctx)
+    void (*callback)(void *, void *, int), void *callback_ctx, struct callback_set * callback_set) // WINSCP
 {
     agent_pending_query *pq = named_pipe_agent_query(
-        query, out, outlen, callback, callback_ctx);
+        query, out, outlen, callback, callback_ctx, callback_set); // WINSCP
     if (pq || *out)
         return pq;
 

source/putty/windows/winstuff.h

@@ -618,19 +618,15 @@ typedef size_t (*handle_inputfn_t)(
     struct handle *h, const void *data, size_t len, int err);
 typedef void (*handle_outputfn_t)(
     struct handle *h, size_t new_backlog, int err);
-struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata,
+struct handle *handle_input_new(tree234 * handles_by_evtomain, HANDLE handle, handle_inputfn_t gotdata, // WINSCP
                                 void *privdata, int flags);
-struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata,
+struct handle *handle_output_new(tree234 * handles_by_evtomain, HANDLE handle, handle_outputfn_t sentdata, // WINSCP
                                  void *privdata, int flags);
 size_t handle_write(struct handle *h, const void *data, size_t len);
 void handle_write_eof(struct handle *h);
-HANDLE *handle_get_events(int *nevents);
-void handle_free(struct handle *h);
-#ifdef MPEXT
-int handle_got_event(HANDLE event);
-#else
-void handle_got_event(HANDLE event);
-#endif
+HANDLE *handle_get_events(tree234 * handles_by_evtomain, int *nevents); // WINSCP
+void handle_free(tree234 * handles_by_evtomain, struct handle *h); // WINSCP
+int handle_got_event(tree234 * handles_by_evtomain, HANDLE event); // WINSCP
 void handle_unthrottle(struct handle *h, size_t backlog);
 size_t handle_backlog(struct handle *h);
 void *handle_get_privdata(struct handle *h);