From 1bfcf572241a4ec0e44e609e5c6b7c0b11b08eea Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Thu, 29 Aug 2024 15:25:54 -0400 Subject: [PATCH] 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 6457cdc85b..4b57d3bd02 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 75b7cc3af6..d341ddaf91 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 11249317f9..77bd6e18a8 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 011acdae76..d2224bdf94 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; -- 2.47.2