[RFC] Is this a bad idea to add a dlopen/dlclose hook?

[RFC] Is this a bad idea to add a dlopen/dlclose hook?

[lvying6]

Hi, all:

We have a project needs dlopen/dlclose hook.
There are two ways to implement these hooks:
1. LD_PRELOAD
2. modify glibc source code and export dlopen/dlclose hook.

I have investigated that the normal glibc function hook
implementation: LD_PRELOAD.
LD_PRELOAD solution: implement same name function which call dlsym
with RTLD_NEXT flag to invoke the real function in glibc. LD_PRELOAD
environment variable specifies the shared library contains the same
name function hook. So all the call of the same name glibc function
will be intercepted by the same name hook function.
This solution intercepts all the glibc function call in process.
But this solution needs to identify which process needs hook, and
re-compile the project with LD_PRELOAD environment variable.

This patch implements solution2, however, we only need to implement
function hook. And function hook can be dynamically
registered/unregistered. Also this solution does not need re-compile
the project.

Here, I want to ask is this a bad idea to directly export dlopen/dlclose
hook in souce code? Is there any other defect compared to LD_PRELOAD
solution?

Thanks in advance

Signed-off-by: lvying6 <lvying6@huawei.com>
---
 dlfcn/Versions             |  2 +-
 dlfcn/dlclose.c            | 27 +++++++++++++++++++++++++++
 dlfcn/dlfcn.h              | 15 +++++++++++++++
 dlfcn/dlopen.c             | 28 ++++++++++++++++++++++++++++
 elf/dl-support.c           |  3 +++
 elf/rtld.c                 |  1 +
 sysdeps/generic/ldsodefs.h |  3 +++
 7 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/dlfcn/Versions b/dlfcn/Versions
index 1df6925a..dfb2dab9 100644
--- a/dlfcn/Versions
+++ b/dlfcn/Versions
@@ -9,7 +9,7 @@ libdl {
     dladdr1; dlinfo;
   }
   GLIBC_2.3.4 {
-    dlmopen;
+    dlmopen; register_dlopen_hook; unregister_dlopen_hook; register_dlclose_hook; unregister_dlclose_hook;
   }
   GLIBC_PRIVATE {
     _dlfcn_hook;
diff --git a/dlfcn/dlclose.c b/dlfcn/dlclose.c
index 6afe6c8a..cf5f9245 100644
--- a/dlfcn/dlclose.c
+++ b/dlfcn/dlclose.c
@@ -35,10 +35,37 @@ dlclose_doit (void *handle)
   GLRO(dl_close) (handle);
 }
 
+static dl_hook_t dlclose_hook_func;
+void
+register_dlclose_hook(dl_hook_t hook)
+{
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  if (dlclose_hook_func != NULL) {
+    __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+    _dl_signal_error (0, NULL, NULL, "dlclose hook is already registered");
+  }
+
+  dlclose_hook_func = hook;
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+}
+
+void
+unregister_dlclose_hook(void)
+{
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  dlclose_hook_func = NULL;
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+}
+
 int
 __dlclose (void *handle)
 {
 # ifdef SHARED
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  if (dlclose_hook_func != NULL)
+	  dlclose_hook_func();
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+
   if (!rtld_active ())
     return _dlfcn_hook->dlclose (handle);
 # endif
diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
index 0921fd72..8d90addf 100644
--- a/dlfcn/dlfcn.h
+++ b/dlfcn/dlfcn.h
@@ -51,10 +51,25 @@ typedef long int Lmid_t;
 
 __BEGIN_DECLS
 
+/* This hook function prototype is used in dlopen/dlclose */
+typedef void (*dl_hook_t)(void);
+
+/* dlopen hook register function */
+extern void register_dlopen_hook(dl_hook_t hook);
+
+/* dlopen hook unregister function */
+extern void unregister_dlopen_hook(void);
+
 /* Open the shared object FILE and map it in; return a handle that can be
    passed to `dlsym' to get symbol values from it.  */
 extern void *dlopen (const char *__file, int __mode) __THROWNL;
 
+/* dlclose hook register function */
+extern void register_dlclose_hook(dl_hook_t hook);
+
+/* dlclose hook unregister function */
+extern void unregister_dlclose_hook(void);
+
 /* Unmap and close a shared object opened by `dlopen'.
    The handle cannot be used again after calling `dlclose'.  */
 extern int dlclose (void *__handle) __THROWNL __nonnull ((1));
diff --git a/dlfcn/dlopen.c b/dlfcn/dlopen.c
index 95bc62b6..9309b41d 100644
--- a/dlfcn/dlopen.c
+++ b/dlfcn/dlopen.c
@@ -70,10 +70,38 @@ dlopen_doit (void *a)
 }
 
 
