http://bugzilla.mindrot.org/show_bug.cgi?id=52
dtucker at zip.com.au changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #1075 is|0 |1
obsolete| |
Attachment #1098 is|0 |1
obsolete| |
------- Comment #27 from dtucker at zip.com.au 2006-11-27 15:07 -------
Created an attachment (id=1214)
--> (http://bugzilla.mindrot.org/attachment.cgi?id=1214&action=view)
Update Marc's patch to 4.5p1
Some changes in nearby code prevent the patch from applying cleanly to
4.5p1.
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=52
jochen.kirn at gmail.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #1214 is|0 |1
obsolete| |
------- Comment #28 from jochen.kirn at gmail.com 2006-11-27 21:37 -------
Created an attachment (id=1215)
--> (http://bugzilla.mindrot.org/attachment.cgi?id=1215&action=view)
Updated Patch for 4.5p1 from Darren Tucker (corrected minor typo)
I've applied the mentioned patch for openssh 4.5p1
the patch itself contains a minor typo (closing bracket) for line 1455
which is corrected in this version of the patch.
The patch fixes the issue with "ssh -f ..." on AIX
(tested&verified on
AIX 5.3 ML 4)
thanks Darren for your quick response to this problem
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=52 ------- Comment #29 from dtucker at zip.com.au 2006-11-27 23:37 ------- (In reply to comment #28)> thanks Darren for your quick response to this problemYou're welcome but Marc deserves the credit for figuring it out, I just pointed you in the right direction. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=52 ------- Comment #30 from tsi at ualberta.ca 2006-11-28 03:03 ------- I'm glad to see this change integrated. This report should be resolved as fixed, as I, for one, now consider this issue to have been corrected. Thanks. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=52 ------- Comment #31 from dtucker at zip.com.au 2006-11-28 23:03 ------- (From update of attachment 1098) The patch has not been applied, I simply pointed someone toward it.>- FD_ISSET(c->rfd, readset)) { >+ (c->detach_close || FD_ISSET(c->rfd, readset))) { > len = read(c->rfd, buf, sizeof(buf)); >- if (len < 0 && (errno == EINTR || errno == EAGAIN)) >+ if (len < 0 && errno == EINTR)I'm wondering if this could ever cause the channel to be shutdown if the read fails with EAGAIN during normal operation. POSIX says EAGAIN should be returned when the descriptor is set for nonblocking IO and select should not mark the fd in that case so this shouldn't happen. On the other hand, comment #9 has one counterexample, plus Google finds http://archives.neohapsis.com/archives/postfix/2003-09/0507.html, which indicates that select can fire on on a non-ready fd (probably not an issue here since it appears to affect only TCP sockets) but it makes me wonder what other instances are out there... [...]>+ program_alive_scheduled = child_terminated;Any reason this is a separate variable rather than just using child_terminated? A possible race where a SIGCHLD arrives after the read? On a related note, I'm also wondering if we be aiming to carry this as a portable-only change (since OpenBSD isn't directly affected and we already have to deal with some pty wackiness from AIX in this area of code, so patches won't apply cleanly already) or should we be aiming to push the change to OpenBSD too? ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=52 ------- Comment #32 from tsi at ualberta.ca 2006-11-29 04:41 ------- (In reply to comment #31)> (From update of attachment 1098 [details]) > The patch has not been applied, I simply pointed someone toward it.> >- FD_ISSET(c->rfd, readset)) { > >+ (c->detach_close || FD_ISSET(c->rfd, readset))) { > > len = read(c->rfd, buf, sizeof(buf)); > >- if (len < 0 && (errno == EINTR || errno == EAGAIN)) > >+ if (len < 0 && errno == EINTR)> I'm wondering if this could ever cause the channel to be shutdown if > the read fails with EAGAIN during normal operation.> POSIX says EAGAIN should be returned when the descriptor is set for > nonblocking IO and select should not mark the fd in that case so this > shouldn't happen.> On the other hand, comment #9 has one counterexample,How so?> plus Google finds > http://archives.neohapsis.com/archives/postfix/2003-09/0507.html, which > indicates that select can fire on on a non-ready fd (probably not an > issue here since it appears to affect only TCP sockets) but it makes me > wonder what other instances are out there...Well, you could accept EAGAIN only before detach_close is set. In the !compat20 case, child_terminated.> >+ program_alive_scheduled = child_terminated;> Any reason this is a separate variable rather than just using > child_terminated? A possible race where a SIGCHLD arrives after the > read?It's been a while, but IIRC, I did this to ensure I was looking at the same child_terminated value across the select(2). I wanted to force read(2)'s only if I also forced select(2) to check the fd(s). Even so, I wish to emphasise here that it is not possible to miss the child's death: In the compat20 case, detach_close ends up taking over from child_terminated. I.e. once set, detach_close is permanent, but collect_children() resets child_terminated. In the !compat20 case, child_terminated isn't reset.> On a related note, I'm also wondering if we be aiming to carry this as > a portable-only change (since OpenBSD isn't directly affected and we > already have to deal with some pty wackiness from AIX in this area of > code, so patches won't apply cleanly already) or should we be aiming to > push the change to OpenBSD too?That's a decision only you can make. My personal view is that the portable version should also run on OpenBSD. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.