[cfi] Lazy CFI initialization

[vitalybuka]

Summary

If SANITIZER_CAN_USE_PREINIT_ARRAY=0 interceptors or cfi callbacks can be called
before constructor.

[vitalybuka]

+static

[eugenis]

compiler-rt/lib/cfi/cfi.cc

426 ↗

(On Diff #135586)

This mutex is not very useful. An interceptor could still go ahead while cfi_init is running in another thread. And we probably can not afford a lock in cfi_slowpath.

Perhaps add a constructor to all cfi-instrumented binaries? Then we would not need lazy initialization in __cfi_slowpath, and I don't care how many locks dlopen() takes.

[eugenis]

A constructor would also mitigate the dlopen issue (see the comment before the interceptor). Not that we've seen this issue in the wild.

[vitalybuka]

As discussed offline, I going to keep primary initialization unchanged and make lazy only interceptors initialization.

[vitalybuka]

just interceptors

[vitalybuka]

rename

[vitalybuka]

use mutex

[eugenis]

compiler-rt/lib/cfi/cfi.cc

416 ↗

(On Diff #135717)

This is technically a data race. Let's just take a lock, this is not performance sensitive.

416 ↗

(On Diff #135717)

call EnsureInterceptorsInitialized() here, or it may become a surprise if someone adds a new interceptor without that check

[eugenis]

compiler-rt/lib/cfi/cfi.cc

416 ↗

(On Diff #135717)

I see, that's too early for that.

compiler-rt/trunk/lib/cfi/cfi.cc

 
 #ifdef CFI_ENABLE_DIAG
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE void
 __cfi_slowpath_diag(u64 CallSiteTypeId, void *Ptr, void *DiagData) {
   CfiSlowPathCommon(CallSiteTypeId, Ptr, DiagData);
 }
 #endif
 
+static void EnsureInterceptorsInitialized();
+
 // Setup shadow for dlopen()ed libraries.
 // The actual shadow setup happens after dlopen() returns, which means that
 // a library can not be a target of any CFI checks while its constructors are
 // running. It's unclear how to fix this without some extra help from libc.
 // In glibc, mmap inside dlopen is not interceptable.
 // Maybe a seccomp-bpf filter?
 // We could insert a high-priority constructor into the library, but that would
 // not help with the uninstrumented libraries.
 INTERCEPTOR(void*, dlopen, const char *filename, int flag) {
+  EnsureInterceptorsInitialized();
   EnterLoader();
   void *handle = REAL(dlopen)(filename, flag);
   ExitLoader();
   return handle;
 }
 
 INTERCEPTOR(int, dlclose, void *handle) {
+  EnsureInterceptorsInitialized();
   EnterLoader();
   int res = REAL(dlclose)(handle);
   ExitLoader();
   return res;
 }
 
+static BlockingMutex interceptor_init_lock(LINKER_INITIALIZED);
+static bool interceptors_inited = false;
+
+static void EnsureInterceptorsInitialized() {
+  BlockingMutexLock lock(&interceptor_init_lock);
+  if (interceptors_inited)
+    return;
+
+  INTERCEPT_FUNCTION(dlopen);
+  INTERCEPT_FUNCTION(dlclose);
+
+  interceptors_inited = true;
+}
+
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 #if !SANITIZER_CAN_USE_PREINIT_ARRAY
 // On ELF platforms, the constructor is invoked using .preinit_array (see below)
 __attribute__((constructor(0)))
 #endif
 void __cfi_init() {
   SanitizerToolName = "CFI";
   InitializeFlags();
   InitShadow();
 
-  INTERCEPT_FUNCTION(dlopen);
-  INTERCEPT_FUNCTION(dlclose);
-
 #ifdef CFI_ENABLE_DIAG
   __ubsan::InitAsPlugin();
 #endif
 }
 
 #if SANITIZER_CAN_USE_PREINIT_ARRAY
 // On ELF platforms, run cfi initialization before any other constructors.
 // On other platforms we use the constructor attribute to arrange to run our
 // initialization early.
 extern "C" {
 __attribute__((section(".preinit_array"),
                used)) void (*__cfi_preinit)(void) = __cfi_init;
 }
 #endif