ssh, apps, examples: take the worker's status from its return

- wolfSSH_OutputPending() moves from wolfssh/internal.h to
  wolfssh/ssh.h as a WOLFSSH_API taking a const WOLFSSH*, defined
  in src/ssh.c beside the other public calls.
- wolfSSH_worker() puts the send's code in the return in place of
  an event when the flush fails outright, and gates its flush on
  wolfSSH_OutputPending(). wolfssh/ssh.h states both.
- Both wolfsshd shell loops, both echoservers and
  ReceiveScpMessage() dispatch on wolfSSH_worker()'s return, and
  call wolfSSH_get_error() only to tell a transient failure from a
  terminal one. The POSIX wolfsshd loop sets wantWrite from
  wolfSSH_OutputPending().
- Five worker tests expect the send's code where they expected the
  event, and tests/testsuite.c calls wolfSSH_OutputPending().
pull/1249/head
Yosuke Shimizu 2026-09-14 15:58:28 +09:00 committed by John Safranek
parent 5ea9b736de
commit a14e6d0f3c
10 changed files with 90 additions and 75 deletions

View File

@ -2277,12 +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 (cnt_r == WS_CHAN_RXD || cnt_r == WS_CHANNEL_CLOSED
|| cnt_r == WS_EOF) {
rc = cnt_r;
}
if (rc == WS_CHAN_RXD) {
if (cnt_r == WS_CHAN_RXD) {
if (lastChannel == shellChannelId) {
cnt_r = wolfSSH_ChannelIdRead(ssh,
shellChannelId, shellBuffer,
@ -2299,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
@ -2315,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 && rc != WS_WANT_WRITE) {
else if (cnt_r == WS_WANT_WRITE) {
/* 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;
}
}
@ -2979,20 +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);
/* Take the owed write before the event overwrites rc. */
if (rc == WS_WANT_WRITE) {
wantWrite = 1;
}
if (cnt_r == WS_CHAN_RXD || cnt_r == WS_CHANNEL_CLOSED
|| cnt_r == WS_EOF) {
rc = cnt_r;
}
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
@ -3007,17 +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) {
/* Recorded above; the channel drain below still runs. */
}
else if (rc == WS_REKEYING) {
wantWrite = 1;
else if (cnt_r == WS_REKEYING) {
continue;
}
else if (rc != WS_WANT_READ) {
else if (cnt_r == WS_WANT_WRITE) {
/* 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)) {
/* unexpected error, kill off child process */
kill(childPid, SIGKILL);
break;
@ -3046,7 +3041,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
break;
/* The credit that read issued may still be queued. */
if (wolfSSH_get_error(ssh) == WS_WANT_WRITE)
if (wolfSSH_OutputPending(ssh))
wantWrite = 1;
childInIdx = 0;

View File

@ -1035,11 +1035,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
channel. The additional channel is only used with the
agent. */
cnt_r = wolfSSH_worker(ssh, &lastChannel);
rc = wolfSSH_get_error(ssh);
if (cnt_r == WS_CHAN_RXD || cnt_r == WS_REKEYING
|| cnt_r == WS_CHANNEL_CLOSED || cnt_r == WS_EOF) {
rc = cnt_r;
}
/* 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, since the library no longer answers for us. Off
@ -1242,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 && rc != WS_WANT_WRITE) {
else if (rc == WS_WANT_WRITE) {
/* 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);

View File

@ -995,11 +995,9 @@ static int ssh_worker(thread_ctx_t* threadCtx)
channel. The additional channel is only used with the
agent. */
cnt_r = wolfSSH_worker(ssh, &lastChannel);
rc = wolfSSH_get_error(ssh);
if (cnt_r == WS_CHAN_RXD || cnt_r == WS_REKEYING
|| cnt_r == WS_CHANNEL_CLOSED || cnt_r == WS_EOF) {
rc = cnt_r;
}
/* 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, since the library no longer answers for us. Off
@ -1169,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 && rc != WS_WANT_WRITE) {
else if (rc == WS_WANT_WRITE) {
/* 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);

View File

@ -5578,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)

View File

@ -4631,7 +4631,7 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
/* 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) {
if (wolfSSH_OutputPending(ssh) && !ssh->disconnected) {
int rxErr = ssh->error;
sendRet = wolfSSH_SendPacket(ssh);
@ -4639,6 +4639,11 @@ int wolfSSH_worker(WOLFSSH* ssh, word32* channelId)
if (ret == WS_SUCCESS) {
ret = sendRet;
}
else if (sendRet != WS_WANT_WRITE && ret != WS_CHANNEL_CLOSED
&& ret != WS_FATAL_ERROR) {
/* The transport is gone, so the event it came with is moot. */
ret = sendRet;
}
else if ((ret == WS_CHANNEL_CLOSED && sendRet != WS_WANT_WRITE)
|| (ret == WS_FATAL_ERROR && rxErr != WS_WANT_READ)) {
/* A failed receive outranks the flush, and so does a close
@ -4691,6 +4696,12 @@ int wolfSSH_GetLastRxId(WOLFSSH* ssh, word32* channelId)
}
int wolfSSH_OutputPending(const WOLFSSH* ssh)
{
return (ssh != NULL && ssh->outputBuffer.length > ssh->outputBuffer.idx);
}
#ifdef WOLFSSH_FWD
int wolfSSH_CTX_SetFwdCb(WOLFSSH_CTX* ctx,

View File

@ -1793,12 +1793,7 @@ int ReceiveScpMessage(WOLFSSH* ssh)
if (err < 0) {
int rc;
rc = wolfSSH_get_error(ssh);
if (err == WS_CHAN_RXD || err == WS_EXTDATA
|| err == WS_CHANNEL_CLOSED) {
rc = err;
}
switch (rc) {
switch (err) {
case WS_CHAN_RXD:
sz = wolfSSH_ChannelIdRead(ssh, lastChannel,
buf + ssh->scpRecvMsgSz,

View File

@ -241,6 +241,13 @@ int wolfSSH_TestsuiteTest(int argc, char** argv)
wolfSSH_Init();
/* Linked against the installed library, so this also proves
* wolfSSH_OutputPending() is exported and not hidden. */
if (wolfSSH_OutputPending(NULL) != 0) {
fprintf(stderr, "wolfSSH_OutputPending(NULL) was not zero\n");
return EXIT_FAILURE;
}
#if defined(FIPS_VERSION_GE) && FIPS_VERSION_GE(5,2)
{
int i;

View File

@ -6769,9 +6769,9 @@ done:
return result;
}
/* Channel data arrives and the flush fails on the same call. ret carries
* WS_CHAN_RXD so the caller reads the data, and ssh->error carries the send
* failure, which is the rule wolfssh/ssh.h states for wolfSSH_worker(). */
/* Channel data arrives and the flush fails on the same call. The send's
* code takes ret, since the data is still buffered but the transport is
* gone. */
static int test_WorkerChanRxdSurfacesSendError(void)
{
WOLFSSH_CTX* ctx = NULL;
@ -6811,7 +6811,7 @@ static int test_WorkerChanRxdSurfacesSendError(void)
wolfSSH_SetIOSend(ctx, ConnResetIoSend);
ret = wolfSSH_worker(ssh, NULL);
if (ret != WS_CHAN_RXD) { result = -1755; goto done; }
if (ret != WS_SOCKET_ERROR_E) { result = -1755; goto done; }
if (wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) {
result = -1756;
goto done;
@ -6895,9 +6895,8 @@ done:
}
/* Extended data arrives and the flush fails on the same call. Neither the
* close nor the receive-failure rule applies, so ret keeps WS_EXTDATA and
* ssh->error carries the send failure. */
/* Extended data arrives and the flush fails on the same call. The send's
* code takes ret, and channelId is left alone with it. */
static int test_WorkerExtDataSurfacesSendError(void)
{
WOLFSSH_CTX* ctx = NULL;
@ -6938,8 +6937,8 @@ static int test_WorkerExtDataSurfacesSendError(void)
wolfSSH_SetIOSend(ctx, ConnResetIoSend);
ret = wolfSSH_worker(ssh, &reportedId);
if (ret != WS_EXTDATA) { result = -1835; goto done; }
if (reportedId != ch->channel) { result = -1836; goto done; }
if (ret != WS_SOCKET_ERROR_E) { result = -1835; goto done; }
if (reportedId != 0xFFFFFFFF) { result = -1836; goto done; }
if (wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) {
result = -1837;
goto done;
@ -6947,7 +6946,7 @@ static int test_WorkerExtDataSurfacesSendError(void)
/* A reset does not discard, so the bytes stay owed. */
if (ssh->outputBuffer.length == 0) { result = -1838; goto done; }
/* The stderr is still there to drain, which is why ret kept it. */
/* The stderr is still there to drain, off the channel rather than ret. */
if (ch->extDataBuffer.length - ch->extDataBuffer.idx != 10) {
result = -1839;
goto done;
@ -6963,8 +6962,8 @@ done:
}
/* The peer half-closes and the flush fails on the same call. ret keeps
* WS_EOF and ssh->error carries the send failure. */
/* The peer half-closes and the flush fails on the same call. The send's
* code takes ret; the half-close is read off the channel instead. */
static int test_WorkerEofSurfacesSendError(void)
{
WOLFSSH_CTX* ctx = NULL;
@ -7005,8 +7004,8 @@ static int test_WorkerEofSurfacesSendError(void)
wolfSSH_SetIOSend(ctx, ConnResetIoSend);
ret = wolfSSH_worker(ssh, &reportedId);
if (ret != WS_EOF) { result = -1845; goto done; }
if (reportedId != ch->channel) { result = -1846; goto done; }
if (ret != WS_SOCKET_ERROR_E) { result = -1845; goto done; }
if (reportedId != 0xFFFFFFFF) { result = -1846; goto done; }
if (wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) {
result = -1847;
goto done;
@ -7083,8 +7082,8 @@ done:
/* The same pass with a rekey in flight. WS_REKEYING would tell the caller to
* keep turning the crank, so a flush that hard-failed keeps ssh->error and
* the rekey mask stands down. */
* keep turning the crank, so a flush that hard-failed takes ret and the rekey
* mask stands down. */
static int test_WorkerKeyingSurfacesSendError(void)
{
WOLFSSH_CTX* ctx = NULL;
@ -7124,7 +7123,7 @@ static int test_WorkerKeyingSurfacesSendError(void)
ssh->isKeying = WOLFSSH_SELF_IS_KEYING;
ret = wolfSSH_worker(ssh, NULL);
if (ret != WS_CHAN_RXD) { result = -1805; goto done; }
if (ret != WS_SOCKET_ERROR_E) { result = -1805; goto done; }
if (wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) {
result = -1806;
goto done;
@ -7201,7 +7200,7 @@ done:
}
/* A send that fails with WS_CBIO_ERR_GENERAL discards the output buffer, so
* no later call retries the flush. ssh->error has to keep the send failure
* no later call retries the flush. The send failure has to reach the caller
* even though the receive reported channel data. */
static int test_WorkerDiscardedFlushKeepsError(void)
{
@ -7242,7 +7241,7 @@ static int test_WorkerDiscardedFlushKeepsError(void)
wolfSSH_SetIOSend(ctx, FailIoSend);
ret = wolfSSH_worker(ssh, NULL);
if (ret != WS_CHAN_RXD) { result = -1765; goto done; }
if (ret != WS_SOCKET_ERROR_E) { result = -1765; goto done; }
/* Nothing is left to flush, so this is the only report there will be. */
if (ssh->outputBuffer.length != 0) { result = -1766; goto done; }
if (wolfSSH_get_error(ssh) != WS_SOCKET_ERROR_E) {

View File

@ -1701,7 +1701,6 @@ WOLFSSH_LOCAL int SendAfterDisconnect(WOLFSSH* ssh);
WOLFSSH_LOCAL int DoReceive(WOLFSSH* ssh);
WOLFSSH_LOCAL int DoProtoId(WOLFSSH* ssh);
WOLFSSH_LOCAL int wolfSSH_SendPacket(WOLFSSH* ssh);
WOLFSSH_LOCAL int wolfSSH_OutputPending(WOLFSSH* ssh);
/* Will the packet just framed reach the peer? A completed flush says so; the
* return does not, since the highwater callback runs after the last byte is
* out and fails with the same codes a lost send does. Take flushes from

View File

@ -96,10 +96,14 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh);
* neither as a substitute for the other.
* A want is transient either way: call again. A caller that tolerates only
* WS_WANT_READ drops live sessions, since a queued write reports
* WS_WANT_WRITE.
* Any other code is an error: WS_BAD_ARGUMENT, or WS_FATAL_ERROR with the
* cause in wolfSSH_get_error() -- WS_DISCONNECT for the peer's disconnect,
* which is how most sessions end.
* WS_WANT_WRITE. A send that fails outright takes the return instead, since
* the event it arrived with is moot once the transport is gone; only a
* WS_CHANNEL_CLOSED keeps the return there.
* Any other code is an error, either in the return itself or as
* WS_FATAL_ERROR with the cause in wolfSSH_get_error() -- WS_DISCONNECT for
* the peer's disconnect, which is how most sessions end.
* To ask whether a write is still owed, call wolfSSH_OutputPending() rather
* than reading a status: it answers after any return, including a success.
*
* For WS_CHAN_RXD, WS_EXTDATA, WS_EOF, WS_SUCCESS and a WS_REKEYING that
* displaced one of those, channelId (when not NULL) names the channel the
@ -111,6 +115,9 @@ WOLFSSH_API void wolfSSH_free(WOLFSSH* ssh);
WOLFSSH_API int wolfSSH_worker(WOLFSSH* ssh, word32* channelId);
WOLFSSH_API int wolfSSH_GetLastRxId(WOLFSSH* ssh, word32* channelId);
/* Returns nonzero if a write is still owed. Session state */
WOLFSSH_API int wolfSSH_OutputPending(const WOLFSSH* ssh);
WOLFSSH_API int wolfSSH_set_fd(WOLFSSH* ssh, WS_SOCKET_T fd);
WOLFSSH_API WS_SOCKET_T wolfSSH_get_fd(const WOLFSSH* ssh);