Just put 1+1 together and realised there's an apache bugzilla.
Apologies for the spam.
Have raised as bz#37586
G
Gerry <[EMAIL PROTECTED]> wrote:
> Evening Gents,
>
> Spotted a real oddity in apr_pollset_poll in both select.c and poll.c in the
> current version 1.2.2 and in the versions used in apache2 20.54 and 2.1.9.
>
> Forgive me but I haven't check the rest as I've got stuff due today into
> test.
>
> I'll take the SELECT based apr_pollset_poll as an example to illustrate...
>
> the apr_int32_t *num return value is given as the result from the select,
> which
> should be the number of signalled FD's.
>
> rv = select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> tvptr);
>
> (*num) = rv;
>
> This is wrong. The reason it's wrong is because you then itterate over all
> of
> the file descriptors in the original query_set to find which exist in either
> the returned readset, writeset, or exceptset.
>
> if (FD_ISSET(fd, &readset) || FD_ISSET(fd, &writeset) ||
> FD_ISSET(fd, &exceptset)) {
>
> Only when the fd matches one in either set is the result_set updated
> accordingly, and the integer 'j' used to track the number of results.
>
> Hence, in some cases (affecting me Mandrake Linux 10.0 3.3.2-6mdk) the 'num'
> out
> value is greater than the number of items copied into the result set.
>
> Now this would escape the notice of most folks unless, like me, your writing
> a
> socks proxy for Set Top Boxes which has a continually growing and shrinking
> list of connections and hence a very dynamic pollset...
>
> RESULT: walking into an invalid entry in the array and core dumping.
>
>
> The Fix is, at the end of the loop when running over the file descriptors,
> you
> should set (*num)=j as shown below.
>
> Let me re-itterate that this pattern is uesed in most/all of the other
> apr_pollset_poll implementation, goes back to APR 0.94/Apache 2.054.
>
> I would (really) love to fix, test, and upload the patches but I haven't the
> time. Sorry gets.
>
>
> Kind Regards,
> Gerry Calderhead
>
>
>
> APR_DECLARE(apr_status_t) apr_pollset_poll(apr_pollset_t *pollset,
> apr_interval_time_t timeout,
> apr_int32_t *num,
> const apr_pollfd_t **descriptors)
> {
> int rv;
> apr_uint32_t i, j;
> struct timeval tv, *tvptr;
> fd_set readset, writeset, exceptset;
>
> if (timeout < 0) {
> tvptr = NULL;
> }
> else {
> tv.tv_sec = (long) apr_time_sec(timeout);
> tv.tv_usec = (long) apr_time_usec(timeout);
> tvptr = &tv;
> }
>
> memcpy(&readset, &(pollset->readset), sizeof(fd_set));
> memcpy(&writeset, &(pollset->writeset), sizeof(fd_set));
> memcpy(&exceptset, &(pollset->exceptset), sizeof(fd_set));
>
> #ifdef NETWARE
> if (HAS_PIPES(pollset->set_type)) {
> rv = pipe_select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> tvptr);
> }
> else
> #endif
> rv = select(pollset->maxfd + 1, &readset, &writeset, &exceptset,
> tvptr);
>
> (*num) = rv;
> if (rv < 0) {
> return apr_get_netos_error();
> }
> if (rv == 0) {
> return APR_TIMEUP;
> }
> j = 0;
> for (i = 0; i < pollset->nelts; i++) {
> apr_os_sock_t fd;
> if (pollset->query_set[i].desc_type == APR_POLL_SOCKET) {
> fd = pollset->query_set[i].desc.s->socketdes;
> }
> else {
> #if !APR_FILES_AS_SOCKETS
> return APR_EBADF;
> #else
> fd = pollset->query_set[i].desc.f->filedes;
> #endif
> }
> if (FD_ISSET(fd, &readset) || FD_ISSET(fd, &writeset) ||
> FD_ISSET(fd, &exceptset)) {
> pollset->result_set[j] = pollset->query_set[i];
> pollset->result_set[j].rtnevents = 0;
> if (FD_ISSET(fd, &readset)) {
> pollset->result_set[j].rtnevents |= APR_POLLIN;
> }
> if (FD_ISSET(fd, &writeset)) {
> pollset->result_set[j].rtnevents |= APR_POLLOUT;
> }
> if (FD_ISSET(fd, &exceptset)) {
> pollset->result_set[j].rtnevents |= APR_POLLERR;
> }
> j++;
> }
> }
>
> (*num) = j; /* <<<<<<<<< ADDED BY GERR >>>>>>>>>>>>>>>>>>>>> */
>
> if (descriptors)
> *descriptors = pollset->result_set;
> return APR_SUCCESS;
> }
>
>
>
>
>
>
>
>
>
>
>
>
>
>
