Created attachment 350449 [details] libselinux-weakref.patch prelink fails to build, because libselinux.a now in rawhide now needs libpthread symbols, prelink is linked statically and definitely doesn't want to link with libpthread.a. But even for dynamically linked apps, forcing -lpthread upon all programs that don't ever use pthreads just because libselinux might be used even in threaded apps is overkill. Look what gthr-posix.h in gcc does (used by libgcc, libstdc++, ...). I'll attach an untested patch. The patch does two things: 1) use weak references to the pthread_* functions, if they are non-NULL (i.e. -lpthread is linked in), it will call the pthread_* functions, otherwise only one thread is present, no locking is needed and we can use global variables 2) (optionally) uses __thread to speed up the pthread_{s,g}etspecific macros. Only using 1) is certainly possible, just use a global array instead of __thread, call weakref_pthread_key_create always if -lpthread is linked in, remove the special destructor handling and implement pthread_{g,s}etspecific as conditional call to weakref_pthread_{g,s}etspecific or global array access. Not sure how portable libselinux aims to be, weakref attribute is available in this form only in GCC 4.2+ and TLS in most temporary glibcs (as in, last 8 years or so).
PAtch
Sorry about the prior mangled comment. The pthread dependency was introduced by a patch by tmraz (cc'd) to address a problem with freeing thread local storage, see: http://marc.info/?l=selinux&m=124160681710110&w=2 And I was going to further use it for lazy init: http://marc.info/?l=selinux&m=124654826606469&w=2 Willing to consider your patch, but obviously someone needs to test it and someone needs to post it to selinux.gov for consideration there. And it will need to be extended to address the lazy init stuff too. The original lazy init patch did use a weak reference as well, but I dropped that since we already had a hard dependency via the setrans code: http://marc.info/?l=selinux&m=124653810913046&w=2
Would it be so bad to just revert tmraz's patch that introduced the pthread dependency and instead just drop fini_context_translations(), i.e. not worry about freeing the 6 small strings cached by setrans_client.c until process exit? Then we'd only need the weak ref support for pthread_once for lazy init, and we already have code for that in the original patch.
Created attachment 350800 [details] libselinux-weakref.patch If you were using __thread unconditionally before (not sure what tmraz's patch actually did), then just going back to using it should work (i.e. tmraz's patch definitely should be reverted). It can even cause segfaults, if you dlclose libselinux.so.*, but some other thread has some translation cached at that point, then libselinux is unmapped, but when that thread terminates libpthread will crash, because the destructor for the key isn't available (forgotten pthread_key_delete call). If you don't care about leaking memory, then no pthread_once/pthread_key_create stuff is needed (but you can potentially leak memory in many threads), if you do, then it should be just used to register one key which will destruct all the __thread variables (and be only called if -lpthread is linked in). __thread variables are always initialized before they are first accessed in each thread, if initialized explicitly with that initializer, if without initializer, then they are zeroed (.tbss section). If fini_*translations crashed and setrans_client.c hadn't ever touched those variables, then you'd be looking at a toolchain bug, in that case you'd want to actually fix that bug rather than writing a buggy workarounds. For GD/LD __thread models there have been two bugs fixed in October last year in glibc, IE model might be a safer choice (but then, it would be more polite to use just one pointer instead of 6 in __thread space, make that a pointer to a struct containing those 6 pointers).
We were getting reports of segfaults in fini_context_translations() in libselinux from Debian and Ubuntu, as reported here: http://marc.info/?l=selinux&m=124159278823607&w=2 Tomas came up with a patch to fix this bug by replacing unconditional use of __thread to using pthread functions here: http://marc.info/?l=selinux&m=124160681710110&w=2 We confirmed that his patch did eliminate the seg faults and merged it. I'd be inclined to revert the patch, drop fini_context_translations, live with potential leak of 6 small strings until process exit, and thus avoid the whole mess. Ok with all parties?
Ok with me. We should comment in the library that this is what is happening.
Fixed in libselinux 2.0.85.
Hey thats my line... Fixed in libselinux 2.0.85-1.fc12