Skip to content

ssh, internal: flush the worker's queued output on every call #1217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 3 commits into
base: master
Choose a base branch
from
Open

ssh, internal: flush the worker's queued output on every call #1217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
454ad89
apps, examples: tolerate a worker want-write
yosuke-wolfssl Sep 2, 2026
12c0d1a
ssh: keep a successful rekey trigger out of ssh->error
yosuke-wolfssl Sep 2, 2026
c9f9857
ssh, internal: always flush the worker's output
yosuke-wolfssl Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/wolfsshd/wolfsshd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
* peer. Both want fixing where they can be tested. */
continue;
}
else if (rc != WS_WANT_READ) {
else if (rc != WS_WANT_READ && rc != WS_WANT_WRITE) {
Comment thread
yosuke-wolfssl marked this conversation as resolved.
Comment thread
yosuke-wolfssl marked this conversation as resolved.
break;
}
}
Expand Down
11 changes: 4 additions & 7 deletions examples/echoserver/echoserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,12 +1016,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
rc = wolfSSH_get_error(ssh);

/* The peer is done sending: hand back the backlog and answer
* its EOF, or a client that half-closed waits on a server
* that never finishes -- the library no longer answers for
* us. Off the channel's own state, not the WS_EOF status: the
* flush inside wolfSSH_worker() can supersede that, and it is
* raised once. Echo mode only; a shell child on a pty is
* still producing, so its EOF waits for the child to exit. */
* its EOF, since the library no longer answers for us. Off
* the channel's own state, not the once-only WS_EOF status.
* Echo mode only; a shell child on a pty still produces. */
if (!eofAnswered && echoOnly) {
WOLFSSH_CHANNEL* eofChannel;

Expand Down Expand Up @@ -1205,7 +1202,7 @@ static int ssh_worker(thread_ctx_t* threadCtx)
* above, which has already run this pass. */
continue;
}
else if (rc != WS_WANT_READ) {
else if (rc != WS_WANT_READ && rc != WS_WANT_WRITE) {
Comment thread
yosuke-wolfssl marked this conversation as resolved.
Comment thread
yosuke-wolfssl marked this conversation as resolved.
#ifdef SHELL_DEBUG
printf("Break:read sshFd returns %d: errno =%x\n",
cnt_r, errno);
Expand Down
7 changes: 3 additions & 4 deletions examples/portfwd/portfwd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,10 +781,9 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)

/* Relay the half-close so a local reader waiting on end-of-input
* returns; nothing else relays it. Driven off the latched channel
* state, not the WS_EOF status: the flush inside wolfSSH_worker()
* can supersede that, and it is raised only once. Only the channel
* appFd is wired to, since half-closing the wrong socket truncates
* a live transfer. */
* state, not the once-only WS_EOF status. Only the channel appFd
* is wired to: half-closing the wrong socket truncates a live
* transfer. */
if (appFdSet && fwdChannel != NULL && !appFdHalfClosed
&& wolfSSH_ChannelGetEof(fwdChannel)) {
int drained;
Expand Down
11 changes: 4 additions & 7 deletions ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,12 +1000,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
rc = wolfSSH_get_error(ssh);

/* The peer is done sending: hand back the backlog and answer
* its EOF, or a client that half-closed waits on a server
* that never finishes -- the library no longer answers for
* us. Off the channel's own state, not the WS_EOF status: the
* flush inside wolfSSH_worker() can supersede that, and it is
* raised once. Echo mode only; a shell child on a pty is
* still producing, so its EOF waits for the child to exit. */
* its EOF, since the library no longer answers for us. Off
* the channel's own state, not the once-only WS_EOF status.
* Echo mode only; a shell child on a pty still produces. */
if (!eofAnswered && echoOnly) {
WOLFSSH_CHANNEL* eofChannel;

Expand Down Expand Up @@ -1170,7 +1167,7 @@ static int ssh_worker(thread_ctx_t* threadCtx)
* above, which has already run this pass. */
continue;
}
else if (rc != WS_WANT_READ) {
else if (rc != WS_WANT_READ && rc != WS_WANT_WRITE) {
#ifdef SHELL_DEBUG
printf("Break:read sshFd returns %d: errno =%x\n",
cnt_r, errno);
Expand Down
12 changes: 11 additions & 1 deletion src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5142,6 +5142,7 @@ static int SendPacketFlush(WOLFSSH* ssh)

if (ssh->ctx->ioSendCb == NULL) {
WLOG(WS_LOG_DEBUG, "Your IO Send callback is null, please set");
ssh->error = WS_SOCKET_ERROR_E;
return WS_SOCKET_ERROR_E;
}

Expand All @@ -5152,6 +5153,7 @@ static int SendPacketFlush(WOLFSSH* ssh)
if (ssh->outputBuffer.length > ssh->outputBuffer.bufferSz ||
ssh->outputBuffer.length < ssh->outputBuffer.idx) {
WLOG(WS_LOG_ERROR, "Bad buffer state");
ssh->error = WS_BUFFER_E;
return WS_BUFFER_E;
}

Expand Down Expand Up @@ -5190,11 +5192,13 @@ static int SendPacketFlush(WOLFSSH* ssh)
ssh->outputBuffer.plainSz = 0;
ShrinkBuffer(&ssh->outputBuffer, 1);
}
ssh->error = WS_SOCKET_ERROR_E;
return WS_SOCKET_ERROR_E;
}

if ((word32)sent > ssh->outputBuffer.length) {
WLOG(WS_LOG_DEBUG, "wolfSSH_SendPacket() out of bounds read");
ssh->error = WS_SEND_OOB_READ_E;
return WS_SEND_OOB_READ_E;
}

Expand All @@ -5219,7 +5223,9 @@ static int SendPacketFlush(WOLFSSH* ssh)
}


/* returns WS_SUCCESS on success */
/* returns WS_SUCCESS on success. Transport failures record their code in
* ssh->error, so a later write to that field on the same pass has to be
* conditional on this having succeeded, or it hides the dead transport. */
int wolfSSH_SendPacket(WOLFSSH* ssh)
{
int ret;
Expand Down Expand Up @@ -14364,6 +14370,10 @@ static int BundlePacket(WOLFSSH* ssh)
}
else {
WLOG(WS_LOG_DEBUG, "BP: failed to encrypt buffer");
if (ssh != NULL) {
/* Drop the aborted packet */
ssh->outputBuffer.length = ssh->packetStartIdx;
}
}

return ret;
Expand Down
92 changes: 35 additions & 57 deletions src/ssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,9 @@ int wolfSSH_shutdown(WOLFSSH* ssh)
/* received response */
ret = WS_SUCCESS;
}
/* A reply queued during that read has not gone out yet. */
if (ret == WS_SUCCESS && wolfSSH_OutputPending(ssh))
ret = WS_WANT_WRITE;
}

if (ssh != NULL && ssh->channelList == NULL) {
Expand Down Expand Up @@ -1288,8 +1291,11 @@ int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh)
if (ret == WS_SUCCESS && SendAfterDisconnect(ssh))
ret = WS_FATAL_ERROR;

if (ret == WS_SUCCESS)
ret = ssh->error = SendKexInit(ssh);
if (ret == WS_SUCCESS) {
ret = SendKexInit(ssh);
if (ret != WS_SUCCESS)
ssh->error = ret;
}

WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_TriggerKeyExchange(), ret = %d", ret);
return ret;
Expand Down Expand Up @@ -3731,6 +3737,7 @@ const char* wolfSSH_GetSessionCommand(const WOLFSSH* ssh)
int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
{
int ret = WS_SUCCESS;
int sendRet = WS_SUCCESS;

WLOG(WS_LOG_DEBUG, "Entering wolfSSH_worker()");

Expand All @@ -3747,59 +3754,30 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
return WS_FATAL_ERROR;
}

#ifdef WOLFSSH_TEST_BLOCK
/* In forced non-blocking test mode, keep legacy ordering (send before
* receive) to match the harness expectations and avoid synthetic spins. */
if (ret == WS_SUCCESS) {
if (ssh->outputBuffer.length != 0)
ret = wolfSSH_SendPacket(ssh);
}
if (ret == WS_SUCCESS)
ret = DoReceive(ssh);
#else
/* Always service inbound data first so window updates can unblock sends. */
if (ret == WS_SUCCESS) {
ret = DoReceive(ssh);
}

/* If receive only wanted read or delivered channel data, still try to
* flush any pending outbound packets. */
if (ret == WS_SUCCESS || ret == WS_WANT_READ || ret == WS_CHAN_RXD
|| ret == WS_EOF) {
int sendRet = WS_SUCCESS;

if (ssh->outputBuffer.length != 0)
sendRet = wolfSSH_SendPacket(ssh);

/* If send is back-pressured, immediately try another receive to pick
* up potential window-adjusts and then return the send status. The
* send status wins; a peer EOF stays latched on the channel. */
if (sendRet == WS_WANT_WRITE || sendRet == WS_WINDOW_FULL) {
int recv2 = DoReceive(ssh);
if (recv2 == WS_SUCCESS || recv2 == WS_WANT_READ || recv2 == WS_CHAN_RXD
|| recv2 == WS_EOF)
ret = sendRet;
else
ret = recv2;
}
else {
/* Preserve meaningful receive status when send succeeded. */
if (sendRet != WS_SUCCESS)
/* Flush queued output whatever DoReceive() made of the socket, since an
* idle receive reports WS_FATAL_ERROR. !ssh->disconnected gates it. */
if (ssh != NULL && !ssh->disconnected && ssh->outputBuffer.length != 0) {
int rxErr = ssh->error;

sendRet = wolfSSH_SendPacket(ssh);
if (sendRet != WS_SUCCESS) {
Comment thread
yosuke-wolfssl marked this conversation as resolved.
if (ret == WS_SUCCESS) {
ret = sendRet;
/* else leave ret as prior receive result (SUCCESS/WANT_READ/CHAN_RXD). */
}
else if ((ret == WS_CHANNEL_CLOSED && sendRet != WS_WANT_WRITE)
Comment thread
yosuke-wolfssl marked this conversation as resolved.
|| (ret == WS_FATAL_ERROR && rxErr != WS_WANT_READ)) {
/* A failed receive outranks the flush, and so does a close
* whose flush hard-failed: callers route teardown on it.
* Every other status keeps the code the send set. */
ssh->error = rxErr;
Comment thread
yosuke-wolfssl marked this conversation as resolved.
}
}
}
#endif /* WOLFSSH_TEST_BLOCK */

/* DoChannelClose() bundles the reply inside DoReceive(), and callers
* treat the close as terminal, so flush it here. The close stays the
* return value; a short flush leaves WS_WANT_WRITE latched. */
if (ret == WS_CHANNEL_CLOSED && ssh->outputBuffer.length != 0) {
int closeErr = ssh->error;

if (wolfSSH_SendPacket(ssh) == WS_SUCCESS)
ssh->error = closeErr;
}

/* WS_EXTDATA and WS_EOF report the channel too, so a multi-channel caller
* can route the drain, or see which channel half-closed. */
Expand All @@ -3809,12 +3787,10 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
*channelId = ssh->lastRxId;
}

/* WS_EXTDATA and WS_EOF are raised once, on arrival; masking either
* strands the event, and the stderr window credit with it. A
* disconnect cannot be seen here: the gate at the top returns before
* this, and the DISCONNECT that sets the flag mid-pass leaves ret
* fatal. */
if (ssh->isKeying && ret != WS_EXTDATA && ret != WS_EOF) {
/* Report the rekey, unless it would hide a once-only WS_EXTDATA
* or WS_EOF, or the error from a flush that failed. */
if (ssh->isKeying && ret != WS_EXTDATA && ret != WS_EOF
&& sendRet == WS_SUCCESS) {
ssh->error = WS_REKEYING;
return WS_REKEYING;
}
Expand Down Expand Up @@ -4252,8 +4228,9 @@ static int _ChannelRead(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
}
}
else {
/* SendPacket() records only WS_WANT_WRITE, so a hard failure has to
* be recorded here; rewriting WS_WANT_WRITE is deliberate. */
/* The adjust can fail before it reaches the transport, so the code
* is recorded here; the log skips a WS_WANT_WRITE, which only asks
* for a retry. */
ssh->error = updateResult;
if (updateResult != WS_WANT_WRITE) {
WLOG(WS_LOG_ERROR,
Expand Down Expand Up @@ -4313,8 +4290,9 @@ static int _ChannelReadExt(WOLFSSH_CHANNEL* channel, byte* buf, word32 bufSz)
ssh->error = savedError;
}
else {
/* SendPacket() sets ssh->error only for WS_WANT_WRITE, so hard
* failures must be recorded here or they stay hidden. */
/* The adjust can fail before it reaches the transport, so the
* code is recorded here; the log skips a WS_WANT_WRITE, which
* only asks for a retry. */
ssh->error = adjustResult;
if (adjustResult != WS_WANT_WRITE) {
WLOG(WS_LOG_ERROR,
Expand Down
16 changes: 9 additions & 7 deletions tests/regress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8277,11 +8277,19 @@ static void TestWorkerReportsDisconnect(void)
wolfSSH_SetIOReadCtx(ssh, &io);
wolfSSH_SetIOWriteCtx(ssh, &io);

/* Queued output, so the flush on this pass has something to push. */
ssh->outputBuffer.length = 1;
ssh->outputBuffer.idx = 0;
ssh->outputBuffer.buffer[0] = 0;

AssertIntEQ(wolfSSH_worker(ssh, NULL), WS_FATAL_ERROR);
AssertIntEQ(wolfSSH_get_error(ssh), WS_DISCONNECT);
AssertTrue(ssh->disconnected);
AssertTrue(ssh->isKeying != 0);
io.outSz = 0;

/* The queued byte stays put: the session ended on this very pass. */
AssertIntEQ(io.outSz, 0);
AssertTrue(wolfSSH_OutputPending(ssh));

/* The message behind it is still queued, and every further pass reports
* the disconnect rather than the WS_SUCCESS of a skipped dispatch or the
Expand Down Expand Up @@ -8954,7 +8962,6 @@ static void TestPasswordEofNoCrash(void)
* still needs to service Receive() so window-adjusts can arrive and
* unblock the flow control. Verify the receive callback is invoked even
* when the first send attempt would block. */
#ifndef WOLFSSH_TEST_BLOCK
static int recvCallCount;

static int WantWriteSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
Expand All @@ -8970,7 +8977,6 @@ static int WantReadRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx)
return WS_CBIO_ERR_WANT_READ;
}

#ifndef WOLFSSH_TEST_BLOCK
static void TestWorkerReadsWhenSendWouldBlock(void)
{
WOLFSSH_CTX* ctx;
Expand Down Expand Up @@ -9005,8 +9011,6 @@ static void TestWorkerReadsWhenSendWouldBlock(void)
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
}
#endif /* !WOLFSSH_TEST_BLOCK */
#endif


#ifdef WOLFSSH_SFTP
Expand Down Expand Up @@ -13303,9 +13307,7 @@ int main(int argc, char** argv)
TestClientBuffersIdempotent();
#endif
TestPasswordEofNoCrash();
#ifndef WOLFSSH_TEST_BLOCK
TestWorkerReadsWhenSendWouldBlock();
#endif

#ifdef KEXDH_REPLY_REGRESS_KEX_ALGO
#ifndef WOLFSSH_NO_RSA_SHA2_256
Expand Down
Loading
Loading