+static dl_hook_t dlopen_hook_func;
+
+void
+register_dlopen_hook(dl_hook_t hook)
+{
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  if (dlopen_hook_func != NULL) {
+    __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+    _dl_signal_error (0, NULL, NULL, "dlopen hook is already registered");
+  }
+
+  dlopen_hook_func = hook;
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+}
+
+void
+unregister_dlopen_hook(void)
+{
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  dlopen_hook_func = NULL;
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+}
+
 void *
 __dlopen (const char *file, int mode DL_CALLER_DECL)
 {
 # ifdef SHARED
+  __rtld_lock_lock_recursive(GL(dl_hook_lock));
+  if (dlopen_hook_func != NULL)
+	  dlopen_hook_func();
+  __rtld_lock_unlock_recursive(GL(dl_hook_lock));
+
   if (!rtld_active ())
     return _dlfcn_hook->dlopen (file, mode, DL_CALLER);
 # endif
diff --git a/elf/dl-support.c b/elf/dl-support.c
index c8439fcc..67b6c1fc 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -217,6 +217,9 @@ __rtld_lock_define_initialized_recursive (, _dl_load_lock)
    list of loaded objects while an object is added to or removed from
    that list.  */
 __rtld_lock_define_initialized_recursive (, _dl_load_write_lock)
+/* This lock is used to register/unregister/execve hook of dlopen/dlclose
+ */
+__rtld_lock_define_initialized_recursive (, _dl_hook_lock)
 
 
 #ifdef HAVE_AUX_VECTOR
diff --git a/elf/rtld.c b/elf/rtld.c
index 4abc6dd4..e090e276 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -236,6 +236,7 @@ struct rtld_global _rtld_global =
 #ifdef _LIBC_REENTRANT
     ._dl_load_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
     ._dl_load_write_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
+	._dl_hook_lock = _RTLD_LOCK_RECURSIVE_INITIALIZER,
 #endif
     ._dl_nns = 1,
     ._dl_ns =
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 95dc8751..bd9c214f 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -345,6 +345,9 @@ struct rtld_global
      list of loaded objects while an object is added to or removed
      from that list.  */
   __rtld_lock_define_recursive (EXTERN, _dl_load_write_lock)
+  /* This lock is used to register/unregister/execve hook of dlopen/dlclose
+   */
+  __rtld_lock_define_recursive (EXTERN, _dl_hook_lock)
 
   /* Incremented whenever something may have been added to dl_loaded.  */
   EXTERN unsigned long long _dl_load_adds;
-- 
2.18.1

Re: [RFC] Is this a bad idea to add a dlopen/dlclose hook?

[Florian Weimer]

> We have a project needs dlopen/dlclose hook.
> There are two ways to implement these hooks:
> 1. LD_PRELOAD
> 2. modify glibc source code and export dlopen/dlclose hook.

We already have such hooks, via the audit mechanism: la_objsearch,
la_objopen and la_objclose.  See elf/sotruss-lib.c for an example for
that uses la_objopen.  There are other examples in the tests.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill