tags 747989 + patch
thanks
Hi,
I'm one of the Debian GNUstep maintainers. Emilio Pozuelo Monfort
from the Release Team pointed me at this bug.
The conclusions at the upstream bug are mostly correct -- Object in
the new runtime is barebone and you either have to implement all the
basic methods with the new libobjc functions, or you have to use
NSObject as your root object (thus depending on GNUstep Base or Cocoa
on MuckOS X). It is possible to do the former, but that is too much
work for no apparent benefit. You could take a look at
gnustep-base/Source/NSObject.m for the complexity involved. There is
too much room for grave errors, even for the basic things. And the
code would be even more cluttered with #ifdefs.
The patch there attempts to do the latter, but it is not completely
correct and doesn't work in this form:
- It overrides NSObject's -init, -retain, -release and -isEqual:.
What you really want is TRObject to become a dummy class fully
inheriting from NSObject.
- The autorelease pool should be created in the constructor, before
the first object is allocated. That's openvpn_plugin_open_v1,
as far as I understand the code (but I may be wrong here). If
there are additional threads created, you need separate pools for
every thread. AFAICS this is not the case here, right?
- The pool has to be destroyed in the destructor, after releasing
the last object. This should be openvpn_plugin_close_v1, but
again, I'm not familiar with the API and do not understand what's
the relationship between this function and openvpn_plugin_func_v1
and when is openvpn_plugin_close_v1 called exactly.
- The pool is destroyed by sending a -release message. -drain may
work, but not in all implementations.
- With the "modern" runtime, you have two cases only -- Apple and
non-Apple, because '-framework Foundation' will work only on
Apple, and you need GNUstep for all other platforms, not only
GNU/Linux. That's necessary on Debian too, because we have
GNU/Hurd and GNU/kFreeBSD.
- The OBJCFLAGS and linker flags are specific to the GNUstep
installation, and may differ drastically. For example, with
GNUSTEP_INSTALLATION_DOMAIN everything (headers, libraries, tools,
etc.) is installed in ~/GNUstep. They also differ depending on
the compiler/runtime (for instance, the new GNUstep runtime +
clang) so you'd better resort to gnustep-config for these. This
will also ensure that they're always correct and you don't have to
adjust your package all the time.
With the attached modified version of the upstream patch the plugin
crashes when I run openvpn as advised at
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=641811#72
(gdb) bt
#0 __strlen_ia32 () at
../sysdeps/i386/i686/multiarch/../../i586/strlen.S:94
#1 0xb7b4ccdf in -[LFString initWithCString:] (self=0x80115180,
_cmd=0xb7b5e798 <_OBJC_SELECTOR_TABLE+88>, cString=0x0) at
LFString.m:57
#2 0xb7b4fa08 in -[LFAuthLDAPConfig initWithConfigFile:] (
self=0xb7b5e748 <_OBJC_SELECTOR_TABLE+8>,
_cmd=0xb7b5c1f8 <_OBJC_SELECTOR_TABLE+120>, fileName=0x0)
at LFAuthLDAPConfig.m:321
#3 0xb7b49e08 in openvpn_plugin_open_v1 (type=0x800c39bc,
argv=0x800c38e4,
envp=0x800da19c) at auth-ldap.m:262
That's a general problem with the code -- the check for argv[1] should
be done earlier so that -initWithConfigFile is not called with a NULL
argument and as a result strlen is not called with a NULL argument.
Or the method could first check if the argument is supplied and return
nil immediately if that is not the case.
Running it with the example configuration file:
(gdb) r --dev null --plugin /usr/lib/openvpn/openvpn-auth-ldap.so
/usr/share/doc/openvpn-auth-ldap/examples/auth-ldap.conf
Starting program: /usr/sbin/openvpn --dev null --plugin
/usr/lib/openvpn/openvpn-auth-ldap.so
/usr/share/doc/openvpn-auth-ldap/examples/auth-ldap.conf
[Thread debugging using libthread_db enabled]
Using host libthread_db library
"/lib/i386-linux-gnu/i686/cmov/libthread_db.so.1".
Fri Jul 4 00:26:56 2014 OpenVPN 2.3.2 i486-pc-linux-gnu [SSL (OpenSSL)] [LZO]
[EPOLL] [PKCS11] [eurephia] [MH] [IPv6] built on Mar 17 2014
Fri Jul 4 00:26:57 2014 ******* WARNING *******: all encryption and
authentication features disabled -- all data will be tunnelled as cleartext
Fri Jul 4 00:26:57 2014 UDPv4 link local (bound): [undef]
Fri Jul 4 00:26:57 2014 UDPv4 link remote: [undef]
And there it stays continuously, I have to interrupt it with C-c.
Please test and let me know if there are problems/questions. Thanks.
--- openvpn-auth-ldap-2.0.3.orig/aclocal.m4
+++ openvpn-auth-ldap-2.0.3/aclocal.m4
@@ -1,4 +1,3 @@
-builtin(include,objc.m4)
builtin(include,pthread.m4)
builtin(include,platform.m4)
builtin(include,check.m4)
@@ -23,7 +22,7 @@
# Result is cached.
#
# Defines one of the following preprocessor macros:
-# APPLE_RUNTIME GNU_RUNTIME
+# APPLE_RUNTIME GNU_RUNTIME MODERN_RUNTIME
#
# Substitutes the following variables:
# OBJC_RUNTIME OBJC_RUNTIME_FLAGS OBJC_LIBS
@@ -31,7 +30,7 @@
#------------------------------------------------------------------------
AC_DEFUN([OD_OBJC_RUNTIME],[
AC_REQUIRE([AC_PROG_OBJC])
- AC_ARG_WITH(objc-runtime, AC_HELP_STRING([--with-objc-runtime], [Specify either "GNU" or "apple"]), [with_objc_runtime=${withval}])
+ AC_ARG_WITH(objc-runtime, AC_HELP_STRING([--with-objc-runtime], [Specify either "GNU", "apple", or "modern"]), [with_objc_runtime=${withval}])
if test x"${with_objc_runtime}" != x; then
case "${with_objc_runtime}" in
@@ -39,8 +38,10 @@
;;
apple)
;;
+ modern)
+ ;;
*)
- AC_MSG_ERROR([${with_objc_runtime} is not a valid argument to --with-objc-runtime. Please specify either "GNU" or "apple"])
+ AC_MSG_ERROR([${with_objc_runtime} is not a valid argument to --with-objc-runtime. Please specify either "GNU", "apple", or "modern"])
;;
esac
fi
@@ -183,6 +184,33 @@
od_cv_objc_runtime_gnu="no"
fi
+ if test x"${with_objc_runtime}" = x || test x"${with_objc_runtime}" = x"modern"; then
+ AC_MSG_CHECKING([for Modern Objective C runtime])
+ AC_CACHE_VAL(od_cv_objc_runtime_modern, [
+ # The following uses quadrigraphs
+ # '@<:@' = '['
+ # '@:>@' = ']'
+ AC_LINK_IFELSE([
+ AC_LANG_PROGRAM([
+ #include <objc/objc.h>
+ #include <objc/runtime.h>
+ ], [
+ id class = objc_lookUpClass("NSObject");
+ id obj = @<:@class alloc@:>@;
+ puts(@<:@obj name@:>@);
+ ])
+ ], [
+ od_cv_objc_runtime_modern="yes"
+ ], [
+ od_cv_objc_runtime_modern="no"
+ ]
+ )
+ ])
+ AC_MSG_RESULT(${od_cv_objc_runtime_modern})
+ else
+ od_cv_objc_runtime_modern="no"
+ fi
+
# Apple runtime is prefered
if test x"${od_cv_objc_runtime_apple}" = x"yes"; then
OBJC_RUNTIME="APPLE_RUNTIME"
@@ -194,6 +222,23 @@
OBJC_RUNTIME_FLAGS="-fgnu-runtime"
AC_MSG_NOTICE([Using GNU Objective-C runtime])
AC_DEFINE([GNU_RUNTIME], 1, [Define if using the GNU Objective-C runtime and compiler.])
+ elif test x"${od_cv_objc_runtime_modern}" = x"yes"; then
+ OBJC_RUNTIME="MODERN_RUNTIME"
+ case "${target_os}" in
+ darwin*) OBJC_RUNTIME_FLAGS="-fnext-runtime"
+ LDFLAGS="-framework Foundation ${LDFLAGS}";;
+ *) AC_CHECK_PROG([have_gs_config],
+ [gnustep-config], [yes])
+ if test x"$have_gs_config" != x"yes"; then
+ AC_MSG_ERROR([Cannot find gnustep-config.])
+ else
+ OBJC_RUNTIME_FLAGS="`gnustep-config --objc-flags`"
+ OBJC_LIBS="`gnustep-config --base-libs`"
+ fi
+ ;;
+ esac
+ AC_MSG_NOTICE([Using Modern Objective-C runtime])
+ AC_DEFINE([MODERN_RUNTIME], 1, [Define if using the Modern Objective-C runtime and compiler.])
else
AC_MSG_FAILURE([Could not locate a working Objective-C runtime.])
fi
--- openvpn-auth-ldap-2.0.3.orig/src/TRObject.h
+++ openvpn-auth-ldap-2.0.3/src/TRObject.h
@@ -40,20 +40,29 @@
#endif
#include <stdbool.h>
+#ifdef MODERN_RUNTIME
+#import <Foundation/NSObject.h>
+#else
#include <objc/Object.h>
+#endif
#include "auth-ldap.h"
@protocol TRObject
+#ifndef MODERN_RUNTIME
/* Reference counting */
- (id) retain;
- (void) release;
/* Equality */
- (BOOL) isEqual: (id) anObject;
+#endif
@end
+#ifdef MODERN_RUNTIME
+@interface TRObject : NSObject <TRObject>
+#else
@interface TRObject : Object <TRObject> {
unsigned int _refCount;
}
@@ -66,6 +75,7 @@
- (void) dealloc;
+#endif
@end
#endif /* TROBJECT_H */
--- openvpn-auth-ldap-2.0.3.orig/src/TRObject.m
+++ openvpn-auth-ldap-2.0.3/src/TRObject.m
@@ -53,6 +53,7 @@
* Additionally, we implement brain-dead, non-thread-safe
* reference counting.
*/
+#ifndef MODERN_RUNTIME
@interface Object (AppleAddedAReallyStupidGCCWarning)
- (void) dealloc;
@end
@@ -102,4 +103,7 @@
[self dealloc];
}
+#else
+@implementation TRObject
+#endif
@end
--- openvpn-auth-ldap-2.0.3.orig/src/auth-ldap.m
+++ openvpn-auth-ldap-2.0.3/src/auth-ldap.m
@@ -48,6 +48,11 @@
#include <TRPacketFilter.h>
#include <TRPFAddress.h>
#include <TRLog.h>
+#ifdef MODERN_RUNTIME
+#import <Foundation/NSAutoreleasePool.h>
+
+static NSAutoreleasePool *pool;
+#endif
/* Plugin Context */
typedef struct ldap_ctx {
@@ -166,7 +171,7 @@
const char userFormat[] = "%u";
/* Copy the template */
- templateString = [[LFString alloc] initWithString: template];
+ templateString = [(LFString *)[LFString alloc] initWithString: template];
/* Initialize the result */
result = [[LFString alloc] init];
@@ -250,6 +255,9 @@
OPENVPN_EXPORT openvpn_plugin_handle_t
openvpn_plugin_open_v1(unsigned int *type, const char *argv[], const char *envp[]) {
+#ifdef MODERN_RUNTIME
+ pool = [[NSAutoreleasePool alloc] init];
+#endif
ldap_ctx *ctx = xmalloc(sizeof(ldap_ctx));
ctx->config = [[LFAuthLDAPConfig alloc] initWithConfigFile: argv[1]];
if (!ctx->config) {
@@ -606,5 +614,8 @@
[ldapUser release];
if (ldap)
[ldap release];
+#ifdef MODERN_RUNTIME
+ [pool release];
+#endif
return (ret);
}
--- openvpn-auth-ldap-2.0.3.orig/tests/Makefile.in
+++ openvpn-auth-ldap-2.0.3/tests/Makefile.in
@@ -26,7 +26,7 @@
CFLAGS+= @CHECK_CFLAGS@ -DTEST_DATA=\"${srcdir}/data\"
OBJCFLAGS+= @CHECK_CFLAGS@ -DTEST_DATA=\"${srcdir}/data\"
LIBS+= -lauth-ldap-testing $(OBJC_LIBS) $(LDAP_LIBS) @CHECK_LIBS@
-LDFLAGS+= -L${top_builddir}src $(LIBS)
+LDFLAGS+= -L${top_builddir}/src $(LIBS)
# Recompile the tests every time
all: tests
--- openvpn-auth-ldap-2.0.3.orig/src/Makefile.in
+++ openvpn-auth-ldap-2.0.3/src/Makefile.in
@@ -64,7 +64,8 @@
$(INSTALL_PLUGIN)
clean::
- rm -f $(TEST_OBJS) $(PLUGIN_OBJS) $(TEST_LIB) $(GEN_SRCS) testplugin
+ rm -f $(TEST_OBJS) $(PLUGIN_OBJS) $(PLUGIN_OBJS:.o=.d) $(TEST_LIB) \
+ $(GEN_SRCS) testplugin
$(CLEAN_PLUGIN)
distclean:: clean
