Fix: sessiond: missing rcu read lock on client in/out events
[lttng-tools.git] / src / bin / lttng-sessiond / notification-thread-events.c
index 8d1290d38a64c64765f223c6516f24b6bcff5c23..2a9423dcfafee2933b44a3b8947fd4dbc364ece6 100644 (file)
@@ -5,6 +5,8 @@
  *
  */
 
+#include "lttng/action/action.h"
+#include "lttng/trigger/trigger-internal.h"
 #define _LGPL_SOURCE
 #include <urcu.h>
 #include <urcu/rculfhash.h>
@@ -674,6 +676,7 @@ void publish_notification_client_list(
                        lttng_trigger_get_const_condition(list->trigger);
 
        assert(!list->notification_trigger_clients_ht);
+       notification_client_list_get(list);
 
        list->notification_trigger_clients_ht =
                        state->notification_trigger_clients_ht;
@@ -1832,12 +1835,10 @@ int handle_notification_thread_command_session_rotation(
        cds_list_for_each_entry(trigger_list_element, &trigger_list->list,
                        node) {
                const struct lttng_condition *condition;
-               const struct lttng_action *action;
                struct lttng_trigger *trigger;
                struct notification_client_list *client_list;
                struct lttng_evaluation *evaluation = NULL;
                enum lttng_condition_type condition_type;
-               bool client_list_is_empty;
                enum action_executor_status executor_status;
 
                trigger = trigger_list_element->trigger;
@@ -1853,26 +1854,7 @@ int handle_notification_thread_command_session_rotation(
                        continue;
                }
 
-               action = lttng_trigger_get_const_action(trigger);
-
-               /* Notify actions are the only type currently supported. */
-               assert(lttng_action_get_type_const(action) ==
-                               LTTNG_ACTION_TYPE_NOTIFY);
-
                client_list = get_client_list_from_condition(state, condition);
-               assert(client_list);
-
-               pthread_mutex_lock(&client_list->lock);
-               client_list_is_empty = cds_list_empty(&client_list->list);
-               pthread_mutex_unlock(&client_list->lock);
-               if (client_list_is_empty) {
-                       /*
-                        * No clients interested in the evaluation's result,
-                        * skip it.
-                        */
-                       continue;
-               }
-
                if (cmd_type == NOTIFICATION_COMMAND_TYPE_SESSION_ROTATION_ONGOING) {
                        evaluation = lttng_evaluation_session_rotation_ongoing_create(
                                        trace_archive_chunk_id);
@@ -2065,9 +2047,46 @@ end:
        return ret;
 }
 
+static
+bool is_trigger_action_notify(const struct lttng_trigger *trigger)
+{
+       bool is_notify = false;
+       unsigned int i, count;
+       enum lttng_action_status action_status;
+       const struct lttng_action *action =
+                       lttng_trigger_get_const_action(trigger);
+       enum lttng_action_type action_type;
+
+       assert(action);
+       action_type = lttng_action_get_type_const(action);
+       if (action_type == LTTNG_ACTION_TYPE_NOTIFY) {
+               is_notify = true;
+               goto end;
+       } else if (action_type != LTTNG_ACTION_TYPE_GROUP) {
+               goto end;
+       }
+
+       action_status = lttng_action_group_get_count(action, &count);
+       assert(action_status == LTTNG_ACTION_STATUS_OK);
+
+       for (i = 0; i < count; i++) {
+               const struct lttng_action *inner_action =
+                               lttng_action_group_get_at_index(
+                                               action, i);
+
+               action_type = lttng_action_get_type_const(inner_action);
+               if (action_type == LTTNG_ACTION_TYPE_NOTIFY) {
+                       is_notify = true;
+                       goto end;
+               }
+       }
+
+end:
+       return is_notify;
+}
+
 /*
- * FIXME A client's credentials are not checked when registering a trigger, nor
- *       are they stored alongside with the trigger.
+ * FIXME A client's credentials are not checked when registering a trigger.
  *
  * The effects of this are benign since:
  *     - The client will succeed in registering the trigger, as it is valid,
@@ -2092,10 +2111,13 @@ int handle_notification_thread_command_register_trigger(
        struct notification_client *client;
        struct notification_client_list *client_list = NULL;
        struct lttng_trigger_ht_element *trigger_ht_element = NULL;
-       struct notification_client_list_element *client_list_element, *tmp;
+       struct notification_client_list_element *client_list_element;
        struct cds_lfht_node *node;
        struct cds_lfht_iter iter;
        bool free_trigger = true;
+       struct lttng_evaluation *evaluation = NULL;
+       struct lttng_credentials object_creds;
+       enum action_executor_status executor_status;
 
        rcu_read_lock();
 
@@ -2146,27 +2168,38 @@ int handle_notification_thread_command_register_trigger(
         * It is not skipped as this is the only action type currently
         * supported.
         */
-       client_list = notification_client_list_create(trigger);
-       if (!client_list) {
-               ret = -1;
-               goto error_free_ht_element;
-       }
-
-       /* Build a list of clients to which this new trigger applies. */
-       cds_lfht_for_each_entry(state->client_socket_ht, &iter, client,
-                       client_socket_ht_node) {
-               if (!trigger_applies_to_client(trigger, client)) {
-                       continue;
+       if (is_trigger_action_notify(trigger)) {
+               client_list = notification_client_list_create(trigger);
+               if (!client_list) {
+                       ret = -1;
+                       goto error_free_ht_element;
                }
 
-               client_list_element = zmalloc(sizeof(*client_list_element));
-               if (!client_list_element) {
-                       ret = -1;
-                       goto error_put_client_list;
+               /* Build a list of clients to which this new trigger applies. */
+               cds_lfht_for_each_entry (state->client_socket_ht, &iter, client,
+                               client_socket_ht_node) {
+                       if (!trigger_applies_to_client(trigger, client)) {
+                               continue;
+                       }
+
+                       client_list_element =
+                                       zmalloc(sizeof(*client_list_element));
+                       if (!client_list_element) {
+                               ret = -1;
+                               goto error_put_client_list;
+                       }
+
+                       CDS_INIT_LIST_HEAD(&client_list_element->node);
+                       client_list_element->client = client;
+                       cds_list_add(&client_list_element->node,
+                                       &client_list->list);
                }
-               CDS_INIT_LIST_HEAD(&client_list_element->node);
-               client_list_element->client = client;
-               cds_list_add(&client_list_element->node, &client_list->list);
+
+               /*
+                * Client list ownership transferred to the
+                * notification_trigger_clients_ht.
+                */
+               publish_notification_client_list(state, client_list);
        }
 
        switch (get_condition_binding_object(condition)) {
@@ -2190,15 +2223,18 @@ int handle_notification_thread_command_register_trigger(
        case LTTNG_OBJECT_TYPE_NONE:
                break;
        default:
-               ERR("[notification-thread] Unknown object type on which to bind a newly registered trigger was encountered");
+               ERR("Unknown object type on which to bind a newly registered trigger was encountered");
                ret = -1;
                goto error_put_client_list;
        }
 
        /*
-        * Since there is nothing preventing clients from subscribing to a
-        * condition before the corresponding trigger is registered, we have
-        * to evaluate this new condition right away.
+        * The new trigger's condition must be evaluated against the current
+        * state.
+        *
+        * In the case of `notify` action, nothing preventing clients from
+        * subscribing to a condition before the corresponding trigger is
+        * registered, we have to evaluate this new condition right away.
         *
         * At some point, we were waiting for the next "evaluation" (e.g. on
         * reception of a channel sample) to evaluate this new condition, but
@@ -2220,24 +2256,72 @@ int handle_notification_thread_command_register_trigger(
         * current state. Otherwise, the next evaluation cycle may only see
         * that the evaluations remain the same (true for samples n-1 and n) and
         * the client will never know that the condition has been met.
-        *
-        * No need to lock the list here as it has not been published yet.
         */
-       cds_list_for_each_entry_safe(client_list_element, tmp,
-                       &client_list->list, node) {
-               ret = evaluate_condition_for_client(trigger, condition,
-                               client_list_element->client, state);
-               if (ret) {
-                       goto error_put_client_list;
-               }
+       switch (get_condition_binding_object(condition)) {
+       case LTTNG_OBJECT_TYPE_SESSION:
+               ret = evaluate_session_condition_for_client(condition, state,
+                               &evaluation, &object_creds.uid,
+                               &object_creds.gid);
+               break;
+       case LTTNG_OBJECT_TYPE_CHANNEL:
+               ret = evaluate_channel_condition_for_client(condition, state,
+                               &evaluation, &object_creds.uid,
+                               &object_creds.gid);
+               break;
+       case LTTNG_OBJECT_TYPE_NONE:
+               ret = 0;
+               goto error_put_client_list;
+       case LTTNG_OBJECT_TYPE_UNKNOWN:
+       default:
+               ret = -1;
+               goto error_put_client_list;
+       }
+
+       if (ret) {
+               /* Fatal error. */
+               goto error_put_client_list;
+       }
+
+       DBG("Newly registered trigger's condition evaluated to %s",
+                       evaluation ? "true" : "false");
+       if (!evaluation) {
+               /* Evaluation yielded nothing. Normal exit. */
+               ret = 0;
+               goto error_put_client_list;
        }
 
        /*
-        * Client list ownership transferred to the
-        * notification_trigger_clients_ht.
+        * Ownership of `evaluation` transferred to the action executor
+        * no matter the result.
         */
-       publish_notification_client_list(state, client_list);
-       client_list = NULL;
+       executor_status = action_executor_enqueue(state->executor, trigger,
+                       evaluation, &object_creds, client_list);
+       evaluation = NULL;
+       switch (executor_status) {
+       case ACTION_EXECUTOR_STATUS_OK:
+               break;
+       case ACTION_EXECUTOR_STATUS_ERROR:
+       case ACTION_EXECUTOR_STATUS_INVALID:
+               /*
+                * TODO Add trigger identification (name/id) when
+                * it is added to the API.
+                */
+               ERR("Fatal error occurred while enqueuing action associated to newly registered trigger");
+               ret = -1;
+               goto error_put_client_list;
+       case ACTION_EXECUTOR_STATUS_OVERFLOW:
+               /*
+                * TODO Add trigger identification (name/id) when
+                * it is added to the API.
+                *
+                * Not a fatal error.
+                */
+               WARN("No space left when enqueuing action associated to newly registered trigger");
+               ret = 0;
+               goto error_put_client_list;
+       default:
+               abort();
+       }
 
        *cmd_result = LTTNG_OK;
 
@@ -3105,6 +3189,7 @@ int handle_notification_thread_client_in(
        size_t offset;
        bool message_is_complete = false;
 
+       rcu_read_lock();
        client = get_client_from_socket(socket, state);
        if (!client) {
                /* Internal error, abort. */
@@ -3150,12 +3235,13 @@ int handle_notification_thread_client_in(
                }
        }
 end:
+       rcu_read_unlock();
        return ret;
 error_disconnect_client:
        pthread_mutex_lock(&client->lock);
        ret = notification_thread_client_disconnect(client, state);
        pthread_mutex_unlock(&client->lock);
-       return ret;
+       goto end;
 }
 
 /* Client ready to receive outgoing data. */
@@ -3166,6 +3252,7 @@ int handle_notification_thread_client_out(
        struct notification_client *client;
        enum client_transmission_status transmission_status;
 
+       rcu_read_lock();
        client = get_client_from_socket(socket, state);
        if (!client) {
                /* Internal error, abort. */
@@ -3182,6 +3269,7 @@ int handle_notification_thread_client_out(
                goto end;
        }
 end:
+       rcu_read_unlock();
        return ret;
 }
 
@@ -3691,37 +3779,21 @@ int handle_notification_thread_channel_sample(
        cds_list_for_each_entry(trigger_list_element, &trigger_list->list,
                        node) {
                const struct lttng_condition *condition;
-               const struct lttng_action *action;
                struct lttng_trigger *trigger;
                struct notification_client_list *client_list = NULL;
                struct lttng_evaluation *evaluation = NULL;
-               bool client_list_is_empty;
                enum action_executor_status executor_status;
 
                ret = 0;
                trigger = trigger_list_element->trigger;
                condition = lttng_trigger_get_const_condition(trigger);
                assert(condition);
-               action = lttng_trigger_get_const_action(trigger);
-
-               /* Notify actions are the only type currently supported. */
-               assert(lttng_action_get_type_const(action) ==
-                               LTTNG_ACTION_TYPE_NOTIFY);
 
                /*
                 * Check if any client is subscribed to the result of this
                 * evaluation.
                 */
                client_list = get_client_list_from_condition(state, condition);
-               assert(client_list);
-               client_list_is_empty = cds_list_empty(&client_list->list);
-               if (client_list_is_empty) {
-                       /*
-                        * No clients interested in the evaluation's result,
-                        * skip it.
-                        */
-                       goto put_list;
-               }
 
                ret = evaluate_buffer_condition(condition, &evaluation, state,
                                previous_sample_available ? &previous_sample : NULL,
This page took 0.027197 seconds and 4 git commands to generate.