threerings / openvpn-auth-ldap

Implements username/password authentication via LDAP for OpenVPN 2.x.
Other
135 stars 63 forks source link

Triggers segfault with gcc 4.7 #31

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Compile with gcc 4.7
2. Configure openvpn to use ldap auth
3. segfault on start

openvpn[1220]: segfault at 0 ip b704125f sp bfa9a150 error 4 in 
libobjc.so.4.0.0[b7034000+16000]

What version of the product are you using? On what operating system?

version 2.0.3
Fedora 17 32-bit but seen on others, see:

https://bugzilla.redhat.com/show_bug.cgi?id=870988
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=641811
http://serverfault.com/questions/327294/openvpn-segmentation-fault

Original issue reported on code.google.com by or...@cora.nwra.com on 6 Feb 2013 at 4:22

GoogleCodeExporter commented 9 years ago
Do you have any backtrace information or anything else that would help to 
determine the cause of the issue? I don't have a Linux / gcc 4.7 system on-hand.

Original comment by landon.j.fuller@gmail.com on 6 Feb 2013 at 4:25

GoogleCodeExporter commented 9 years ago
I've attached the full backtrace.

Program terminated with signal 11, Segmentation fault.
#0  __objc_responds_to (sel=<optimized out>, object=0xb716c340)
    at /usr/src/debug/gcc-4.7.2-20120921/libobjc/sendmsg.c:398
398       res = sarray_get_safe (dtable, (size_t) sel->sel_id);
Missing separate debuginfos, use: debuginfo-install 
cyrus-sasl-lib-2.1.23-31.fc17.i686 keyutils-libs-1.5.5-2.fc17.i686 
krb5-libs-1.10.2-6.fc17.i686 libcom_err-1.42.3-3.fc17.i686 
nspr-4.9.4-1.fc17.i686 nss-3.14.1-3.fc17.i686 
nss-softokn-freebl-3.14.1-3.fc17.i686 nss-util-3.14.1-1.fc17.i686 
zlib-1.2.5-7.fc17.i686
(gdb) list
393
394           objc_mutex_unlock (__objc_runtime_mutex);
395         }
396
397       /* Get the method from the dispatch table.  */
398       res = sarray_get_safe (dtable, (size_t) sel->sel_id);
399       return (res != 0) ? YES : NO;
400     }
401
402     BOOL
(gdb) print *dtable
$1 = {buckets = 0xb7f5ec40, empty_bucket = 0xb7f5ea60, version = {version = 3, 
    next_free = 0x3}, ref_count = 1, is_copy_of = 0xb7f5ebe8, capacity = 160}
(gdb) up
#1  __objc_forward (object=0xb716c340, sel=0xb7169ba0, args=0xbf918ee0)
    at /usr/src/debug/gcc-4.7.2-20120921/libobjc/sendmsg.c:905
905       if (__objc_responds_to (object, frwd_sel))
(gdb) print *sel
$2 = {sel_id = 0xd0000, sel_types = 0x0}
(gdb) up 3
#4  0xb77acb32 in plugin_open_item (init_point=2, envp=0xb7f5e944, retlist=0x0, 
p=0xb7f6098c, 
    o=<optimized out>) at plugin.c:302
