apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dror Shilo" <Dror.Sh...@ericom.com>
Subject RE: pollset_poll broken ?
Date Tue, 22 Nov 2005 07:39:42 GMT
I have also found this bug along time ago but noting was done:

see:

>From "Dror Shilo" <Dror.Shilo@ericom.com> 
Subject RE: bug in poll.c of apr 0.9.4 
Date Mon, 23 Aug 2004 15:26:42 GMT 
I use apr-0.9.4

if you don't have the poll function
this makes apr to implement apr_pollset_poll with a select call ( this how it is done in windows)
apr_status_t  apr_pollset_poll  (apr_pollset_t  *pollset, apr_interval_time_t  timeout, apr_int32_t
*num, const apr_pollfd_t **descriptors)

the num parameter have to return the length of descriptors 

with the Unix poll function this done correct, but the select call returns the total number
of socket handles that are ready and contained in the fd_set  structures
therefore if we have 2 socket that one have an FD_READ and the second have FD_READ and FD_WRITE
it will return 3 , but the lenght of descriptors is only 2 , this is a bug 
there for the fix for this bug is to add this  line at file poll.c line 622  
(*num) = j;

Dror Shilo
Ericom software


 

-----Original Message-----
From: Gerry [mailto:gerry@everythingsucks.co.uk]
Sent: Tuesday, November 22, 2005 12:22 AM
To: dev@apr.apache.org
Subject: BUG: pollset_poll broken ?


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