Thanks for diagnosing this. I'm adding a check for this bug into
the APR unit tests now. Once that's ready, I'll commit the fix for
select.c.
Brian
On Nov 21, 2005, at 2:21 PM, Gerry 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;
}