302             p->plugin_handle = (*p->open1)(&p->plugin_type_mask, o->argv, 
envp);
(gdb) list
297            * Call the plugin initialization
298            */
299           if (p->open2)
300             p->plugin_handle = (*p->open2)(&p->plugin_type_mask, o->argv, 
envp, retlist);
301           else if (p->open1)
302             p->plugin_handle = (*p->open1)(&p->plugin_type_mask, o->argv, 
envp);
303           else
304             ASSERT (0);
305
306           msg (D_PLUGIN, "PLUGIN_INIT: POST %s '%s' intercepted=%s %s",
(gdb) print *p
$3 = {initialized = 1, 
  so_pathname = 0xb7f60924 "/usr/lib/openvpn/plugin/lib/openvpn-auth-ldap.so", 
  plugin_type_mask = 4095, requested_initialization_point = 2, handle = 0xb7f60d90, 
  open1 = 0xb7159f70 <openvpn_plugin_open_v1>, open2 = 0, 
  func1 = 0xb715a590 <openvpn_plugin_func_v1>, func2 = 0, 
  close = 0xb715a020 <openvpn_plugin_close_v1>, abort = 0, client_constructor = 0, 
  client_destructor = 0, min_version_required = 0, initialization_point = 0, 
  plugin_handle = 0x0}

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 4:49

Attachments:

GoogleCodeExporter commented 9 years ago
Here are the build logs for the Fedora package.  There are some warnings there:

http://kojipkgs.fedoraproject.org//packages/openvpn-auth-ldap/2.0.3/9.fc17/data/
logs/i686/build.log

like:

LFAuthLDAPConfig.m: In function '-[LFAuthLDAPConfig initWithConfigFile:]':
LFAuthLDAPConfig.m:315:2: warning: 'TRArray' may not respond to '+alloc' 
[enabled by default]
LFAuthLDAPConfig.m:316:2: warning: 'SectionState' may not respond to '+alloc' 
[enabled by default]
LFAuthLDAPConfig.m:321:2: warning: 'LFString' may not respond to '+alloc' 
[enabled by default]
LFAuthLDAPConfig.m:329:2: warning: 'TRConfig' may not respond to '+alloc' 
[enabled by default]

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 4:55

GoogleCodeExporter commented 9 years ago
What are in the missing frames between #1 and #4? It sounds like the objc 
runtime changed.

Original comment by landon.j.fuller@gmail.com on 6 Feb 2013 at 4:56

GoogleCodeExporter commented 9 years ago
Those warnings are not present in the Fedora 16 build (which works)

http://kojipkgs.fedoraproject.org//packages/openvpn-auth-ldap/2.0.3/8.fc16/data/
logs/i686/build.log

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 4:56

GoogleCodeExporter commented 9 years ago
Just a guess, but perhaps you are basing on Object rather than NSObject?

http://stackoverflow.com/questions/10119971/objective-c-compilation-with-gcc-4-6
-2

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 5:01

GoogleCodeExporter commented 9 years ago
See the attached gdb.txt for the full backtrace

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 5:02

GoogleCodeExporter commented 9 years ago
Yep! Sounds like GNU is following in Apple's footsteps on the bolting of 
libobjc with Foundation. That'll mean we need to gut TRObject and possibly 
TRAutoreleasePool.

Original comment by landon.j.fuller@gmail.com on 6 Feb 2013 at 5:06

GoogleCodeExporter commented 9 years ago
So, I'd like to get this fixed ASAP.  What can I do to help?

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 5:57

GoogleCodeExporter commented 9 years ago
If you have time to look into it, the first order of business would be to look 
into modifying TRObject to inherit from NSObject in the auth-ldap-2.0 branch, 
preferably keyed off of an autoconf test so we don't break older GNU libobjc 
users.

Original comment by landon.j.fuller@gmail.com on 6 Feb 2013 at 5:58

GoogleCodeExporter commented 9 years ago
Here's a first attempt.  Mind you I haven't done objective-c since 1993 on a 
NeXTstation.  I'm not sure I've got the best naming for the autoconf stuff, as 
it kind of triggers two things - the test in configure (although nothing else 
in the code actually uses the runtime api directly) and using the gnustep 
foundation class.

It at least compiles okay with only a couple warnings:

makeheaders.c:1983:28: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:1983:49: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:2002:28: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:2002:49: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:2013:28: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:2013:49: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
makeheaders.c:2744:26: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
auth-ldap.m:169:2: warning: passing argument 1 of ‘initWithString:’ from 
distinct Objective-C type [enabled by default]
LFString.m:64:2: warning: passing argument 1 of ‘initWithString:’ from 
distinct Objective-C type [enabled by default]

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 6:53

Attachments:

GoogleCodeExporter commented 9 years ago
No luck running though, looks like your comment about TRAutoreleasePool is apt. 
 What to do now?

Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.401 openvpn[7898] 
autorelease called without pool for object (0xb913556c) of class 
NSMethodSignature in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.427 openvpn[7898] 
autorelease called without pool for object (0xb91c0d4c) of class 
NSMutableDataMalloc in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.448 openvpn[7898] 
autorelease called without pool for object (0xb9135de4) of class GSCodeBuffer 
in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.448 openvpn[7898] 
autorelease called without pool for object (0xb91c1a34) of class 
GSCInlineString in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.448 openvpn[7898] 
autorelease called without pool for object (0xb91c09d4) of class NSException in 
thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.451 openvpn[7898] 
autorelease called without pool for object (0xb91be8fc) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.452 openvpn[7898] 
autorelease called without pool for object (0xb91bdeec) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.452 openvpn[7898] 
autorelease called without pool for object (0xb91b98bc) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.452 openvpn[7898] 
autorelease called without pool for object (0xb91b7024) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.452 openvpn[7898] 
autorelease called without pool for object (0xb91b69d4) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb91b8a44) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb91b6a04) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb917346c) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb9161dfc) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb9161c84) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb915fc64) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb916816c) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.453 openvpn[7898] 
autorelease called without pool for object (0xb91680dc) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.454 openvpn[7898] 
autorelease called without pool for object (0xb916806c) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.454 openvpn[7898] 
autorelease called without pool for object (0xb9157504) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.454 openvpn[7898] 
autorelease called without pool for object (0xb914b35c) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: 2013-02-06 12:12:58.455 openvpn[7898] 
autorelease called without pool for object (0xb914efac) of class 
NSLongLongNumber in thread <NSThread: 0xb90e1304>
Feb  6 12:12:58 inferno openvpn[7898]: /usr/sbin/openvpn: Uncaught exception 
NSMallocException, reason: Allocating closure

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 7:17

