Opened 7 years ago

Closed 6 years ago

#41077 closed defect (fixed)

mod_jk fails with "source and destination buffer overlap" on Maverick

Reported by: girgen@… Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.2.1
Keywords: mavericks haspatch Cc: todmorrison (Tod Morrison), cooljeanius (Eric Gallager)
Port: mod_jk

Description

Apple appears to have made strcpy() enforce that the src and dest buffers may not overlap.

This makes mod_jk fail. There is a patch upstreams alread. https://issues.apache.org/bugzilla/show_bug.cgi?id=55696

This patch to the macport just uses the referred patch. Also, set JAVA_HOME using /usr/libexec/java_home for convenience.

--- Portfile.orig	2013-10-30 12:40:54.000000000 +0100
+++ Portfile	2013-10-30 12:43:02.000000000 +0100
@@ -4,6 +4,7 @@
 
 name				mod_jk
 version				1.2.27
+revision			1
 
 categories			www java
 license				Apache-2 BSD
@@ -39,7 +40,7 @@
 if { [llength [array get env "JAVA_HOME"]] > 0 } {
 	set javahome $env(JAVA_HOME)
 } else {
-	set javahome ""
+	set javahome [exec /usr/libexec/java_home]
 }
 if { ![file isdirectory ${javahome}] } {
 	if { ${os.platform} == "darwin" } {
@@ -72,6 +73,7 @@
 	}
 }
 
