Skip to content
Open
Changes from all commits
Commits
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
42 changes: 39 additions & 3 deletions apps/wolfsshd/wolfsshd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1234,6 +1234,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
int windowFull = 0; /* Contains size of bytes from shellBuffer that did
* not get passed on to wolfSSH yet. This happens
* with window full errors or when rekeying. */
int windowFullExt = 0; /* Nonzero when the bytes held in shellBuffer
* belong to the extended (stderr) data stream
* and must be resent with
* wolfSSH_extended_data_send(). */
int wantWrite = 0;
int peerConnected = 1;
int stdoutEmpty = 0;
Expand Down Expand Up @@ -1543,8 +1547,14 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}

rc = select((int)maxFd + 1, &readFds, &writeFds, NULL, NULL);
if (rc == -1)
if (rc == -1) {
/* Signal (e.g. SIGCHLD from child exit) interrupted select.
* Re-evaluate the loop condition so any pending windowFull
* data and remaining pipe contents still get drained. */
if (errno == EINTR)
continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] PTY sessions can hang after SIGCHLD · Logic errors

The EINTR retry re-enters the loop after SIGCHLD, but PTY mode never sets stdoutEmpty, so the loop shutdown condition stays true after the shell exits.

Fix: Set stdoutEmpty on PTY EOF or make the shutdown condition not require stdoutEmpty for PTY sessions.

break;
}
}
else {
pending = 1; /* found some pending SSH data */
Expand Down Expand Up @@ -1623,15 +1633,25 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,

/* if the window was previously full, try resending the data */
if (windowFull) {
cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId,
shellBuffer, windowFull);
if (windowFullExt) {
cnt_w = wolfSSH_extended_data_send(ssh, shellBuffer,
windowFull);
}
else {
cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId,
shellBuffer, windowFull);
}
if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) {
continue;
}
Comment thread
ejohnstown marked this conversation as resolved.
else if (cnt_w == WS_WANT_WRITE) {
wantWrite = 1;
continue;
}
else if (cnt_w < 0) {
kill(childPid, SIGINT);
break;
}
else {
windowFull -= cnt_w;
if (windowFull > 0) {
Expand All @@ -1640,6 +1660,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
if (windowFull < 0)
windowFull = 0;
windowFullExt = 0;
}
}

Expand All @@ -1660,15 +1681,22 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
cnt_r);
if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */
windowFull = cnt_r - cnt_w;
windowFullExt = 1;
WMEMMOVE(shellBuffer, shellBuffer + cnt_w,
windowFull);
/* don't let the stdout read below trample the
* buffered stderr remainder */
continue;
}
else if (cnt_w == WS_WINDOW_FULL ||
cnt_w == WS_REKEYING) {
windowFull = cnt_r; /* save amount to be sent */
windowFullExt = 1;
continue;
}
else if (cnt_w == WS_WANT_WRITE) {
windowFull = cnt_r;
windowFullExt = 1;
wantWrite = 1;
continue;
}
Expand Down Expand Up @@ -1698,15 +1726,19 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
shellBuffer, cnt_r);
if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */
windowFull = cnt_r - cnt_w;
windowFullExt = 0;
WMEMMOVE(shellBuffer, shellBuffer + cnt_w,
windowFull);
}
else if (cnt_w == WS_WINDOW_FULL ||
cnt_w == WS_REKEYING) {
windowFull = cnt_r; /* save amount to be sent */
windowFullExt = 0;
continue;
}
else if (cnt_w == WS_WANT_WRITE) {
windowFull = cnt_r;
windowFullExt = 0;
wantWrite = 1;
continue;
}
Expand Down Expand Up @@ -1734,15 +1766,19 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
shellBuffer, cnt_r);
if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */
windowFull = cnt_r - cnt_w;
windowFullExt = 0;
WMEMMOVE(shellBuffer, shellBuffer + cnt_w,
windowFull);
}
else if (cnt_w == WS_WINDOW_FULL ||
cnt_w == WS_REKEYING) {
windowFull = cnt_r;
windowFullExt = 0;
continue;
}
else if (cnt_w == WS_WANT_WRITE) {
windowFull = cnt_r;
windowFullExt = 0;
wantWrite = 1;
continue;
}
Expand Down
Loading