You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2014/04/24 10:54:16 UTC

[Bug 56452] New: IPv6 address and log level debug caused crash

https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

            Bug ID: 56452
           Summary: IPv6 address and log level debug caused crash
           Product: Tomcat Connectors
           Version: 1.2.40
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_jk
          Assignee: dev@tomcat.apache.org
          Reporter: shimizuhiroto123@gmail.com

When the following environment set, crash occured.
* mod_jk 1.2.39 or latter
* IPv6
* JkLogLevel debug

I think char buf[64] is too small.

---mod_jk.log
[Tue Apr 22 12:30:44.350 2014] [39832:140226284476384] [debug]
jk_open_socket::jk_connect.c (766):
 socket 14 [2001:c0a8:1424:0:a00:1f49:::59892 ->
2001:c0a8:6422:0:3a35:3938:3932:0:8009] connected
---

---
$ gdb /usr/sbin/httpd core.39832
:
(gdb) thread apply all backtrace
:
#5  0x00007f88ef48a587 in jk_open_socket (addr=<value optimized out>,
keepalive=0, timeout=5, connect_timeout=5000,
    sock_buf=<value optimized out>, l=0x7f88fbe832b8) at jk_connect.c:771
#6  0x00007f88ef4a9055 in ajp_connect_to_endpoint (ae=0x7f88fbedb4a0,
l=0x7f88fbe832b8) at jk_ajp_common.c:1011
#7  0x00007f88ef4a94e9 in ajp_send_request (e=<value optimized out>,
s=0x7fff4a6122d0, l=0x7f88fbe832b8,
    ae=0x7f88fbedb4a0, op=0x7fff4a611090) at jk_ajp_common.c:1662
:
(gdb) f 5
#5  0x00007f88ef48a587 in jk_open_socket (addr=<value optimized out>,
keepalive=0, timeout=5, connect_timeout=5000,
    sock_buf=<value optimized out>, l=0x7f88fbe832b8) at jk_connect.c:771
warning: Source file is more recent than executable.
771    }
(gdb) l
766                jk_log(l, JK_LOG_DEBUG, "socket %d [%s] connected",
767                       sd, jk_dump_sinfo(sd, buf));
768        }
:
---