+patchfiles-append jk_map.c.patch
 if {[variant_isset universal]} {
     patchfiles-append   configure_universal.patch
     post-configure {
--- /dev/null	2013-10-30 12:43:06.000000000 +0100
+++ files/jk_map.c.patch	2013-10-30 12:42:02.000000000 +0100
@@ -0,0 +1,62 @@
+Index: common/jk_map.c
+===================================================================
+--- common/jk_map.c	(revision 1535519)
++++ common/jk_map.c	(working copy)
+@@ -183,33 +183,37 @@
+ 
+ int jk_map_get_int(jk_map_t *m, const char *name, int def)
+ {
+-    char buf[100];
+     const char *rc;
+-    size_t len;
+     int int_res;
+-    int multit = 1;
+ 
+-    sprintf(buf, "%d", def);
+-    rc = jk_map_get_string(m, name, buf);
++    rc = jk_map_get_string(m, name, NULL);
+ 
+-    len = strlen(rc);
+-    if (len) {
+-        char *lastchar = &buf[0] + len - 1;
+-        strcpy(buf, rc);
+-        if ('m' == *lastchar || 'M' == *lastchar) {
+-            *lastchar = '\0';
+-            multit = 1024 * 1024;
++    if(NULL == rc) {
++        int_res = def;
++    } else {
++        size_t len = strlen(rc);
++        int multit = 1;
++
++        if (len) {
++            char buf[100];
++            char *lastchar;
++            strncpy(buf, rc, 100);
++	    lastchar = buf + len - 1;
++            if ('m' == *lastchar || 'M' == *lastchar) {
++                *lastchar = '\0';
++                multit = 1024 * 1024;
++            }
++            else if ('k' == *lastchar || 'K' == *lastchar) {
++                *lastchar = '\0';
++                multit = 1024;
++            }
++            int_res = multit * atoi(buf);
+         }
+-        else if ('k' == *lastchar || 'K' == *lastchar) {
+-            *lastchar = '\0';
+-            multit = 1024;
+-        }
+-        int_res = atoi(buf);
++        else
++            int_res = def;
+     }
+-    else
+-        int_res = def;
+ 
+-    return int_res * multit;
++    return int_res;
+ }
+ 
+ double jk_map_get_double(jk_map_t *m, const char *name, double def)

Attachments (5)

mod_jk.diff (2.5 KB) - added by girgen@… 7 years ago.
mod_jk.2.diff (2.9 KB) - added by raimue (Rainer Müller) 7 years ago.
mod_jk-Portfile.diff (1.4 KB) - added by girgen@… 7 years ago.
mod_jk.3.diff (2.6 KB) - added by girgen@… 7 years ago.
Update mod_jk to 1.2.40
main.log (58.0 KB) - added by mf2k (Frank Schima) 7 years ago.

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by girgen@…

Attachment: mod_jk.diff added

comment:1 Changed 7 years ago by ryandesign (Ryan Schmidt)

Keywords: mavericks haspatch added
Priority: HighNormal

comment:2 Changed 7 years ago by robertoschwald@…

Why was the priority changed to normal? This is a full stopper for all apache2 with mod_jk installations. apache2 crashes with the mentioned error after upgrade outdated..

comment:3 Changed 7 years ago by robertoschwald@…

Patch works here. Thanks for that.

comment:4 Changed 7 years ago by robertoschwald@…

I just saw that there is no maintainer for the port. I would like to take over maintenance so we have a fix asap.

comment:5 in reply to:  2 Changed 7 years ago by larryv (Lawrence Velázquez)

Replying to robertoschwald@…:

Why was the priority changed to normal? This is a full stopper for all apache2 with mod_jk installations. apache2 crashes with the mentioned error after upgrade outdated..

We generally reserve High priority for security issues.

comment:6 Changed 7 years ago by raimue (Rainer Müller)

The statement

set javahome [exec /usr/libexec/java_home]

looks wrong to me. What if there is no Java SDK installed on the system? Won't this lead to an error here? Could it be a shim that pops up the automatic installation dialog? Or is it guaranteed that his binary always exists on all versions of Mac OS X? Not sure what to make of this.

Furthermore, the whole detection seems to be required for the +jni variant, which probably links against the Java runtime specified with ${javahome}. But what happens if I change the system default Java runtime? I am aware this issue existed before this change, just writing down what I see as I am looking at the port.

Upstream raised some concerns about the use of strncpy(3) in the patch. I agree with them and would recommend to use strlcpy(3), which would most probably not be portable to Linux/glibc, but works fine on various *BSD versions and OS X. Updated patch attached.

Changed 7 years ago by raimue (Rainer Müller)

Attachment: mod_jk.2.diff added

Changed 7 years ago by girgen@…

Attachment: mod_jk-Portfile.diff added

comment:7 Changed 7 years ago by girgen@…

The strlcpy patch looks great to me, absolutely an improvement.

java_home is only required by +jni, so the line javahome check should only be run in case of variant +jni. Patch applied.

comment:8 in reply to:  6 Changed 7 years ago by larryv (Lawrence Velázquez)

Replying to raimue@…:

The statement

set javahome [exec /usr/libexec/java_home]

looks wrong to me. What if there is no Java SDK installed on the system? Won't this lead to an error here? Could it be a shim that pops up the automatic installation dialog?

In that case, this command will just print some default path, which is probably not the behavior we want. To error out if no JDK is installed, we should use:

/usr/libexec/java_home --failfast

comment:9 Changed 7 years ago by todmorrison (Tod Morrison)

Cc: todmorrison@… added

Cc Me!

comment:10 Changed 7 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:11 Changed 7 years ago by robertoschwald@…

Upstream fixed the bug: https://issues.apache.org/bugzilla/show_bug.cgi?id=55696

Fixed with mod_jk 1.2.40 which will be released, soon.

Changed 7 years ago by girgen@…

Attachment: mod_jk.3.diff added

Update mod_jk to 1.2.40

comment:12 in reply to:  11 Changed 7 years ago by girgen@…

Replying to robertoschwald@…:

Upstream fixed the bug: https://issues.apache.org/bugzilla/show_bug.cgi?id=55696

Fixed with mod_jk 1.2.40 which will be released, soon.

OK, 1.2.40 is released. I've attached a patch [mod_jk.3.diff] that updates the port. JNI stuff is removed from mod_jk since 1.2.29 [ish], so I removed the stuff about JAVA_HOME completely. There is no dependency on java anymore, only tomcat requires that.

Also, since the port is orphan, I add myself as maintainer. It would be the first macport I'd maintain, my experience in this field lies mainly with FreeBSD ports, but since we use Mac for development, I have a personal and professional interest in the mod_jk port being up-to-date on mac...

comment:13 Changed 7 years ago by robertoschwald@…

Any chance a committer releases a new version, soon? This bug is open since 8 months and is still a full-stopper. Thanks.

comment:14 Changed 7 years ago by mf2k (Frank Schima)

This patch does not work for me:

$ sudo port install mod_jk
--->  Computing dependencies for mod_jk
--->  Fetching archive for mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_13.x86_64.tbz2 from http://packages.macports.org/mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_13.x86_64.tbz2 from http://lil.fr.packages.macports.org/mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_13.x86_64.tbz2 from http://nue.de.packages.macports.org/macports/packages/mod_jk
--->  Fetching distfiles for mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://sea.us.distfiles.macports.org/macports/distfiles/mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://distfiles.macports.org/mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://www.apache.org/dist/tomcat/tomcat-connectors/jk
--->  Verifying checksums for mod_jk                                            
--->  Extracting mod_jk
--->  Configuring mod_jk
--->  Building mod_jk
--->  Staging mod_jk into destroot
Error: Failed to destroot mod_jk: xinstall: Cannot stat: /opt/local/var/macports/build/_opt_mports_trunk_dports_www_mod_jk/mod_jk/work/tomcat-connectors-1.2.40-src/native/apache-2.0/mod_jk.so, No such file or directory
Error: See /opt/local/var/macports/logs/_opt_mports_trunk_dports_www_mod_jk/mod_jk/main.log for details.
Error: Follow http://guide.macports.org/#project.tickets to report a bug.
Error: Processing of port mod_jk failed

Changed 7 years ago by mf2k (Frank Schima)

Attachment: main.log added

comment:15 Changed 7 years ago by robertoschwald@…

Patch mod_jk.3.diff​ provided works here flawlessly.

]# sudo port install mod_jk
--->  Computing dependencies for mod_jk
--->  Fetching archive for mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_14.x86_64.tbz2 from http://nue.de.packages.macports.org/macports/packages/mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_14.x86_64.tbz2 from http://lil.fr.packages.macports.org/mod_jk
--->  Attempting to fetch mod_jk-1.2.40_0.darwin_14.x86_64.tbz2 from http://mse.uk.packages.macports.org/sites/packages.macports.org/mod_jk
--->  Fetching distfiles for mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://nue.de.distfiles.macports.org/macports/distfiles/mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://lil.fr.distfiles.macports.org/mod_jk
--->  Attempting to fetch tomcat-connectors-1.2.40-src.tar.gz from http://www.mirrorservice.org/sites/ftp.apache.org/tomcat/tomcat-connectors/jk
--->  Verifying checksums for mod_jk                                                 
--->  Extracting mod_jk
--->  Configuring mod_jk
--->  Building mod_jk
--->  Staging mod_jk into destroot
Warning: violation by /opt/local/apache2
Warning: mod_jk violates the layout of the ports-filesystems!
Warning: Please fix or indicate this misbehavior (if it is intended), it will be an error in future releases!
--->  Installing mod_jk @1.2.40_0
#
# Example file /opt/local/apache2/conf/workers.properties.sample has
# been installed to illustrate use of the jk connector between 
# apache2 and tomcat.
#
# You will want to create a working copy of this file as workers.properties
# and configure the uri mappings within it, or by using directives within httpd.conf.
#
# Be sure to also add the following line to your httpd.conf:
#
#     LoadModule jk_module modules/mod_jk.so
#
--->  Activating mod_jk @1.2.40_0
--->  Cleaning mod_jk
--->  Updating database of binaries
--->  Scanning binaries for linking errors               
--->  No broken files found.                             

jkstatus output:

JK Status Manager for localhost:80

Server Version:	Apache/2.2.29 (Unix) mod_ssl/2.2.29 OpenSSL/1.0.1j DAV/2 mod_jk/1.2.40	   	Server Time:	Mon, 20 Oct 2014 17:03:43 CEST
JK Version:	mod_jk/1.2.40		Unix Seconds:	1413817423

As mod_jk when currently installed still always and for everybody crashes httpd during startup, I will add a note to the dev mailinglist so someone integrates the patch or adds girgen as maintainer.

comment:16 Changed 6 years ago by neverpanic (Clemens Lang)

Committed in r127221. I didn't do a test build to save bandwidth (on mobile unfortunately), but it can't get much more broken than it already is. Let's see what the buildbots say.

I also unified the whitespace to conform to our standard modeline in r127222, removed an unnecessary code block in r127223 and converted the post-install ui_msgs to notes in r127224. Let me know if you object to any of these changes and sorry for the delay.

comment:17 Changed 6 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.