ssh.c: report the byte count from a channel read that defers its credit

[yosuke-wolfssl]

Problem

Two defects in the same three lines of src/ssh.c.

1. A read that copied bytes reports failure. _ChannelRead() — behind
wolfSSH_ChannelRead() and wolfSSH_ChannelIdRead() — consumes the bytes, then
returns the window-adjust send result:

inputBuffer->idx += bufSz;                    /* bytes consumed */
updateResult = _UpdateChannelWindow(channel);
if (updateResult == WS_SUCCESS)
    updateResult = bufSz;
return updateResult;                          /* WS_WANT_WRITE -> "failed" */

WS_WANT_WRITE is routine on a non-blocking socket, not an error. A caller using the
usual if (cnt_r <= 0) break; shape tears the session down after the data has already
left the input buffer (src/wolfscp.c, apps/wolfsshd/wolfsshd.c). A hard transport
failure during the adjust was invisible on both paths.

2. A channel that never gets its window back. wolfSSH_stream_read() credited
before advancing inputBuffer->idx, so it credited the previous read's bytes. On a
session's first read idx is 0 and no SSH_MSG_CHANNEL_WINDOW_ADJUST is sent at all;
if that read drained the window, neither side can move.

Fix (src/ssh.c)

Both paths report the bytes copied and keep the send result in ssh->error, matching
_ChannelReadExt(). wolfSSH_stream_read() advances idx before crediting, which is
also the fix for defect 2. _ChannelRead() retires a stale WS_WANT_WRITE only when
this read's own credit went out, and gains its sibling's idx > length guard and
savedError restore.

src/wolfsftp.c: wolfSSH_SFTP_Close()'s STATE_CLOSE_SEND arm was the file's
only ungated NoticeError() check, so a parked WS_WANT_WRITE made it report a
failed close. Now gated like its siblings.

API compatibility

The three read entry points now return the byte count where a deferred or failed
adjust produced a negative return; the status moves to wolfSSH_get_error().
wolfSSH_ChannelReadExt() already had this shape (50ee1b61). Documented on all
three prototypes. Needs a ChangeLog.md line at release prep.

Tests (tests/unit.c)

One harness per path: a deferred adjust, then a hard failure, then a clean credit. The
ChannelIdRead harness adds an empty read, which must leave a seeded WS_WANT_WRITE
standing.

Verification

make check passes, and unit.test is clean under ASan + UBSan and under -Werror
with gcc-13 in 6 configurations. Negative controls: reverting the ssh->error
recording fails the hard-failure assertions, restoring the old ordering sends 0
adjusts where the fix sends 1, and removing the retire gate lets an empty read clear a
WS_WANT_WRITE. (scripts/sftp.test and scripts/scp.test need a serial run on a
loaded machine — a pre-existing 2 second ready-file timeout, unrelated to this change.)

Not in this PR

wolfSSH_worker() does not flush a parked adjust when its receive is idle, so a
read-only application whose peer has exhausted its window can stall — and there is no
public flush, since wolfSSH_SendPacket() and wolfSSH_OutputPending() are
WOLFSSH_LOCAL. Closing it means restructuring wolfSSH_worker(), which is
caller-visible; that is on its own branch. This PR neither creates nor widens the gap,
and its header no longer promises a flush that does not happen.

[Copilot]

Pull request overview

This pull request fixes a correctness issue in wolfSSH’s channel read paths where a deferred CHANNEL_WINDOW_ADJUST send (e.g., WS_WANT_WRITE on non-blocking sockets) could cause the read API to report failure even after bytes were already delivered/consumed, leading callers to prematurely tear down connections.

Changes:

• Update wolfSSH_stream_read() and the internal _ChannelRead() path (used by wolfSSH_ChannelIdRead() / wolfSSH_ChannelRead()) to always return the number of bytes copied/consumed, decoupling that from the window-adjust send result.
• Record non-success window-adjust send results in ssh->error (and log hard failures), matching the “bytes delivered vs. credit flushed” split already used by _ChannelReadExt().
• Add targeted unit tests covering both affected read entry points under a WS_WANT_WRITE send scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/ssh.c	Ensures channel/stdout read APIs report bytes read even when window-adjust send is deferred, while still surfacing the deferred/failed credit via ssh->error.
tests/unit.c	Adds unit tests validating correct byte reporting, payload integrity, local window crediting, and ssh->error behavior under deferred window-adjust sends.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1192

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 #1192

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.

[ejohnstown]

The flush advice added to three public prototypes in wolfssh/ssh.h is wrong in the case that matters.

Details

Proved with a harness against libwolfssh_test.a: after a deferred adjust, five wolfSSH_worker() calls on an idle socket produce zero IO sends and leave the 24-byte WINDOW_ADJUST sitting in the output buffer, because GetInputData() returns WS_FATAL_ERROR with WS_WANT_READ and that is outside worker’s flush gate. That is exactly the state stalled channel is in. The alternative remedy is unreachable: wolfSSH_SendPacket and wolfSSH_OutputPending are WOLFSSH_LOCAL, so a read-only application has no public flush at all. Not created by this PR, but this PR promotes the wrong advice into the public API documentation. (This is the same underlying gap as the existing worker-idle-flush note.)

[ejohnstown]

One other thing. Make the PR title match your one commit's title. Thanks!