From 70b22bd97d4fc83776c03d9abfec0950014950da Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 1 Sep 2026 19:27:02 -0700 Subject: [PATCH] echoserver: let the cleanup callback tear down WOLFSSH_FWD_LOCAL_CLEANUP now runs, and it runs from DoChannelClose() ahead of the WS_CHANNEL_CLOSED the worker sees. The handler has already closed the socket and moved the state on by then, so the recovery branch no longer matched and left ssh_worker() holding a closed descriptor. - guard the handler's close: the open can fail after the setup, with nothing yet connected - gate the handler on the channel id the library passes in the port parameter. A channel can outlive its turn in the single forwarding slot, and a cleanup arriving after the next forward has moved in would close that one's live socket - have the recovery branch clear its stale copy of the descriptor when the handler got there first, and still do the whole teardown for a locally opened forward, which draws no callback - resolve the closed channel with wolfSSH_GetLastRxId(). wolfSSH_worker() names the channel only for the data and EOF statuses, so the recovery branch was comparing against a stale zero and ran only for a forward that happened to be channel 0 - clear the pending direct connect as well: it is only cleared on success, so a refused target left it set and the worker connected again with the host name the handler had just freed --- examples/echoserver/echoserver.c | 69 ++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 00320b7c..9bda3270 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -508,17 +508,28 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, appCtx->state = APP_STATE_CONNECT; } else if (action == WOLFSSH_FWD_LOCAL_CLEANUP) { - WCLOSESOCKET(appCtx->appFd); - appCtx->appFd = -1; - if (fwdCbCtx->hostName) { - WFREE(fwdCbCtx->hostName, NULL, 0); - fwdCbCtx->hostName = NULL; + /* The channel id rides in the port parameter. A channel can outlive + * its turn in the slot, so only the holder may tear it down. */ + if (port == appCtx->channelId) { + /* This runs now, so the socket may already be gone: the open can + * fail after the setup, before anything connected. */ + if (appCtx->appFd != (WS_SOCKET_T)-1) { + WCLOSESOCKET(appCtx->appFd); + appCtx->appFd = -1; + } + if (fwdCbCtx->hostName) { + WFREE(fwdCbCtx->hostName, NULL, 0); + fwdCbCtx->hostName = NULL; + } + if (fwdCbCtx->originName) { + WFREE(fwdCbCtx->originName, NULL, 0); + fwdCbCtx->originName = NULL; + } + /* A refused connect leaves this set; retire it with the + * channel. */ + fwdCbCtx->isDirect = 0; + appCtx->state = APP_STATE_INIT; } - if (fwdCbCtx->originName) { - WFREE(fwdCbCtx->originName, NULL, 0); - fwdCbCtx->originName = NULL; - } - appCtx->state = APP_STATE_INIT; } else if (action == WOLFSSH_FWD_REMOTE_SETUP) { struct sockaddr_in addr; @@ -1181,21 +1192,35 @@ static int ssh_worker(thread_ctx_t* threadCtx) } else if (rc == WS_CHANNEL_CLOSED) { #ifdef WOLFSSH_FWD - if (threadCtx->fwdCtx.state == APP_STATE_CONNECTED && - lastChannel == threadCtx->fwdCtx.channelId) { - /* Read zero-returned. Socket is closed. Go back - to listening. */ - if (fwdFd != -1) { - WCLOSESOCKET(fwdFd); + /* wolfSSH_worker() names the channel only for the + * data and EOF statuses; DoChannelClose() recorded + * the id it retired. */ + wolfSSH_GetLastRxId(ssh, &lastChannel); + if (lastChannel == threadCtx->fwdCtx.channelId) { + if (threadCtx->fwdCtx.appFd == -1) { + /* The LOCAL_CLEANUP handler ran ahead of + * this and closed the socket; only this + * copy of the descriptor is stale. */ fwdFd = -1; - threadCtx->fwdCtx.appFd = -1; } - if (threadCtx->fwdCbCtx.originName != NULL) { - WFREE(threadCtx->fwdCbCtx.originName, - NULL, 0); - threadCtx->fwdCbCtx.originName = NULL; + else if (threadCtx->fwdCtx.state + == APP_STATE_CONNECTED) { + /* A locally opened forward is armed by no + * LOCAL_SETUP and so draws no cleanup. Its + * teardown is still ours: go back to + * listening. */ + if (fwdFd != -1) { + WCLOSESOCKET(fwdFd); + fwdFd = -1; + threadCtx->fwdCtx.appFd = -1; + } + if (threadCtx->fwdCbCtx.originName != NULL) { + WFREE(threadCtx->fwdCbCtx.originName, + NULL, 0); + threadCtx->fwdCbCtx.originName = NULL; + } + threadCtx->fwdCtx.state = APP_STATE_LISTEN; } - threadCtx->fwdCtx.state = APP_STATE_LISTEN; } #endif continue;