ssh.c: don't fail a channel read whose window credit is deferred

[yosuke-wolfssl]

Problem

src/ssh.c has three channel read paths. _ChannelReadExt() already separates
"bytes were delivered" from "the window-adjust packet made it onto the wire".
The other two conflate them.

_ChannelRead() — behind wolfSSH_ChannelIdRead() and wolfSSH_ChannelRead() —
consumes the bytes, then returns the send result:

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

_UpdateChannelWindow() returns WS_WANT_WRITE whenever the adjust would block —
routine on a non-blocking socket, not an error. The caller is told the read failed
after the data has already left the input buffer, so those bytes are gone, and a
caller using the usual if (cnt_r <= 0) break; shape tears the connection down
(src/wolfscp.c:1762 is one). wolfSSH_stream_read() has the same shape without the
loss — it skips inputBuffer->idx += n, so the data is re-delivered — but still
reports a failure for a read that copied n bytes.

SendPacket() sets ssh->error only for WS_WANT_WRITE, so a hard transport failure
during the adjust was invisible on both paths.

Fix (src/ssh.c)

Both paths now report the bytes copied and keep the send result out of band, matching
_ChannelReadExt():

• _ChannelRead() returns bufSz unconditionally; wolfSSH_stream_read()
  advances inputBuffer->idx before the adjust, then returns n.
• Both record a non-success adjust result in ssh->error and WLOG at
  WS_LOG_ERROR for anything other than WS_WANT_WRITE.
• _ChannelRead() also clears a stale WS_WANT_WRITE once the adjust does go out
  with the output buffer drained — its entry points, unlike wolfSSH_stream_read(),
  do not reset ssh->error.

A deferred credit still goes out: wolfSSH_worker() and the next SendChannelData()
flush pending output before doing anything else.

API note: wolfSSH_stream_read(), wolfSSH_ChannelRead() and
wolfSSH_ChannelIdRead() now return the byte count where a deferred or failed window
adjust previously produced a negative return. Callers that relied on that negative
return must check wolfSSH_get_error() after a successful read. Documented in
wolfssh/ssh.h and the wolfSSH_stream_read() block comment.

Tests (tests/unit.c)

One harness per path, each putting a full window of channel data and reading it back:

Phase	IO send	Asserts
Deferred credit	WS_CBIO_ERR_WANT_WRITE	byte count, payload, ssh->error, credited window, consumed buffer
Hard failure	WS_CBIO_ERR_GENERAL	byte count still returned, ssh->error == WS_SOCKET_ERROR_E, credit left owed
Clean credit (wolfSSH_ChannelIdRead() only)	full send	ssh->error back to WS_SUCCESS, no credit owed

Verification

• unit.test, api.test, testsuite.test and scripts/{sftp,scp,fwd,get-put}.test
  all pass; unit.test also clean under ASan + UBSan.
• Clean under -Werror with gcc-13 across 6 configurations (enable-all, Zephyr
  defines, sftp-only, scp-only, default, small-stack).
• Negative controls: reverting the ssh->error recording fails both hard-failure
  assertions; reverting the stale-WS_WANT_WRITE clearing fails the third phase.

The echo server's worker loop has its own partial-write problems in this area. Those
are a separate rework PR that builds on this one — deliberately not in scope here.

[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.