[lttng-dev] [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading

[Francis Deslauriers]

2018-02-07 15:59 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>:
> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote:
>
>> dlopen() increments the refcount of the library thus preventing the
>> refcount to reach zero in the case of dlclose;
>
> The changelog and comment do not explain _why_ this is needed.
>
> The scenario is:
>
> - an application is _not_ linked against liblttng-ust.so
> - the application dlopen() a tracepoint probe,
> - the tracepoint probe .so is linked against liblttng-ust, and this is what ends up
>   getting lttng-ust loaded,
>
> Given that our goal is to allow dlclose() of tracepoint probes (new features), we don't
> want the side effect of this dlclose() to also try to unload liblttng-ust. Because
> liblttng-ust does _not_ support being dlclose'd, we need to "pin" it in memory by
> grabbing an extra reference on the library, with a NODELETE RTLD flag.
>
I agree, here is a revised version of the commit message and comment:

Manually dlopen() liblttng-ust.so to prevent unloading

The support of dlclose() allows for the following problematic scenario:
- Application is not linked against the liblttng-ust.so
- Application dlopen() a probe provider library that is linked against
liblttng-ust.so
- Application dlclose() the probe provider

In this scenario, the probe provider has a dependency on liblttng-ust.so, so
when it's loaded by the application, liblttng-ust.so is loaded too. The probe
provider library now has the only reference to the liblttng-ust.so library.
When the application calls dlclose() on it, all its references are dropped,
thus triggering the unloading of both the probe provider library and
liblttng-ust.so.

This scenario is problematic because the probe provider needs the
liblttng-ust.so library to complete its own unloading. As we can't control the
order in which libraries are unloaded, we must ensure that liblttng-ust.so is
not unloaded before the probe provider. To prevent that unloading, we pin it in
memory by grabbing an extra reference on the library, with a RTLD_NODELETE
flag. This will prevent the dynamic loader from ever removing the
liblttng-ust.so library from the process' address space.


Thank you,
Francis

> Thanks,
>
> Mathieu
>
>
>>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
>> ---
>> configure.ac                  |  1 +
>> liblttng-ust/Makefile.am      |  2 ++
>> liblttng-ust/lttng-ust-comm.c | 22 ++++++++++++++++++++++
>> 3 files changed, 25 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index b0b4157..4fc6f9c 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
>> m4_define([UST_LIB_V_PATCH], [0])
>>
>> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
>> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
>> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
>> # note: remember to update tracepoint.h dlopen() to match this version
>> # number. TODO: eventually automate by exporting the major number.
>>
>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
>> index 982be69..a7edfd5 100644
>> --- a/liblttng-ust/Makefile.am
>> +++ b/liblttng-ust/Makefile.am
>> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
>>       string-utils.c \
>>       string-utils.h
>>
>> +liblttng_ust_runtime_la_CFLAGS =
>> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
>> +
>> if HAVE_PERF_EVENT
>> liblttng_ust_runtime_la_SOURCES += \
>>       lttng-context-perf-counters.c \
>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
>> index 511b9cf..ed912b8 100644
>> --- a/liblttng-ust/lttng-ust-comm.c
>> +++ b/liblttng-ust/lttng-ust-comm.c
>> @@ -27,6 +27,7 @@
>> #include <sys/stat.h>
>> #include <sys/types.h>
>> #include <sys/wait.h>
>> +#include <dlfcn.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <errno.h>
>> @@ -59,6 +60,9 @@
>> #include "../libringbuffer/getcpu.h"
>> #include "getenv.h"
>>
>> +/* Concatenate lttng ust shared library name with its major version number. */
>> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
>> LTTNG_UST_LIBRARY_VERSION_MAJOR
>> +
>> /*
>>  * Has lttng ust comm constructor been called ?
>>  */
>> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>       pthread_attr_t thread_attr;
>>       int timeout_mode;
>>       int ret;
>> +     void *handle;
>>
>>       if (uatomic_xchg(&initialized, 1) == 1)
>>               return;
>> @@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>       lttng_ust_loaded = 1;
>>
>>       /*
>> +      * Manually load liblttng-ust.so to increment the dynamic loader's internal
>> +      * refcount for this library so it never becomes zero, thus never gets
>> +      * unloaded from the address space of the process. Since we are already
>> +      * running in the constructor of the LTTNG_UST_LIB_SO_NAME library, calling
>> +      * dlopen will simply increment the refcount and no additionnal work is
>> +      * needed by the dynamic loader as the shared library is already loaded in
>> +      * the address space. As a safe guard, we use the RTLD_NODELETE flag to
>> +      * prevent unloading of the UST library if its refcount becomes zero
>> +      * (which should never happen). Do the return value check but discard the
>> +      * handle at the end of the function as it's not needed.
>> +      */
>> +     handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
>> +     if (!handle) {
>> +             ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
>> +     }
>> +
>> +     /*
>>        * We want precise control over the order in which we construct
>>        * our sub-libraries vs starting to receive commands from
>>        * sessiond (otherwise leading to errors when trying to create
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
Francis Deslauriers
Software developer
EfficiOS inc.