Ticket #12068: 0001-winsftp.c-avoid-creating-multiple-netevents.patch

File 0001-winsftp.c-avoid-creating-multiple-netevents.patch, 4.3 KB (added by Tobias Giesen, 4 years ago)

Final (Correted)Patch from the PuTTY Project

  • windows/window.c 

    From bd5c957e5bd0f850ab30c6b1ab94bfbbabe9f006 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 21 Dec 2019 13:31:02 +0000
Subject: [PATCH] winsftp.c: avoid creating multiple netevents.

The do_select function is called with a boolean parameter indicating
whether we're supposed to start or stop paying attention to network
activity on a given socket. So if we freeze and unfreeze the socket in
mid-session because of backlog, we'll call do_select(s, false) to
freeze it, and do_select(s, true) to unfreeze it.

But the implementation of do_select in the Windows SFTP code predated
the rigorous handling of socket backlogs, so it assumed that
do_select(s, true) would only be called at initialisation time, i.e.
only once, and therefore that it was safe to use that flag as a cue to
set up the Windows event object to associate with socket activity.
Hence, every time the socket was frozen and unfrozen, we would create
a new netevent at unfreeze time, leaking the old one.

I think perhaps part of the reason why that was hard to figure out was
that the boolean parameter was called 'startup' rather than 'enable'.
To make it less confusing the next time I read this code, I've also
renamed it, and while I was at it, adjusted another related comment.
---
 windows/window.c   |  4 ++--
 windows/winplink.c |  4 ++--
 windows/winsftp.c  | 14 +++++++++-----
 windows/winstuff.h |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/windows/window.c b/windows/window.c
index f9e09aa3..a17a9854 100644
    a b void cleanup_exit(int code)  
    10261026/*
    10271027 * Set up, or shut down, an AsyncSelect. Called from winnet.c.
    10281028 */
    1029 char *do_select(SOCKET skt, bool startup)
     1029char *do_select(SOCKET skt, bool enable)
    10301030{
    10311031    int msg, events;
    1032     if (startup) {
     1032    if (enable) {
    10331033        msg = WM_NETEVENT;
    10341034        events = (FD_CONNECT | FD_READ | FD_WRITE |
    10351035                  FD_OOB | FD_CLOSE | FD_ACCEPT);

  • windows/winplink.c 

    diff --git a/windows/winplink.c b/windows/winplink.c
index dce5691f..39ca8fb5 100644
    a b static void version(void)  
    190190    exit(0);
    191191}
    192192
    193 char *do_select(SOCKET skt, bool startup)
     193char *do_select(SOCKET skt, bool enable)
    194194{
    195195    int events;
    196     if (startup) {
     196    if (enable) {
    197197        events = (FD_CONNECT | FD_READ | FD_WRITE |
    198198                  FD_OOB | FD_CLOSE | FD_ACCEPT);
    199199    } else {

  • windows/winsftp.c 

    diff --git a/windows/winsftp.c b/windows/winsftp.c
index b120890c..91c3f2fd 100644
    a b char *dir_file_cat(const char *dir, const char *file)  
    467467 */
    468468static SOCKET sftp_ssh_socket = INVALID_SOCKET;
    469469static HANDLE netevent = INVALID_HANDLE_VALUE;
    470 char *do_select(SOCKET skt, bool startup)
     470char *do_select(SOCKET skt, bool enable)
    471471{
    472472    int events;
    473     if (startup)
     473    if (enable)
    474474        sftp_ssh_socket = skt;
    475475    else
    476476        sftp_ssh_socket = INVALID_SOCKET;
    477477
     478    if (netevent == INVALID_HANDLE_VALUE)
     479        netevent = CreateEvent(NULL, false, false, NULL);
     480
    478481    if (p_WSAEventSelect) {
    479         if (startup) {
     482        if (enable) {
    480483            events = (FD_CONNECT | FD_READ | FD_WRITE |
    481484                      FD_OOB | FD_CLOSE | FD_ACCEPT);
    482             netevent = CreateEvent(NULL, false, false, NULL);
    483485        } else {
    484486            events = 0;
    485487        }
    … … char *ssh_sftp_get_cmdline(const char *prompt, bool no_fds_ok)  
    732734    do {
    733735        ret = do_eventsel_loop(ctx->event);
    734736
    735         /* Error return can only occur if netevent==NULL, and it ain't. */
     737        /* do_eventsel_loop can't return an error (unlike
     738         * ssh_sftp_loop_iteration, which can return -1 if select goes
     739         * wrong or if the socket doesn't exist). */
    736740        assert(ret >= 0);
    737741    } while (ret == 0);
    738742

  • windows/winstuff.h 

    diff --git a/windows/winstuff.h b/windows/winstuff.h
index 0b191d54..c189c37a 100644
    a b DECL_WINDOWS_FUNCTION(GLOBAL, int, select,  
    350350 * Provided by each client of winnet.c, and called by winnet.c to turn
    351351 * on or off WSA*Select for a given socket.
    352352 */
    353 char *do_select(SOCKET skt, bool startup);
     353char *do_select(SOCKET skt, bool enable);
    354354
    355355/*
    356356 * Network-subsystem-related functions provided in other Windows modules.