apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gerry <ge...@everythingsucks.co.uk>
Subject BUG: pollset_poll broken ?
Date Mon, 21 Nov 2005 22:21:40 GMT
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;
}















Mime
View raw message