---jk_connect.c
jk_sock_t jk_open_socket(jk_sockaddr_t *addr, int keepalive,
                         int timeout, int connect_timeout,
                         int sock_buf, jk_logger_t *l)
{
    char buf[64];

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #7 from Rainer Jung <ra...@kippdata.de> ---
Chris: any chance you've got the updated patch lying around somewhere? Or can
do it during the next few days?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #1 from Christopher Schultz <ch...@christopherschultz.net> ---
Agreed.

The use of 'buf' when passed-into inet_ntop4|6 inside jk_dump_sinfo is also not
sane: the final argument should be the length of the buffer available. It is
blindly set to 16 (for IPv4) or 64 (for IPv6) instead of sending the real value
which will be (64 - ps).

Unfortunately, jk_dump_sinfo has no way of knowing the size of the buffer being
passed to it. That could easily be fixed, as jk_dump_sinfo is only used
internally to jk_connect.c (I don't know if anyone bothers using mod_jk as a
library for anything else).

I'm not increasing the severity of this bug to MAJOR because while it's a
buffer-overrun crash, it only happens when in DEBUG mode.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #6 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Konstantin Kolinko from comment #5)
> First, the above code has an error. The IPv4 address part can be longer by 4
> decimal digits than the one written above.

Aah, yes. Thanks for pointing that out.

> Second, I do not care on the actual implementation. I mentioned APR code as
> an example of their output.
> 
> The stackoverflow article mentioned INET6_ADDRSTRLEN constant, but I do not
> know whether that is portable.

On Linux, I can see them here:
$ grep 'INET\(6\)\?_ADDRSTRLEN' `find /usr/include -name "*.h"`
/usr/include/netinet/in.h:#define INET_ADDRSTRLEN 16
/usr/include/netinet/in.h:#define INET6_ADDRSTRLEN 46

Mac OS X has them in comparable locations:
$  grep 'INET\(6\)\?_ADDRSTRLEN' `find /usr/include -name "*.h"`
/usr/include/netinet/in.h:#define INET_ADDRSTRLEN                 16
/usr/include/netinet6/in6.h:#define    INET6_ADDRSTRLEN    46

The Linux mac page for inet_ntop mentions both INET_ADDRSTRLEN and
INET6_ADDRSTRLEN, and the "COMFORMING TO" section says POSIX.1-2001 with no
caveats about those constants. If we like POSIX.1-2001 then I think we're all
set.

I like the use of INET_ADDRSTRLEN better than the sizeof thing for some reason,
so I'll use that. Anyone with a problem can file a bug; it's easy to fix if
someone has an old system. I'll update the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

Rainer Jung <ra...@kippdata.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Rainer Jung <ra...@kippdata.de> ---
I committed a very similar patch in r1648948.

It would be very nice if you could check it, especially whether I got the math
for the buffer size correct.

Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #5 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Christopher Schultz from comment #4)
> That's only 97 bytes plus a NULL-terminator. Where did you get the 39 from?

39 is the size of IPv6 address in the form used in your example. You have two
IPv6 addresses in that string.

45-39 = 6 characters missing  x 2 IPv6 addresses = 12 more characters are
needed in the buffer.

> we
> could do something like this if you'd prefer:
> 
>   char buf[sizeof "0000:0000:0000:0000:0000:0000:192.168.0.1:65535 ->
> 0000:0000:0000:0000:0000:0000:192.168.0.1:65535" + 1];

First, the above code has an error. The IPv4 address part can be longer by 4
decimal digits than the one written above.

Second, I do not care on the actual implementation. I mentioned APR code as an
example of their output.

The stackoverflow article mentioned INET6_ADDRSTRLEN constant, but I do not
know whether that is portable.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #4 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Konstantin Kolinko from comment #3)
> (In reply to Christopher Schultz from comment #2)
> > Created attachment 31557 [details]
> > Proposed patch (against mod_jk/trunk)
> > 
> > I have raised the size of buf from 64 to 100 characters. The message looks
> > like it will need 96 bytes for a complete IPv6 message (e.g.
> > 2001:0db8:0000:0000:0000:ff00:0042:8329:65556 ->
> > 2001:0db8:0000:0000:0000:ff00:0042:8329:65535 + \0), so I just rounded up to
> > 100 bytes.
> > 
> > I've also added proper length-tracking to the buffer and more protection
> > against overruns.
> 
> The following discussion says that IPv6 address needs maximum 45 characters,
> not 39. So you would need 12 bytes more than 96 = 108.

Bah, I forgot about tunnelled-IPv4. However:

"0000:0000:0000:0000:0000:0000:192.168.0.1:65535 ->
0000:0000:0000:0000:0000:0000:192.168.0.1:65535"

That's only 97 bytes plus a NULL-terminator. Where did you get the 39 from?

> http://stackoverflow.com/questions/166132/maximum-length-of-the-textual-
> representation-of-an-ipv6-address
> 
> Also see tmp buffer size here:
> http://svn.apache.org/viewvc/apr/apr/trunk/network_io/unix/inet_ntop.
> c?revision=573491&view=markup#l149

I did see that code in APR while researching. I suppose I could use that, but I
figured that "buf" might be used for other things so I didn't consider it. I
think it turns out that "buf" is really just for this purpose, so we could do
something like this if you'd prefer:

  char buf[sizeof "0000:0000:0000:0000:0000:0000:192.168.0.1:65535 ->
0000:0000:0000:0000:0000:0000:192.168.0.1:65535" + 1];

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Christopher Schultz from comment #2)
> Created attachment 31557 [details]
> Proposed patch (against mod_jk/trunk)
> 
> I have raised the size of buf from 64 to 100 characters. The message looks
> like it will need 96 bytes for a complete IPv6 message (e.g.
> 2001:0db8:0000:0000:0000:ff00:0042:8329:65556 ->
> 2001:0db8:0000:0000:0000:ff00:0042:8329:65535 + \0), so I just rounded up to
> 100 bytes.
> 
> I've also added proper length-tracking to the buffer and more protection
> against overruns.

The following discussion says that IPv6 address needs maximum 45 characters,
not 39. So you would need 12 bytes more than 96 = 108.

http://stackoverflow.com/questions/166132/maximum-length-of-the-textual-representation-of-an-ipv6-address

Also see tmp buffer size here:
http://svn.apache.org/viewvc/apr/apr/trunk/network_io/unix/inet_ntop.c?revision=573491&view=markup#l149

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 56452] IPv6 address and log level debug caused crash

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=56452

--- Comment #2 from Christopher Schultz <ch...@christopherschultz.net> ---
Created attachment 31557
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=31557&action=edit
Proposed patch (against mod_jk/trunk)

I have raised the size of buf from 64 to 100 characters. The message looks like
it will need 96 bytes for a complete IPv6 message (e.g.
2001:0db8:0000:0000:0000:ff00:0042:8329:65556 ->
2001:0db8:0000:0000:0000:ff00:0042:8329:65535 + \0), so I just rounded up to
100 bytes.

I've also added proper length-tracking to the buffer and more protection
against overruns.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org