| Summary: | sshd freeze when build without HAVE_PPOLL | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Yaroslav <yaroslav.kuzmin> | ||||
| Component: | sshd | Assignee: | Assigned to nobody <unassigned-bugs> | ||||
| Status: | CLOSED FIXED | ||||||
| Severity: | normal | CC: | djm, dtucker | ||||
| Priority: | P5 | ||||||
| Version: | 8.9p1 | ||||||
| Hardware: | Itanium | ||||||
| OS: | Other | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 3395 | ||||||
| Attachments: |
|
||||||
Fair enough. Created attachment 3586 [details]
Check that returned events were actually requestd
I collapsed the checks onto the same line and put the cheaper check first.
Does this also resolve the problem?
yes it resolve the problem Comment on attachment 3586 [details] Check that returned events were actually requestd Looks good to me. Although unrelated, I think the POLLIN case should check (revents & (POLLIN|POLLHUP)) as this can be returned too (not sure whether without POLLIN too). Also maybe both POLLIN and POLLOUT cases should check POLLERR in revents. I saw this in bug #3405 (In reply to Damien Miller from comment #4) > Looks good to me. Although unrelated, I think the POLLIN case should > check (revents & (POLLIN|POLLHUP)) as this can be returned too (not > sure whether without POLLIN too). I don't follow: it's not checking revents that will be returned from poll(), it's checking events that were passed to poll(). According to the man page POLLHUP is meaningless and ignored in events and only meaningful in revents (which we're not checking). > Also maybe both POLLIN and POLLOUT cases should check POLLERR in > revents. I saw this in bug #3405 Good point, I'll do that as a separate patch. (In reply to Darren Tucker from comment #5) [...] > > Also maybe both POLLIN and POLLOUT cases should check POLLERR in > > revents. I saw this in bug #3405 > > Good point, I'll do that as a separate patch. I take that back :-) Same thing, we are not checking the return from a poll(), we are checking the return from a select() and mapping it to what would be returned by poll(). Conditions that would cause poll() to set POLLERR will cause select to set the bit in readfds or writefds (likely both), but we can't tell that from what select returns. We won't see there's an error condition until the calling code attempts a read or write and gets a 0 or -1 returned. Anyway the patch has been applied (both master and V_8_9) and will be in the next release. Thanks for the report. Closing bugs from openssh-9.1 release cycle OpenSSH 9.3 has been released. Close resolved bugs |
In openbsd-compat/bsd-poll.c file in ppoll() function. After calling pselect() and converting fd_set (readfds, writefds, exceptfds) into struct pollfd , there is no check whether this event was requested or not. if (FD_ISSET(fd, readfds)) fds[i].revents |= POLLIN; if (FD_ISSET(fd, writefds)) fds[i].revents |= POLLOUT; if (FD_ISSET(fd, exceptfds)) fds[i].revents |= POLLPRI; this causes an unsolicited event to be handled it's better to check whether this event was requested or not if (FD_ISSET(fd, readfds)) if (fds[i].events & POLLIN) fds[i].revents |= POLLIN; if (FD_ISSET(fd, writefds)) if (fds[i].events & POLLOUT) fds[i].revents |= POLLOUT; if (FD_ISSET(fd, exceptfds)) if (fds[i].events & POLLPRI) fds[i].revents |= POLLPRI;