Fix: sessiond ust-app session teardown race

Add a deleted flag within the ust app session which is raised (with ust
app session lock held) at delete, and checked within each RCU traversal,
again with ust app session lock held.

This takes care of races between teardown of an application (unregister)
and execution of commands which are accessing the app session
concurrently.

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

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index 5cc9622f4c444aa41994cf2de22f1fa907731dcf..2dd69495efaf095b633247b4f24b849a8571076f 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -677,6 +677,9 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess,
 
        pthread_mutex_lock(&ua_sess->lock);
 
+       assert(!ua_sess->deleted);
+       ua_sess->deleted = true;
+
        registry = get_session_registry(ua_sess);
        if (registry) {
                /* Push metadata for application before freeing the application. */
@@ -3228,6 +3231,11 @@ void ust_app_unregister(int sock)
                 */
                pthread_mutex_lock(&ua_sess->lock);
 
+               if (ua_sess->deleted) {
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       continue;
+               }
+
                /*
                 * Normally, this is done in the delete session process which is
                 * executed in the call rcu below. However, upon registration we can't
@@ -3820,6 +3828,12 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess,
                assert(ua_sess);
 
                pthread_mutex_lock(&ua_sess->lock);
+
+               if (ua_sess->deleted) {
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       continue;
+               }
+
                if (!strncmp(uchan->name, DEFAULT_METADATA_NAME,
                                        sizeof(uchan->name))) {
                        copy_channel_attr_to_ustctl(&ua_sess->metadata_attr, &uchan->attr);
@@ -3889,6 +3903,11 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess,
 
                pthread_mutex_lock(&ua_sess->lock);
 
+               if (ua_sess->deleted) {
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       continue;
+               }
+
                /* Lookup channel in the ust app session */
                lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
                ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
@@ -3955,6 +3974,12 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess,
                }
 
                pthread_mutex_lock(&ua_sess->lock);
+
+               if (ua_sess->deleted) {
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       continue;
+               }
+
                /* Lookup channel in the ust app session */
                lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
                ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
@@ -4006,6 +4031,11 @@ int ust_app_start_trace(struct ltt_ust_session *usess, struct ust_app *app)
 
        pthread_mutex_lock(&ua_sess->lock);
 
+       if (ua_sess->deleted) {
+               pthread_mutex_unlock(&ua_sess->lock);
+               goto end;
+       }
+
        /* Upon restart, we skip the setup, already done */
        if (ua_sess->started) {
                goto skip_setup;
@@ -4106,6 +4136,11 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app)
 
        pthread_mutex_lock(&ua_sess->lock);
 
+       if (ua_sess->deleted) {
+               pthread_mutex_unlock(&ua_sess->lock);
+               goto end_no_session;
+       }
+
        /*
         * If started = 0, it means that stop trace has been called for a session
         * that was never started. It's possible since we can have a fail start
@@ -4186,6 +4221,10 @@ int ust_app_flush_app_session(struct ust_app *app,
 
        pthread_mutex_lock(&ua_sess->lock);
 
+       if (ua_sess->deleted) {
+               goto end_deleted;
+       }
+
        health_code_update();
 
        /* Flushing buffers */
@@ -4215,6 +4254,7 @@ int ust_app_flush_app_session(struct ust_app *app,
 
        health_code_update();
 
+end_deleted:
        pthread_mutex_unlock(&ua_sess->lock);
 
 end_not_compatible:
@@ -4448,6 +4488,11 @@ void ust_app_global_create(struct ltt_ust_session *usess, struct ust_app *app)
 
        pthread_mutex_lock(&ua_sess->lock);
 
+       if (ua_sess->deleted) {
+               pthread_mutex_unlock(&ua_sess->lock);
+               goto end;
+       }
+
        /*
         * We can iterate safely here over all UST app session since the create ust
         * app session above made a shadow copy of the UST global domain from the
@@ -4590,6 +4635,12 @@ int ust_app_add_ctx_channel_glb(struct ltt_ust_session *usess,
                }
 
                pthread_mutex_lock(&ua_sess->lock);
+
+               if (ua_sess->deleted) {
+                       pthread_mutex_unlock(&ua_sess->lock);
+                       continue;
+               }
+
                /* Lookup channel in the ust app session */
                lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
                ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
@@ -4648,6 +4699,12 @@ int ust_app_enable_event_pid(struct ltt_ust_session *usess,
        }
 
        pthread_mutex_lock(&ua_sess->lock);
+
+       if (ua_sess->deleted) {
+               ret = 0;
+               goto end_unlock;
+       }
+
        /* Lookup channel in the ust app session */
        lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &iter);
        ua_chan_node = lttng_ht_iter_get_node_str(&iter);

diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h
index aa66059adafb5aaa2d0270f0f0634ca3414b34b6..b62e218a03a74842d8bcf08dc3dbef0471f6f01e 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.h
+++ b/src/bin/lttng-sessiond/ust-app.h
@@ -182,6 +182,8 @@ struct ust_app_session {
        int started;  /* allows detection of start vs restart. */
        int handle;   /* used has unique identifier for app session */
 
+       bool deleted;   /* Session deleted flag. Check with lock held. */
+
        /*
         * Tracing session ID. Multiple ust app session can have the same tracing
         * session id making this value NOT unique to the object.