Bug 59897 - Buffer Overflow in FD_SET in nb_connect (jk_connect.c) leading to apache2 crash
Summary: Buffer Overflow in FD_SET in nb_connect (jk_connect.c) leading to apache2 crash
Alias: None
Product: Tomcat Connectors
Classification: Unclassified
Component: mod_jk (show other bugs)
Version: 1.2.41
Hardware: PC Linux
: P2 normal with 3 votes (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Keywords: PatchAvailable
Depends on:
Reported: 2016-07-25 12:33 UTC by Michael Diener
Modified: 2018-08-22 12:13 UTC (History)
3 users (show)

[PATCH] Use poll(2) in posix nb_connect (2.87 KB, patch)
2016-11-03 13:59 UTC, Koen Wilde
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Diener 2016-07-25 12:33:57 UTC
mod_jk occasionally crashes Apache because due to a buffer overflow.

mod_jk 1.2.41 (happens also for 1.2.37)
Apache 2.4.7
Tomcat 6.0.39
Java 1.6.0_45 x86
Linux Ubuntu 14.04 x64 (3.13.0-91-generic)

Here is the error log from Apache:

**** buffer overflow detected ***: /usr/sbin/apache2 terminated=======
======= Memory map: ========
7fe688000000-7fe68806a000 rw-p 00000000 00:00 0
7fe68806a000-7fe68c000000 ---p 00000000 00:00 0
7fffa6c27000-7fffa6c48000 rw-p 00000000 00:00 0 [stack]
7fffa6c86000-7fffa6c88000 r-xp 00000000 00:00 0 [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
[Wed Jun 29 05:01:50.052325 2016] [core:notice] [pid 1747:tid
140641581987712] AH00051: child pid 17018 exit signal Aborted (6), possible
coredump in /etc/apache2

I was able to trace it down to the method nb_connect in jk_connect.c. In version 1.2.41 the issue is line 291:

280>   do {
281>        rc = connect(sd, (const struct sockaddr *)&addr->sa.sin, addr->salen);
282>    } while (rc == -1 && errno == EINTR);
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);
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);

From what I understand a buffer overflow would only happen for FD_SET if
the fd_set gets over 1024 descriptors. I made sure that my ulimit for open
files is set and applied large enough, so that's not it.

I tried to switch FD_SET to poll and it seems to work now also for sd greater than

struct pollfd pfd_read;
pfd_read.fd = sd;
pfd_read.events = POLLOUT;
rc = poll(&pfd_read, 1, timeout);

This would be a possible fix for the problem - at least it works fine in my setup.
Also, poll() already seems to be used somewhere else in this particular source file, so no extra import necessary.

Here more configuration files:


<IfModule jk_module>

        JkWorkersFile /etc/libapache2-mod-jk/workers.properties
        JkLogFile /var/log/apache2/mod_jk.log
        JkLogLevel warn
        JkShmFile /var/log/apache2/jk-runtime-status









        port="xxx" protocol="AJP/1.3" redirectPort="8443"
        enableLookups="false" maxThreads="65536" minSpareThreads="25"
        connectionTimeout="300000" packetSize="65536" request.secret="xxx"

Apache mpm_event:

        StartServers                     2
        ServerLimit          16

        MinSpareThreads          256
        MaxSpareThreads          1280

        ThreadLimit                      1024
        ThreadsPerChild          1024

        MaxRequestWorkers         16384
        MaxConnectionsPerChild   0

Please also see my question about this in the tomcat_users mailing group here (continued in July):
Comment 1 Michael Diener 2016-07-25 12:56:19 UTC
One more thing to add, although Apache mpm_event is used, most connections are via SSL, so AFAIK it should behave like mpm_worker.
Comment 2 Koen Wilde 2016-11-03 13:59:59 UTC
Created attachment 34417 [details]
[PATCH] Use poll(2) in posix nb_connect

This issue is caused by limitations of the select(2) system call. From the (linux) manpage:

> POSIX allows an implementation to define an upper limit, advertised via the
> constant FD_SETSIZE, on the range of file descriptors that can be specified
> in a file descriptor set.  The Linux kernel imposes no fixed limit, but the
> glibc implementation makes fd_set a fixed-size type, with FD_SETSIZE defined
> as 1024, and the FD_*() macros operating according to that limit.  To
> monitor file descriptors greater than 1023, use poll(2) instead.

As Michiel already noted, poll(2) is already imported in jk_connect.c, so using poll(2) doesn't add any new dependencies.

I've attached a patch that uses poll(2) if it is available at compile time; otherwise it falls back to the current select(2) implementation.

On the long run, it would probably be preferable to use some kind of event library like libuv or libevent that abstracts over the kernel interface, and automatically uses the optimal one available (e.g. epoll on linux and kqueue on FreeBSD). This would both improve portability and performance, and possibly code simplicity.
Comment 3 Christopher Schultz 2017-09-01 14:47:31 UTC
I think this patch is worth serious consideration and testing.

(I feel like we had this conversation elsewhere, too.)
Comment 4 Mark Thomas 2018-08-22 12:13:25 UTC
Many thanks for the patch. Applied to 1.2.x for 1.2.44 onwards.