All,

Michael Deiner found a buffer overflow in the call to FD_SET macro on
line 291 of jk_connect.c:

280>   do {
281>        rc = connect(sd, (const struct sockaddr *)&addr->sa.sin,
addr->salen);
282>    } while (rc == -1 && errno == EINTR);
283>
284>    if ((rc == -1) && (errno == EINPROGRESS || errno == EALREADY)
285>                   && (timeout > 0)) {
286>        fd_set wfdset;
287>        struct timeval tv;
288>        socklen_t rclen = (socklen_t)sizeof(rc);
289>
290>        FD_ZERO(&wfdset);
*291>        FD_SET(sd, &wfdset);*
292>        tv.tv_sec = timeout / 1000;
293>        tv.tv_usec = (timeout % 1000) * 1000;
294>        rc = select(sd + 1, NULL, &wfdset, NULL, &tv);

I'd like to fix this so it won't bring-down the server :)

But it quickly gets complicated.

The method itself takes "sd" (a jk_sock_t) as an argument to the
function, and we can check immediately whether it will cause FD_SET to
overflow -- easy: just check to see if the value is too large -- but
what should we do in that case?

This function should be connecting to a back-end Tomcat server, but if
we have too many outbound connections, we'll fail.

I'm not sure it makes any sense to let things get this far.

The proposed solution[1] is to use poll() instead of select(), but that
won't work on every platform, plus I'd like to be able to fix the buffer
overflow right away while we work-out a solution for poll() that will
work in most environments.

I think if the connection_pool_size exceeds FD_SETSIZE we should refuse
to start. Any other behavior will eventually cause errors.

Thoughts?

... and if we want to refuse to start... where is the best place to do
that and how?

-chris

[1]
https://lists.apache.org/thread.html/839da944652d028431bebbf74ce0635d02666868944de49c05d81057@%3Cusers.tomcat.apache.org%3E

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to