GoogleCodeExporter commented 9 years ago
Awesome, thanks for looking into this.

Perhaps conveniently, our autorelease pool implementation is only in trunk, and 
we don't rely on autorelease in the 2.0 branch, so you won't have to worry 
about updating our pool implementation.

However, from this, it looks like we indirectly rely on autorelease when using 
the new autorelease runtime, which just means we need a pool in place. My 
suggestion would be to create and destroy an autorelease pool in 
openvpn_plugin_func_v1().

If GNU is going the Apple direction, this is probably done with either 
@autorelease {} or NSAutoreleasePool.

Original comment by landon.j.fuller@gmail.com on 6 Feb 2013 at 7:23

GoogleCodeExporter commented 9 years ago
Here's an updated version that seems to work.  I decided to store the pool 
pointer in the openvpn plugin handle.

It's unfortunate that using gnustep-base pulls in some more dependencies.  I 
guess the other solution would be to add more functionality to TRObject rather 
than using NSObject?

Original comment by or...@cora.nwra.com on 6 Feb 2013 at 8:38

Attachments:

GoogleCodeExporter commented 9 years ago
Unfortunately (or fortunately, depending on how you look at it) Apple (and by 
extension, GCC et al) are moving in the direction of wedding ObjC to the 
Foundation library. In later releases of Mac OS X, NSObject is actually found 
in libobjc.

The patch looks good, thanks for doing this. I have two minor suggestions:

- In auth-ldap.m, I'd keep the NSAutoreleasePool *pool on the stack, rather 
than allocating as part of the ldap_ctx. Autorelease pools are managed 
per-thread as a stack, and their lifetime is essentially tied to the 
winding/unwinding of the call stack.
- Define a generic MODERN_RUNTIME that we can enable on OS X and on GNU when 
using a 'modern' ObjC runtime that provides NSObject. Or, I guess 
RUNTIME_PROVIDES_NSOBJECT or something might be better?

Original comment by landon.j.fuller@gmail.com on 7 Feb 2013 at 4:33

GoogleCodeExporter commented 9 years ago
Okay, I put pool on the stack.  I had some build issues that were causing me to 
think that doing that wasn't working, but it seems okay so far.

On the Linux/GNU side we need to do a couple extra/gnu specific things 
including:
- We still need to specify -fgnu-runtime on Linux and -fnext-runtime on Mac
- We need -lgnustep-base library for NSObject on Linux and -framework 
Foundation on Mac.  Although I suppose one could be expected to set 
LIBS/LDFLAGS accordingly. 

Perhaps we can distinguish this based on the target system type?  The updated 
patch does that and seems to work on Fedora 18 and OS X 10.8.

Also, do you think using NSObject is still the way to go versus adding the 
needed methods (alloc, etc) to TRObject?  Probably, as it looks like Object is 
no longer defined by default on Mac.

Original comment by or...@cora.nwra.com on 7 Feb 2013 at 9:30

Attachments:

GoogleCodeExporter commented 9 years ago
So this seems to be working okay for me.  I'm going to try to get this into 
Fedora soon.  Any chance for a quick release or test tarball?

Original comment by or...@cora.nwra.com on 8 Feb 2013 at 6:35

GoogleCodeExporter commented 9 years ago
Sorry, for whatever reason Google Code didn't send me an e-mail for your last 
comment. Let me see if I can wrap up the merge and put out a release tomorrow.

Original comment by landon.j.fuller@gmail.com on 11 Feb 2013 at 11:58

GoogleCodeExporter commented 9 years ago
Thanks.

Original comment by or...@cora.nwra.com on 14 Feb 2013 at 7:25

GoogleCodeExporter commented 9 years ago
I had time to review for inclusion; the only two things remaining that I'm 
concerned about are:

- The use of the linux/darwin host type to decide between gnustep/foundation 
linker flags (we use openvpn-auth-ldap on freebsd, for instance).
- Combining the test for the runtime type (gnu/next) with the test for whether 
they're 'modern' (eg, require NSObject). It would be preferable to separate the 
test for a 'modern' runtime for the test for whether the gnu or next runtime 
could be used.

I did write an autoconf test some years ago that could be used for the former:
    https://trac.macports.org/browser/trunk/base/m4/foundation.m4#L263

I have trouble carving out sufficient time for the plugin given my day job's 
workload, so I really appreciate your time contribution here.

> Also, do you think using NSObject is still the way to go versus adding the 
needed methods (alloc, etc) to TRObject?  Probably, as it looks like Object is 
no longer defined by default on Mac.

I think we don't get much choice; the alternative is implementing our own 
NSObject-compatible root object, and it's probably easier to just inherit from 
theirs.

Original comment by landon.j.fuller@gmail.com on 19 Feb 2013 at 3:13

GoogleCodeExporter commented 9 years ago
Was there ever any resolution to this? I was using this module as part of a 
proof of concept, which worked great, but moving to gcc 4.7 has left it dying 
horribly right as it's getting the OK to roll out :/

Original comment by will.fri...@gmail.com on 7 Jan 2014 at 8:55

GoogleCodeExporter commented 9 years ago
For anyone else running into this issue, apply the patch from post #16, run 
autoconf & autoheader, run configure with "modern" argument for the runtime, 
and then make. The resulting .so should work OK.

It would be nice to have an official 2.0.4 or something, but this at least will 
get you running.

Original comment by will.fri...@gmail.com on 15 Jan 2014 at 4:40

GoogleCodeExporter commented 9 years ago
Hi Will. Thanks for looking into this. If there's any chance you have a moment 
to look into those last two patch issues that were left, I can get a proper 
2.0.4 release out.

Original comment by landon.j.fuller@gmail.com on 15 Jan 2014 at 4:43

GoogleCodeExporter commented 9 years ago
Like you, time is at a premium, but I'll leave this up as something to look at 
should the opportunity present itself :)

Original comment by will.fri...@gmail.com on 15 Jan 2014 at 10:16