On Sun, 4 Apr 2021, Christian Weisgerber wrote: > I've been puzzling over the disparate exit codes returned by ssh > when the remote command is killed by a signal: > > $ ssh host1 'kill -HUP $$'; echo $? > 129 > $ ssh host2 'kill -HUP $$'; echo $? > 255 > > This results from the interaction of > (a) the behavior of the login shell and > (b) an implementation gap in OpenSSH's ssh(1) client. > > If the login shell is bash, its exit triggers the WIFSIGNALED() > check in session_exit_message() and an "exit-signal" message is > sent. However, client_input_channel_req() does not handle > "exit-signal" at all, exit_status is not set and retains its > initial value -1. Therefore ssh returns 255. > > If the login shell is OpenBSD's ksh, its exit triggers the WIFEXITED() > check instead and an "exit-status" message is sent. This is handled > in client_input_channel_req, and ssh returns the received exit code > of 128 + signal. > > This is not the place to discuss the correctness of the respective > shell behavior. > > However, shouldn't ssh(1) handle "exit-signal" and synthesize an exit > status of 128 + signal? That's reasonable. This implements exit-signal at the client: diff --git a/clientloop.c b/clientloop.c index 9fcf12e..0678cd3 100644 --- a/clientloop.c +++ b/clientloop.c @@ -1710,14 +1710,27 @@ client_input_channel_open(int type, u_int32_t seq, struct ssh *ssh) return r; } +static int +sig_ntod(const char *s) +{ + int i; + + for (i = 0; i < NSIG; i++) { + if (strcmp(sys_signame[i], s) == 0) + return i; + } + return -1; +} + static int client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh) { Channel *c = NULL; - char *rtype = NULL; + char *rtype = NULL, *signame = NULL; u_char reply; u_int id, exitval; - int r, success = 0; + u_char coredumped; + int r, success = 0, signum; if ((r = sshpkt_get_u32(ssh, &id)) != 0) return r; @@ -1740,8 +1753,11 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh) goto out; chan_rcvd_eow(ssh, c); } else if (strcmp(rtype, "exit-status") == 0) { - if ((r = sshpkt_get_u32(ssh, &exitval)) != 0) + if ((r = sshpkt_get_u32(ssh, &exitval)) != 0 || + (r = sshpkt_get_end(ssh)) != 0) { + error_fr(r, "parse exit-status"); goto out; + } if (c->ctl_chan != -1) { mux_exit_message(ssh, c, exitval); success = 1; @@ -1751,11 +1767,39 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh) exit_status = exitval; } else { /* Probably for a mux channel that has already closed */ - debug_f("no sink for exit-status on channel %d", - id); + debug_f("no sink for exit-status on channel %d", id); } - if ((r = sshpkt_get_end(ssh)) != 0) + } else if (strcmp(rtype, "exit-signal") == 0) { + if ((r = sshpkt_get_cstring(ssh, &signame, NULL)) != 0 || + (r = sshpkt_get_u8(ssh, &coredumped)) != 0 || + (r = sshpkt_get_cstring(ssh, NULL, NULL)) != 0 || /* msg */ + (r = sshpkt_get_cstring(ssh, NULL, NULL)) != 0 || /* lang */ + (r = sshpkt_get_end(ssh)) != 0) { + goto out; + error_fr(r, "parse exit-signal"); goto out; + } + if ((signum = sig_ntod(signame)) == -1) + exitval = 255; + else + exitval = 128 + signum; + if (c->ctl_chan != -1) { + debug("received SIG%s%s on mux channel %u", + signame, coredumped ? " (core dumped)" : "", id); + /* XXX extend mux protocol to pass through exit-sig */ + mux_exit_message(ssh, c, exitval); + success = 1; + } else if ((int)id == session_ident) { + /* Record exit value of local session */ + debug("received SIG%s%s", signame, + coredumped ? " (core dumped)" : ""); + success = 1; + exit_status = exitval; + } else { + /* Probably for a mux channel that has already closed */ + debug_f("no sink for exit-signal SIG%s on channel %u", + signame, id); + } } if (reply && c != NULL && !(c->flags & CHAN_CLOSE_SENT)) { if (!c->have_remote_id) @@ -1769,6 +1813,7 @@ client_input_channel_req(int type, u_int32_t seq, struct ssh *ssh) r = 0; out: free(rtype); + free(signame); return r; } _______________________________________________ openssh-unix-dev mailing list openssh-unix-dev@xxxxxxxxxxx https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev