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

Merged
ejohnstown merged 6 commits into from
Sep 14, 2026
Merged

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
6 commits
Select commit Hold shift + click to select a range
cda04fc
apps, examples: tolerate a worker want-write
yosuke-wolfssl Sep 2, 2026
951fe5c
ssh, internal: always flush the worker's output
yosuke-wolfssl Sep 2, 2026
81764ba
scp, test.h, apps, examples: wait to write and route on the return
yosuke-wolfssl Sep 10, 2026
b38b4de
scp, apps: correct three ssh->error reads
ejohnstown Sep 11, 2026
63e85ed
scp: drain extended data in the SCP read path
yosuke-wolfssl Sep 14, 2026
dd64402
ssh, apps, examples: take the worker's status from its return
yosuke-wolfssl Sep 14, 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
34 changes: 26 additions & 8 deletions apps/wolfssh/wolfssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ static void PauseForSocket(void)
#define FLUSH_QUEUE_TIMEOUT 10


/* Statuses that are not the flush's failure: an event the worker reported,
* or the owed write itself. Anything else came from the send. */
static int FlushEventOk(int code)
{
return code == WS_SUCCESS || code == WS_WANT_READ || code == WS_CHAN_RXD
|| code == WS_EXTDATA || code == WS_REKEYING || code == WS_EOF
|| code == WS_WANT_WRITE;
}


/* A packet the socket wasn't ready for stays queued in the session, and the
* send that queued it still reports the data as taken. The peer can't answer
* a message it never received, so push the queue out here rather than go
Expand All @@ -329,18 +339,26 @@ static int FlushQueuedSend(WOLFSSH* ssh, wolfSSL_Mutex* lock)
/* the session holds the detail behind a fatal error */
ret = wolfSSH_get_error(ssh);
}

/* Sample the owed write under the lock the worker ran under, since
* another thread writes ssh->error too. */
if (FlushEventOk(ret)) {
int err = wolfSSH_get_error(ssh);

if (!FlushEventOk(err)) {
ret = err;
}
else {
ret = (err == WS_WANT_WRITE) ? WS_WANT_WRITE : WS_SUCCESS;
}
}

if (lock != NULL) {
wc_UnLockMutex(lock);
}
} while (ret == WS_WANT_WRITE && WTIME(NULL) < deadline);

/* The queue is out. Whatever the worker made of the peer's end of the
* conversation is for the reader to sort out. A rekey started on the way
* through is the reader's as well, the send itself went out. */
if (ret == WS_WANT_READ || ret == WS_CHAN_RXD || ret == WS_EXTDATA
|| ret == WS_REKEYING || ret == WS_EOF) {
ret = WS_SUCCESS;
}
/* The deadline can run out with the packet still queued. */
} while (ret == WS_WANT_WRITE && WTIME(NULL) < deadline);

return ret;
}
Expand Down
57 changes: 41 additions & 16 deletions apps/wolfsshd/wolfsshd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2056,14 +2056,29 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
/* read in case a window-change packet might be queued */
{
int rc;
int selected = WS_SELECT_SEND_READY;
WS_SOCKET_T fd = wolfSSH_get_fd(ssh);
word32 lastChannel = 0;

do {
rc = wolfSSH_worker(ssh, &lastChannel);
if (rc < 0) {
rc = wolfSSH_get_error(ssh);
}
} while (rc == WS_WANT_WRITE);
if (rc == WS_WANT_WRITE) {
selected = tcp_select_write(fd, 1);
}
} while (rc == WS_WANT_WRITE
&& selected == WS_SELECT_SEND_READY);

