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

[yosuke-wolfssl]

Problem

A read-only application on a non-blocking socket stalls permanently.

A channel read credits the window, ChannelCreditWindow() bundles a CHANNEL_WINDOW_ADJUST into ssh->outputBuffer, and the socket write blocks. The credit is not re-parked, so wolfSSH_SendPacket() is the only thing that can discharge it, and the peer has spent its window and goes silent waiting for it. The application calls wolfSSH_worker(), as wolfssh/ssh.h directs — but the worker gated its flush on DoReceive()'s return, and an idle socket makes DoReceive() return WS_FATAL_ERROR, which the gate did not list. No write is attempted, on that call or any later one.

The fix (src/ssh.c)

wolfSSH_worker() flushes whenever output is queued and the session is live:

if (ssh != NULL && !ssh->disconnected && ssh->outputBuffer.length != 0) {

• ssh->error keeps the receive's code when the receive failed, and the close's when a WS_CHANNEL_CLOSED pass hard-failed its flush. Every other status keeps the send's.
• WS_REKEYING is withheld when the flush failed, so a dead transport is not reported as a rekey to drive.
• Removes the second DoReceive(), its WS_WINDOW_FULL arm, the WOLFSSH_TEST_BLOCK ordering fork, and the separate WS_CHANNEL_CLOSED flush — all four existed to work around the gate.

The gate ignores ret deliberately. An idle receive and a hard one both surface as WS_FATAL_ERROR, so a narrower gate would key on ssh->error == WS_WANT_READ and reintroduce this same class of bug: a status nobody thought to list stops the flush.

ssh->error discipline (src/internal.c)

SendPacketFlush() now records its code on every transport failure path, not only WS_WANT_WRITE — otherwise a hard send failure left the idle receive's WS_WANT_READ in place and callers would select for read on a dead socket. Two rules follow, both stated on wolfSSH_SendPacket(): a later write to ssh->error on the same pass must be conditional on the flush having succeeded, and success is never written into the field. wolfSSH_TriggerKeyExchange() violated the second, and runs from HighwaterCheck() inside wolfSSH_SendPacket(), so a clean flush could zero the field for any caller.

What the wider flush exposed

Flushing on every pass reaches two places the old gate shielded:

• BundlePacket() left outputBuffer.length past packetStartIdx when framing failed, so an unpadded, un-MAC'd, unencrypted partial packet stayed queued. The old gate excluded WS_FATAL_ERROR, so nothing went out; the new one would have put it on the wire. Its failure arm now drops that packet alone.
• wolfSSH_shutdown() mapped WS_CHAN_RXD and WS_EOF from its close-read to WS_SUCCESS. If the receive highwater fires on that read it queues a KEXINIT, and a blocked socket left the teardown reporting success with bytes owed. It now reports WS_WANT_WRITE, which ssh.h already documents for the call.

Consumers

Three shell loops treated anything but WS_WANT_READ as fatal — examples/echoserver/echoserver.c:1205, apps/wolfsshd/wolfsshd.c:1364, ide/Espressif/.../echoserver.c:1170 — one line each. For these three the teardown is pre-existing: master's worker already returns WS_WANT_WRITE via its second DoReceive() path, and master's SendPacketFlush() already records it, so they already drop a live session when the send buffer fills mid-transfer. This change only makes it easier to reach.

ssh.h states the worker's contract and its two exceptions; WS_WINDOW_FULL comes off the return list, since no path reaches it. No public API change.

Tests

Sixteen new unit tests cover the flush on an idle receive, the owed flush across calls, and what ret and ssh->error hold after a receive, send, buffer, callback or framing failure — each against channel data, extended data, a half-close, a rekey, or a channel close. One arm is knowingly uncovered: a DoReceive() returning plain WS_SUCCESS with output queued, which needs a non-channel packet builder nothing else uses.

Verification

• unit.test 159 passed / 0 failed; regress.test passed. Clean under gcc-13 -Werror across 6 configurations, plus lint.
• Every fix has a negative control: reverting it fails its own test and no other.
• Network contention (-DWOLFSSH_TEST_BLOCK, scripts/sftp.test) passes at WOLFSSH_BLOCK_PROB 70/50/30 in 61s/33s/13s. scp.test and get-put.test skip under that macro by their own design.

Known limitations, not addressed here

Five caller loops mishandle an owed flush, and they are deferred together because none has a test that can verify a fix — nothing drives a shell session under -DWOLFSSH_TEST_BLOCK.

File	Fault
examples/echoserver/echoserver.c:806	select() at :999 watches read fds, NULL timeout
ide/Espressif/.../echoserver.c:794	same, select() at :984
apps/wolfsshd/wolfsshd.c:1364	skips its select() when wolfSSH_stream_peek() has data
examples/echoserver/echoserver.c:1496	retries with no wait at all
apps/wolfsshd/wolfsshd.c:1131	retries with no wait at all

The last two are a regression this PR creates, not one it inherits. On master an idle receive attempts no flush, so ssh->error is WS_WANT_READ and both loops exit after one pass; here the flush runs every pass and re-sets WS_WANT_WRITE, so they spin until the peer's window opens. One is wolfsshd, which ships.

An earlier revision fixed those two with a tcp_select_write() helper. It was pulled out: fixing two of five left the class half-done and grew the diff without closing the stall. The follow-up writes the shell-session harness first, then fixes all five.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 2
1 finding(s) posted as inline comments (see file-level comments below)

Required changes (1)

wolfsshd WIN32 forced-command drain loop becomes an unbounded CPU spin

File: apps/wolfsshd/wolfsshd.c:1131
Function: SHELL_Subsystem
Category: Resource leaks

wolfSSH_worker() now leaves ssh->error == WS_WANT_WRITE on an idle receive with queued output (src/ssh.c:3731-3746), so this unpaced loop never terminates while the peer stops reading the non-blocking socket (set at wolfsshd.c:3412) — a remote 100% CPU spin per session.

Recommendation: Wait for socket writability with a bounded select/timeout between passes and give up after a cap instead of looping on WS_WANT_WRITE.

Referenced code: apps/wolfsshd/wolfsshd.c:1131-1135 (5 lines)

---

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 1

Required changes (1)

wolfsshd Windows drain loop becomes a delay-free busy-wait on the worker's new WS_WANT_WRITE

File: apps/wolfsshd/wolfsshd.c:1131
Function: SHELL_Subsystem
Category: Incorrect error handling

wolfSSH_worker() now leaves ssh->error == WS_WANT_WRITE when an idle receive is followed by a blocked flush (previously the removed second DoReceive() left WS_WANT_READ). This drain loop has no select(), sleep, or iteration cap, so a peer that stops reading pins a CPU core on the non-blocking connection socket for as long as it keeps the socket unwritable.

Recommendation: Wait for write readiness with select() on the write set, or bound the loop with a deadline and a pause between retries.

Referenced code: apps/wolfsshd/wolfsshd.c:1131-1136 (6 lines)

---

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[yosuke-wolfssl]

Note for last Fenrir-bot comments and reviewers:

Confirmed, and deferred deliberately.

The mechanism is as described: that drain has no select(), sleep or cap, and
wolfSSH_worker() now leaves WS_WANT_WRITE where the removed second
DoReceive() left WS_WANT_READ. The shell loop at :1364 has the same
problem for a different reason — it skips its select() whenever
wolfSSH_stream_peek() has buffered data.

Both are folded into one follow-up with examples/echoserver's ssh_worker()
and the ESP-IDF copy, which need the same write-set rework.
apps/wolfsshd/wolfsshd.c:1942-1945 is the model: one select per iteration
watching read and write together, driven by the wantWrite flag the worker's
status sets.

Not fixing it in this PR because nothing can verify it. windows-sftp.yml
drives the SFTP subsystem, never SHELL_Subsystem()'s shell path, and a
busy-wait that terminates fails no assertion — so neither CI nor a local MSVC
build would catch a mistake, only a compile break. An earlier blind fix to this
same function introduced a spin that review caught. The follow-up writes the
shell-session harness first, then the fix.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1217

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.