| Summary: | sshd closes stderr but not stdout when child process exits | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | JD Paul <jdpaul> | ||||||
| Component: | sshd | Assignee: | Damien Miller <djm> | ||||||
| Status: | CLOSED FIXED | ||||||||
| Severity: | normal | CC: | ahmedsayeed1982, djm, dtucker, jjelen | ||||||
| Priority: | P5 | ||||||||
| Version: | 6.1p1 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 2915 | ||||||||
| Attachments: |
|
||||||||
|
Description
JD Paul
2013-02-16 02:50:32 AEDT
thanks, we'll take a look at this for 6.3. Retarget to openssh-6.4 Retarget 6.3 -> 6.4 Retarget incomplete bugs / feature requests to 6.6 release Retarget incomplete bugs / feature requests to 6.6 release Retarget to 6.7 release, since 6.6 was mostly bugfixing. Remove from 6.6 tracking bug Retarget incomplete bugs to 6.8 release. These bugs are no longer targeted at the imminent 6.7 release OpenSSH 6.8 is approaching release and closed for major work. Retarget these bugs for the next release. Retarget to 6.9 AFAIK, some of this behaviour was the workaround on bug 52. Yes, the issue is certainly related to the bug #52, but I don't think it is caused by the workaround for this bug. The patch for the above mentioned bug modifies handling of output and error streams, but each of them different way. The stdout is blocked without TTY but the stderr is not. I spent some time with this case and finally ended up with the patch below. I was unable to reproduce the problems from the previous bug nor from this one. Yes, it is basically the same hack for the seconds stream, but probably better than noting and running into another weird and unexpected SIGPIPE exits. Also running regress tests passed for me just fine with 6.6p1 version, but this part didn't see many changes since then so git head should be good too (still running some tests). diff --git a/channels.c b/channels.c index 7ee1f98..2601ad5 100644 --- a/channels.c +++ b/channels.c @@ -1835,7 +1835,7 @@ channel_handle_efd(Channel *c, fd_set *readset, fd_set *writeset) debug2("channel %d: read %d from efd %d", c->self, len, c->efd); if (len < 0 && (errno == EINTR || ((errno == EAGAIN || - errno == EWOULDBLOCK) && !c->detach_close))) + errno == EWOULDBLOCK) && !(c->isatty && c->detach_close)))) return 1; if (len <= 0) { debug2("channel %d: closing read-efd %d", Damien, any chance to get this fixed upstream? We got confirmation that the patch from last comment works and solves the problem of closing stderr. Created attachment 3240 [details]
Copy close-deferral logic for efd from rfd
IMO this is the best way to fix it - this copies the conditions that we use for stdout for stderr. Relative to Jakub's patch, it additionally tests the channel state.
Seems to pass the test that JD Paul kindly shared.
This has been applied - thanks again for the detailed analysis and sorry that this slipped my radar for so long. This should be in the OpenSSH 8.0 release. close bugs that were resolved in OpenSSH 8.5 release cycle [spam removed] |