krb5 commit: Block library unloading to avoid finalizer races
ghudson at mit.edu
ghudson at mit.edu
Wed Sep 11 17:03:47 EDT 2024
https://github.com/krb5/krb5/commit/1bfcf572241a4ec0e44e609e5c6b7c0b11b08eea
commit 1bfcf572241a4ec0e44e609e5c6b7c0b11b08eea
Author: Greg Hudson <ghudson at mit.edu>
Date: Thu Aug 29 15:25:54 2024 -0400
Block library unloading to avoid finalizer races
Library finalizers can run due to the library being unloaded or the
process exiting. If the library is being unloaded, global memory
resources must be released to avoid memory leaks. But if the process
is exiting, releasing memory resources can lead to race conditions if
another thread invokes functions from the library during or after
finalizer execution. Most commonly this manifests as an assertion
error about trying to lock a destroyed mutex.
We can block unloading of our library on ELF platforms by passing "-z
nodelete" to the linker. Add a shell variable "lib_unload_prevented"
to the shlib.conf outputs, set it on platforms where we can block
unloading, and suppress finalizers when it is set.
On Windows we can detect if the process is exiting by checking for a
non-null lpvReserved argument in DllMain(). Do not execute finalizers
when the process is executing.
ticket: 9139 (new)
src/aclocal.m4 | 3 +++
src/config/shlib.conf | 34 +++++++++++++++++++++++-----------
src/include/k5-platform.h | 5 ++++-
src/lib/win_glue.c | 6 +++++-
4 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/src/aclocal.m4 b/src/aclocal.m4
index 6457cdc85..4b57d3bd0 100644
--- a/src/aclocal.m4
+++ b/src/aclocal.m4
@@ -139,6 +139,9 @@ fi
if test "$use_linker_fini_option" = yes; then
AC_DEFINE(USE_LINKER_FINI_OPTION,1,[Define if link-time options for library finalization will be used])
fi
+if test "$lib_unload_prevented" = yes; then
+ AC_DEFINE(LIB_UNLOAD_PREVENTED,1,[Define if library unloading is prevented])
+fi
])
dnl find dlopen
diff --git a/src/config/shlib.conf b/src/config/shlib.conf
index 75b7cc3af..d341ddaf9 100644
--- a/src/config/shlib.conf
+++ b/src/config/shlib.conf
@@ -32,9 +32,15 @@ DYNOBJEXT='$(SHLIBEXT)'
MAKE_DYNOBJ_COMMAND='$(MAKE_SHLIB_COMMAND)'
DYNOBJ_EXPDEPS='$(SHLIB_EXPDEPS)'
DYNOBJ_EXPFLAGS='$(SHLIB_EXPFLAGS)'
-#
+# On some platforms we will instruct the linker to run named functions
+# (specified by LIBINITFUNC and LIBFINIFUNC in each library's Makefile.in) as
+# initializers or finalizers.
use_linker_init_option=no
use_linker_fini_option=no
+# Where possible we will prevent unloading of the libraries we build, in which
+# case we can skip running finalizers. Do not set use_linker_fini_option if
+# setting lib_unload_prevented.
+lib_unload_prevented=no
STOBJEXT=.o
SHOBJEXT=.so
@@ -280,7 +286,7 @@ mips-*-netbsd*)
SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
SHLIBSEXT='.so.$(LIBMAJOR)'
SHLIBEXT=.so
- LDCOMBINE='ld -shared -soname $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+ LDCOMBINE='ld -shared -soname $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) -z nodelete'
SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
RPATH_FLAG='-Wl,-rpath -Wl,'
@@ -292,13 +298,14 @@ mips-*-netbsd*)
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
PROFFLAGS=-pg
+ lib_unload_prevented=yes
;;
*-*-netbsd*)
PICFLAGS=-fPIC
SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
SHLIBEXT=.so
- LDCOMBINE='$(CC) -shared'
+ LDCOMBINE='$(CC) -shared -Wl,-z,nodelete'
SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
RPATH_FLAG=-R
@@ -310,6 +317,7 @@ mips-*-netbsd*)
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
PROFFLAGS=-pg
+ lib_unload_prevented=yes
;;
*-*-freebsd*)
@@ -327,7 +335,7 @@ mips-*-netbsd*)
CC_LINK_SHARED='$(CC) $(PROG_LIBPATH) $(PROG_RPATH_FLAGS) $(CFLAGS) $(LDFLAGS)'
CXX_LINK_SHARED='$(CXX) $(PROG_LIBPATH) $(PROG_RPATH_FLAGS) $(CXXFLAGS) $(LDFLAGS)'
SHLIBEXT=.so
- LDCOMBINE='ld -Bshareable'
+ LDCOMBINE='ld -Bshareable -z nodelete'
SHLIB_RPATH_FLAGS='--enable-new-dtags -rpath $(SHLIB_RDIRS)'
SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
CC_LINK_STATIC='$(CC) $(PROG_LIBPATH) $(CFLAGS) $(LDFLAGS)'
@@ -335,13 +343,14 @@ mips-*-netbsd*)
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
PROFFLAGS=-pg
+ lib_unload_prevented=yes
;;
*-*-openbsd*)
PICFLAGS=-fpic
SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
SHLIBEXT=.so
- LDCOMBINE='ld -Bshareable'
+ LDCOMBINE='ld -Bshareable -z nodelete'
SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
RPATH_FLAG=-R
@@ -353,6 +362,7 @@ mips-*-netbsd*)
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
PROFFLAGS=-pg
+ lib_unload_prevented=yes
;;
*-*-darwin* | *-*-rhapsody*)
@@ -383,20 +393,19 @@ mips-*-netbsd*)
*-*-solaris*)
if test "$ac_cv_c_compiler_gnu" = yes; then
PICFLAGS=-fPIC
- LDCOMBINE='$(CC) $(CFLAGS) -shared -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+ LDCOMBINE='$(CC) $(CFLAGS) -shared -Wl,-z,nodelete -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
else
PICFLAGS=-KPIC
# Solaris cc doesn't default to stuffing the SONAME field...
- LDCOMBINE='$(CC) $(CFLAGS) -dy -G -z text -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $$initfini'
+ LDCOMBINE='$(CC) $(CFLAGS) -dy -G -z text -z nodelete -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $$initfini'
#
case $krb5_cv_host in
*-*-solaris2.[1-7] | *-*-solaris2.[1-7].*)
# Did Solaris 7 and earlier have a linker option for this?
;;
*)
- INIT_FINI_PREP='initfini=; for f in . $(LIBINITFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,initarray=$${f}__auxinit"; fi; done; for f in . $(LIBFINIFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,finiarray=$$f"; fi; done'
+ INIT_FINI_PREP='initfini=; for f in . $(LIBINITFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,initarray=$${f}__auxinit"; fi; done;'
use_linker_init_option=yes
- use_linker_fini_option=yes
;;
esac
fi
@@ -414,6 +423,7 @@ mips-*-netbsd*)
CXX_LINK_STATIC='$(PURE) $(CXX) $(PROG_LIBPATH) $(CXXFLAGS) $(LDFLAGS)'
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
+ lib_unload_prevented=yes
;;
*-*-linux* | *-*-gnu* | *-*-k*bsd*-gnu)
@@ -424,7 +434,7 @@ mips-*-netbsd*)
# Linux ld doesn't default to stuffing the SONAME field...
# Use objdump -x to examine the fields of the library
# UNDEF_CHECK is suppressed by --enable-asan
- LDCOMBINE='$(CC) -shared -fPIC -Wl,-h,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $(UNDEF_CHECK)'
+ LDCOMBINE='$(CC) -shared -fPIC -Wl,-z,nodelete -Wl,-h,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $(UNDEF_CHECK)'
UNDEF_CHECK='-Wl,--no-undefined'
# $(EXPORT_CHECK) runs export-check.pl when in maintainer mode.
LDCOMBINE_TAIL='-Wl,--version-script binutils.versions $(EXPORT_CHECK)'
@@ -442,6 +452,7 @@ mips-*-netbsd*)
CXX_LINK_STATIC='$(CXX) $(PROG_LIBPATH) $(CXXFLAGS) $(LDFLAGS)'
RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
+ lib_unload_prevented=yes
## old version:
# Linux libc does weird stuff at shlib link time, must be
@@ -459,7 +470,7 @@ mips-*-netbsd*)
PICFLAGS=-fpic
SHLIBVEXT='.so.$(LIBMAJOR)'
SHLIBEXT=.so
- LDCOMBINE='ld -Bshareable'
+ LDCOMBINE='ld -Bshareable -z nodelete'
SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
PROG_RPATH_FLAGS='-Wl,-rpath,$(PROG_RPATH)'
@@ -471,6 +482,7 @@ mips-*-netbsd*)
/:/g"`'
RUN_VARS='LD_LIBRARY_PATH'
PROFFLAGS=-pg
+ lib_unload_prevented=yes
;;
*-*-aix5*)
diff --git a/src/include/k5-platform.h b/src/include/k5-platform.h
index 11249317f..77bd6e18a 100644
--- a/src/include/k5-platform.h
+++ b/src/include/k5-platform.h
@@ -153,6 +153,9 @@
doing the pthread test at run time on systems where that works, so
we use the k5_once_t stuff instead.)
+ UNIX, with library unloading prevented or when building static
+ libraries: we don't need to run finalizers.
+
UNIX, with compiler support: MAKE_FINI_FUNCTION declares the
function as a destructor, and the run time linker support or
whatever will cause it to be invoked when the library is unloaded,
@@ -398,7 +401,7 @@ typedef struct { int error; unsigned char did_run; } k5_init_t;
# endif
-#elif !defined(SHARED)
+#elif !defined(SHARED) || defined(LIB_UNLOAD_PREVENTED)
/*
* In this case, we just don't care about finalization. The code will still
diff --git a/src/lib/win_glue.c b/src/lib/win_glue.c
index 011acdae7..d2224bdf9 100644
--- a/src/lib/win_glue.c
+++ b/src/lib/win_glue.c
@@ -401,7 +401,7 @@ control(int mode)
#ifdef _WIN32
-BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpReserved)
+BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpvReserved)
{
switch (fdwReason)
{
@@ -420,6 +420,10 @@ BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpReserved)
break;
case DLL_PROCESS_DETACH:
+ /* If lpvReserved is non-null, the process is exiting and we do not
+ * need to clean up library memory. */
+ if (lpvReserved != NULL)
+ break;
if (control(DLL_SHUTDOWN))
return FALSE;
break;
More information about the cvs-krb5
mailing list