Fix: leak of sessiond configuration on launch of run-as worker

The run-as worker is spawned through fork() without using
exec*(). This means that any resource allocated by the session
daemon before the launch of the run-as worker will be leaked in
the run-as worker's process.

A callback is added to the run_as launch interface to allow users
a chance to clean-up after the fork occurs. This mechanism is
fragile as it may not always be easy (or possible) to track all
such resources in the future. This makes a strong argument for using a
new process image (through exec*()) and forego any such problem at
some point.

The lttng-consumerd from a similar (and more severe) problem with its
own run-as worker. A fix adressing the consumerd's problem follows.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-consumerd/lttng-consumerd.c b/src/bin/lttng-consumerd/lttng-consumerd.c
index d97dae2d1dc43667624964316a5b11b8d646e3f0..ddd07a14279eb3e88bc93b5497e8989a08cb2b3a 100644 (file)
--- a/src/bin/lttng-consumerd/lttng-consumerd.c
+++ b/src/bin/lttng-consumerd/lttng-consumerd.c
@@ -411,7 +411,7 @@ int main(int argc, char **argv)
                set_ulimit();
        }
 
-       if (run_as_create_worker(argv[0]) < 0) {
+       if (run_as_create_worker(argv[0], NULL, NULL) < 0) {
                goto exit_init_data;
        }
 

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index c45280d11583f0d8f1f44b125546b24b7d5e3232..24855b9767632bc1584316b0b0c80362d870c2e5 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -1336,6 +1336,26 @@ static void destroy_all_sessions_and_wait(void)
        DBG("Destruction of all sessions completed");
 }
 
+static int run_as_worker_post_fork_cleanup(void *data)
+{
+       struct sessiond_config *sessiond_config = data;
+
+       sessiond_config_fini(sessiond_config);
+       return 0;
+}
+
+static int launch_run_as_worker(const char *procname)
+{
+       /*
+        * Clean-up before forking the run-as worker. Any dynamically
+        * allocated memory of which the worker is not aware will
+        * be leaked as the process forks a run-as worker (and performs
+        * no exec*()). The same would apply to any opened fd.
+        */
+       return run_as_create_worker(procname, run_as_worker_post_fork_cleanup,
+                       &config);
+}
+
 /*
  * main
  */
@@ -1469,7 +1489,7 @@ int main(int argc, char **argv)
                }
        }
 
-       if (run_as_create_worker(argv[0]) < 0) {
+       if (launch_run_as_worker(argv[0]) < 0) {
                goto exit_create_run_as_worker_cleanup;
        }
 

diff --git a/src/common/runas.c b/src/common/runas.c
index d727cf4179380b094e98259f4dfa477bf41038f8..84b3513ee52d0039d7b8a488838360ca499b81f5 100644 (file)
--- a/src/common/runas.c
+++ b/src/common/runas.c
@@ -870,7 +870,9 @@ end:
 }
 
 static
-int run_as_create_worker_no_lock(const char *procname)
+int run_as_create_worker_no_lock(const char *procname,
+               post_fork_cleanup_cb clean_up_func,
+               void *clean_up_user_data)
 {
        pid_t pid;
        int i, ret = 0;
@@ -915,6 +917,12 @@ int run_as_create_worker_no_lock(const char *procname)
                reset_sighandler();
 
                set_worker_sighandlers();
+               if (clean_up_func) {
+                       if (clean_up_func(clean_up_user_data) < 0) {
+                               ERR("Run-as post-fork clean-up failed, exiting.");
+                               exit(EXIT_FAILURE);
+                       }
+               }
 
                /* Just close, no shutdown. */
                if (close(worker->sockpair[0])) {
@@ -998,7 +1006,7 @@ int run_as_restart_worker(struct run_as_worker *worker)
        run_as_destroy_worker();
 
        /* Create a new run_as worker process*/
-       ret = run_as_create_worker_no_lock(procname);
+       ret = run_as_create_worker_no_lock(procname, NULL, NULL);
        if (ret < 0 ) {
                ERR("Restarting the worker process failed");
                ret = -1;
@@ -1214,12 +1222,15 @@ int run_as_extract_sdt_probe_offsets(int fd, const char* provider_name,
 }
 
 LTTNG_HIDDEN
-int run_as_create_worker(const char *procname)
+int run_as_create_worker(const char *procname,
+               post_fork_cleanup_cb clean_up_func,
+               void *clean_up_user_data)
 {
        int ret;
 
        pthread_mutex_lock(&worker_lock);
-       ret = run_as_create_worker_no_lock(procname);
+       ret = run_as_create_worker_no_lock(procname, clean_up_func,
+                       clean_up_user_data);
        pthread_mutex_unlock(&worker_lock);
        return ret;
 }

diff --git a/src/common/runas.h b/src/common/runas.h
index 5319c26d877e0262b5dbe8cdf923262f2bb33651..4d5639c4b6b77035a071255de313f303df7a35e1 100644 (file)
--- a/src/common/runas.h
+++ b/src/common/runas.h
@@ -23,6 +23,21 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+/*
+ * The run-as process is launched by forking without an exec*() call. This means
+ * that any resource allocated before the run-as worker is launched should be
+ * cleaned-up after the fork(). This callback allows the user to perform this
+ * clean-up.
+ *
+ * Note that the callback will _not_ be invoked if the LTTNG_DEBUG_NOCLONE
+ * environment variable is set as the clean-up is not needed (and may not be
+ * expected).
+ *
+ * A negative return value will cause the run-as process to exit with a non-zero
+ * value.
+ */
+typedef int (*post_fork_cleanup_cb)(void *user_data);
+
 LTTNG_HIDDEN
 int run_as_mkdir_recursive(const char *path, mode_t mode, uid_t uid, gid_t gid);
 LTTNG_HIDDEN
@@ -41,7 +56,8 @@ int run_as_extract_sdt_probe_offsets(int fd, const char *provider_name,
                const char* probe_name, uid_t uid, gid_t gid,
                uint64_t **offsets, uint32_t *num_offset);
 LTTNG_HIDDEN
-int run_as_create_worker(const char *procname);
+int run_as_create_worker(const char *procname,
+               post_fork_cleanup_cb clean_up_func, void *clean_up_user_data);
 LTTNG_HIDDEN
 void run_as_destroy_worker(void);