/* A dead socket ends the session */
if (ret == WS_SUCCESS
Comment thread
yosuke-wolfssl marked this conversation as resolved.
&& (selected == WS_SELECT_ERROR_READY
|| selected == WS_SELECT_FAIL
|| rc == WS_SOCKET_ERROR_E
|| rc == WS_DISCONNECT)) {
ret = WS_FATAL_ERROR;
}
}
}
}
Expand Down Expand Up @@ -2262,8 +2277,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
agent. */
cnt_r = wolfSSH_worker(ssh, &lastChannel);
if (cnt_r < 0) {
rc = wolfSSH_get_error(ssh);
if (rc == WS_CHAN_RXD) {
if (cnt_r == WS_CHAN_RXD) {
if (lastChannel == shellChannelId) {
cnt_r = wolfSSH_ChannelIdRead(ssh,
shellChannelId, shellBuffer,
Expand All @@ -2280,10 +2294,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
}
}
else if (rc == WS_CHANNEL_CLOSED) {
else if (cnt_r == WS_CHANNEL_CLOSED) {
continue;
}
else if (rc == WS_EOF) {
else if (cnt_r == WS_EOF) {
/* The peer is done sending. No EOF of ours here: it
* latches eofTxd and the child's remaining console
* output would then be refused, which both send sites
Expand All @@ -2296,7 +2310,12 @@ 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 (cnt_r == WS_WANT_WRITE) {
Comment thread
ejohnstown marked this conversation as resolved.
/* Transient; the queue drives the write side. */
}
else if (cnt_r != WS_FATAL_ERROR
|| (wolfSSH_get_error(ssh) != WS_WANT_READ
&& wolfSSH_get_error(ssh) != WS_WANT_WRITE)) {
break;
}
}
Expand Down Expand Up @@ -2960,12 +2979,14 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
the channel itself, so the id the worker would report here is
not needed. */
cnt_r = wolfSSH_worker(ssh, NULL);
if (wolfSSH_OutputPending(ssh)) {
wantWrite = 1;
}
if (cnt_r < 0) {
rc = wolfSSH_get_error(ssh);
if (rc == WS_CHAN_RXD) {
if (cnt_r == WS_CHAN_RXD) {
/* Arrival only; the drain below owns the read. */
}
else if (rc == WS_CHANNEL_CLOSED) {
else if (cnt_r == WS_CHANNEL_CLOSED) {
/* The channel is retired, so nothing more can reach the
* child and the drain below is skipped on this pass.
* Close its stdin here or it blocks forever on input
Expand All @@ -2980,18 +3001,18 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
peerConnected = 0;
continue;
}
else if (rc == WS_EOF) {
else if (cnt_r == WS_EOF) {
/* Half-close, handled below. */
}
else if (rc == WS_WANT_WRITE) {
wantWrite = 1;
else if (cnt_r == WS_REKEYING) {
continue;
}
else if (rc == WS_REKEYING) {
wantWrite = 1;
continue;
else if (cnt_r == WS_WANT_WRITE) {
/* Transient; the queue drives the write side. */
}
else if (rc != WS_WANT_READ) {
else if (cnt_r != WS_FATAL_ERROR
|| (wolfSSH_get_error(ssh) != WS_WANT_READ
&& wolfSSH_get_error(ssh) != WS_WANT_WRITE)) {
/* unexpected error, kill off child process */
kill(childPid, SIGKILL);
break;
Expand Down Expand Up @@ -3019,6 +3040,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
if (cnt_r <= 0)
break;

/* The credit that read issued may still be queued. */
if (wolfSSH_OutputPending(ssh))
wantWrite = 1;

childInIdx = 0;
childInSz = cnt_r;

Expand Down
2 changes: 1 addition & 1 deletion examples/client/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,7 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
* is still owed, so the drain below is exactly what is wanted. */
if (ret != WS_SOCKET_ERROR_E && wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E
&& wolfSSH_get_error(ssh) != WS_CHANNEL_CLOSED) {
if (ret != WS_SUCCESS) {
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
Comment thread
yosuke-wolfssl marked this conversation as resolved.
ClientFreeBuffers(pubKeyName, privKeyName, NULL);
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
Expand Down
28 changes: 17 additions & 11 deletions examples/echoserver/echoserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,17 +1035,14 @@ static int ssh_worker(thread_ctx_t* threadCtx)
channel. The additional channel is only used with the
agent. */
cnt_r = wolfSSH_worker(ssh, &lastChannel);
/* Take the worker's status before the drain below: its
* reads and sends latch their own into ssh->error. */
rc = wolfSSH_get_error(ssh);
/* The channel reads below overwrite cnt_r with a byte
* count, so keep the worker's status. */
rc = cnt_r;

/* 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 @@ -1243,7 +1240,12 @@ 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_WRITE) {
Comment thread
ejohnstown marked this conversation as resolved.
/* Transient; the queue drives the write side. */
}
else if (rc != WS_FATAL_ERROR
|| (wolfSSH_get_error(ssh) != WS_WANT_READ
&& wolfSSH_get_error(ssh) != WS_WANT_WRITE)) {
#ifdef SHELL_DEBUG
printf("Break:read sshFd returns %d: errno =%x\n",
cnt_r, errno);
Expand Down Expand Up @@ -1533,8 +1535,12 @@ static int sftp_worker(thread_ctx_t* threadCtx)
ret = error = wolfSSH_get_error(ssh);

/* there is an edge case where the last SFTP handshake message sent got a
* WANT_WRITE case, keep trying to send it here. */
* WANT_WRITE case, keep trying to send it here. Waits for the socket to
* take bytes again rather than retrying into a full one. */
while (error == WS_WANT_WRITE) {
selected = tcp_select_write(s, TEST_SFTP_TIMEOUT);
if (selected != WS_SELECT_SEND_READY)
Comment thread
yosuke-wolfssl marked this conversation as resolved.
break;
ret = wolfSSH_worker(ssh, NULL);
error = wolfSSH_get_error(ssh);
}
Expand Down
11 changes: 6 additions & 5 deletions examples/portfwd/portfwd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,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 Expand Up @@ -935,7 +934,9 @@ THREAD_RETURN WOLFSSH_THREAD portfwd_worker(void* args)
}

ret = wolfSSH_shutdown(ssh);
if (ret != WS_SUCCESS)
/* The socket closes next, so a queued write and a retired channel are
* both done as far as this teardown is concerned. */
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE && ret != WS_CHANNEL_CLOSED)
err_sys("Closing port forward stream failed.");

WCLOSESOCKET(sshFd);
Expand Down
4 changes: 2 additions & 2 deletions examples/scpclient/scpclient.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
if (ret != WS_CHANNEL_CLOSED && ret != WS_SOCKET_ERROR_E &&
wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E &&
wolfSSH_get_error(ssh) != WS_CHANNEL_CLOSED) {
if (ret != WS_SUCCESS) {
if (ret != WS_SUCCESS && ret != WS_WANT_WRITE) {
Comment thread
ejohnstown marked this conversation as resolved.
WLOG(WS_LOG_DEBUG, "Sending the shutdown messages failed.");
}
else {
Expand Down Expand Up @@ -351,7 +351,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
#endif

if ((ret != WS_SUCCESS) && (ret != WS_CHANNEL_CLOSED)
&& (ret != WS_EOF))
&& (ret != WS_EOF) && (ret != WS_WANT_WRITE))
Comment thread
yosuke-wolfssl marked this conversation as resolved.
((func_args*)args)->return_code = 1;
return 0;
}
Expand Down
22 changes: 12 additions & 10 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 @@ -995,17 +995,14 @@ static int ssh_worker(thread_ctx_t* threadCtx)
channel. The additional channel is only used with the
agent. */
cnt_r = wolfSSH_worker(ssh, &lastChannel);
/* Take the worker's status before the drain below: its
* reads and sends latch their own into ssh->error. */
rc = wolfSSH_get_error(ssh);
/* The channel reads below overwrite cnt_r with a byte
* count, so keep the worker's status. */
rc = cnt_r;

/* 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,12 @@ 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_WRITE) {
Comment thread
ejohnstown marked this conversation as resolved.
/* Transient; the queue drives the write side. */
}
else if (rc != WS_FATAL_ERROR
|| (wolfSSH_get_error(ssh) != WS_WANT_READ
&& wolfSSH_get_error(ssh) != WS_WANT_WRITE)) {
#ifdef SHELL_DEBUG
printf("Break:read sshFd returns %d: errno =%x\n",
cnt_r, errno);
Expand Down
16 changes: 11 additions & 5 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5479,6 +5479,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 @@ -5489,6 +5490,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 @@ -5527,11 +5529,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 @@ -5556,7 +5560,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 All @@ -5572,10 +5578,6 @@ int wolfSSH_SendPacket(WOLFSSH* ssh)
}


int wolfSSH_OutputPending(WOLFSSH* ssh)
{
return (ssh != NULL && ssh->outputBuffer.length > ssh->outputBuffer.idx);
}


static int GetInputData(WOLFSSH* ssh, word32 size)
Expand Down Expand Up @@ -14908,6 +14910,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
Loading
Loading